From: Sean M. <se...@ro...> - 2012-03-28 20:07:05
Attachments:
vxl.patch
|
Hi all, Attached is a patch that fixes compiler errors building vxl as C++11 (with clang). 3 changes: - replace a -1 with UINT_MAX - conditionalize a Mac OS X 10.4 hack to be done only if using the 10.4 headers - in C++11 only, implement math wrappers like vnl_math_isnan using std::isnan (new in C++11) I tested this only on OS X 10.7 with clang, but the changes are straightforward. Please review. Cheers, -- ____________________________________________________________ Sean McBride, B. Eng se...@ro... Rogue Research www.rogue-research.com Mac Software Developer Montréal, Québec, Canada |
From: Peter V. <pet...@ya...> - 2012-03-29 19:07:17
|
Sorry Sean, it seems like I had roughly the same idea as you, before I saw your post; but I used 0xffffffff instead of UINT_MAX. Which of the two is preferred? -- Peter. Sean McBride wrote: Index: core/vgl/vgl_triangle_3d.cxx =================================================================== --- core/vgl/vgl_triangle_3d.cxx (revision 34632) +++ core/vgl/vgl_triangle_3d.cxx (working copy) @@ -375,7 +375,7 @@ namespace { - static const unsigned calc_edge_index_lookup[8] = {-1, 0, 2, 0, -1, 1, 2, 1}; + static const unsigned calc_edge_index_lookup[8] = {UINT_MAX, 0, 2, 0, UINT_MAX, 1, 2, 1}; //: Given the [0,2] index of two vertices, in either order, return the edge index [0,2] // E.g. between vertices 2 and 0 is edge 2. // Use precalculated list lookup, probably fastest. |
From: Sean M. <se...@ro...> - 2012-03-29 19:25:14
|
Peter, Well, I'm not familiar with the code, or what it's trying to do, but I'd say UINT_MAX. The C++ standard does not guarantee that 'int' is 32 bits, so 0xffffffff and UINT_MAX are not necessarily the same. Sean On Thu, 29 Mar 2012 20:07:08 +0100, Peter Vanroose said: >Sorry Sean, >it seems like I had roughly the same idea as you, before I saw your post; >but I used 0xffffffff instead of UINT_MAX. > >Which of the two is preferred? > > >-- Peter. > > >Sean McBride wrote: >Index: core/vgl/vgl_triangle_3d.cxx >=================================================================== >--- core/vgl/vgl_triangle_3d.cxx (revision 34632) >+++ core/vgl/vgl_triangle_3d.cxx (working copy) >@@ -375,7 +375,7 @@ > > namespace > { >- static const unsigned calc_edge_index_lookup[8] = {-1, 0, 2, 0, -1, >1, 2, 1}; >+ static const unsigned calc_edge_index_lookup[8] = {UINT_MAX, 0, 2, 0, >UINT_MAX, 1, 2, 1}; > //: Given the [0,2] index of two vertices, in either order, return >the edge index [0,2] > // E.g. between vertices 2 and 0 is edge 2. > // Use precalculated list lookup, probably fastest. |
From: Peter V. <pet...@ya...> - 2012-03-29 19:35:51
|
OK, I'll change it now. -- Peter. -- ________________________________ Från: Sean McBride <se...@ro...> Well, I'm not familiar with the code, or what it's trying to do, but I'd say UINT_MAX. The C++ standard does not guarantee that 'int' is 32 bits, so 0xffffffff and UINT_MAX are not necessarily the same. Sean On Thu, 29 Mar 2012 20:07:08 +0100, Peter Vanroose said: >Sorry Sean, >it seems like I had roughly the same idea as you, before I saw your post; >but I used 0xffffffff instead of UINT_MAX. > >Which of the two is preferred? > > >-- Peter. > > >Sean McBride wrote: >Index: core/vgl/vgl_triangle_3d.cxx >=================================================================== >--- core/vgl/vgl_triangle_3d.cxx (revision 34632) >+++ core/vgl/vgl_triangle_3d.cxx (working copy) >@@ -375,7 +375,7 @@ > > namespace > { >- static const unsigned calc_edge_index_lookup[8] = {-1, 0, 2, 0, -1, >1, 2, 1}; >+ static const unsigned calc_edge_index_lookup[8] = {UINT_MAX, 0, 2, 0, >UINT_MAX, 1, 2, 1}; > //: Given the [0,2] index of two vertices, in either order, return >the edge index [0,2] > // E.g. between vertices 2 and 0 is edge 2. > // Use precalculated list lookup, probably fastest. |
From: Gehua Y. <yan...@gm...> - 2012-03-29 20:36:11
|
Hi Peter, Just a dumb question without looking at the code: wouldn't it be better if calc_edge_index_lookup is an array of signed int instead? I wonder if it will be a quick change as you are already on it. Gehua. On Thu, Mar 29, 2012 at 3:35 PM, Peter Vanroose <pet...@ya...>wrote: > OK, I'll change it now. > > -- Peter. > > > -- > ------------------------------ > *Från:* Sean McBride <se...@ro...> > ** > Well, I'm not familiar with the code, or what it's trying to do, but I'd > say UINT_MAX. The C++ standard does not guarantee that 'int' is 32 bits, > so 0xffffffff and UINT_MAX are not necessarily the same. > > Sean > > On Thu, 29 Mar 2012 20:07:08 +0100, Peter Vanroose said: > > >Sorry Sean, > >it seems like I had roughly the same idea as you, before I saw your post; > >but I used 0xffffffff instead of UINT_MAX. > > > >Which of the two is preferred? > > > > > >-- Peter. > > > > > >Sean McBride wrote: > >Index: core/vgl/vgl_triangle_3d.cxx > >=================================================================== > >--- core/vgl/vgl_triangle_3d.cxx (revision 34632) > >+++ core/vgl/vgl_triangle_3d.cxx (working copy) > >@@ -375,7 +375,7 @@ > > > > namespace > > { > >- static const unsigned calc_edge_index_lookup[8] = {-1, 0, 2, 0, -1, > >1, 2, 1}; > >+ static const unsigned calc_edge_index_lookup[8] = {UINT_MAX, 0, 2, 0, > >UINT_MAX, 1, 2, 1}; > > //: Given the [0,2] index of two vertices, in either order, return > >the edge index [0,2] > > // E.g. between vertices 2 and 0 is edge 2. > > // Use precalculated list lookup, probably fastest. > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------------------------ > This SF email is sponsosred by: > Try Windows Azure free for 90 days Click Here > http://p.sf.net/sfu/sfd2d-msazure > _______________________________________________ > Vxl-users mailing list > Vxl...@li... > https://lists.sourceforge.net/lists/listinfo/vxl-users > > |
From: Peter V. <pet...@ya...> - 2012-03-30 07:00:12
|
No, that would break the code: there is e.g. the condition a bit further asserting that the lookup value should be < 3; a negative value would pass, while it shouldn't. -- Peter. -- ________________________________ Från: Gehua Yang <yan...@gm...> Till: Peter Vanroose <p.v...@ie...> Kopia: "vxl...@li..." <vxl...@li...> Skickat: torsdag, 29 mars 2012 22:36 Ämne: Re: [Vxl-users] [PATCH] fix for compiler errors with clang with C++11 Hi Peter, Just a dumb question without looking at the code: wouldn't it be better if calc_edge_index_lookup is an array of signed int instead? I wonder if it will be a quick change as you are already on it. Gehua. On Thu, Mar 29, 2012 at 3:35 PM, Peter Vanroose <pet...@ya...> wrote: OK, I'll change it now. > > >-- Peter. > > > > >-- > > >________________________________ > Från: Sean McBride <se...@ro...> > > >Well, I'm not familiar with the code, or what it's trying to do, but I'd say UINT_MAX. The C++ standard does not guarantee that 'int' is 32 bits, so 0xffffffff and UINT_MAX are not necessarily the same. > >Sean > >On Thu, 29 Mar 2012 20:07:08 +0100, Peter Vanroose said: > >>Sorry Sean, >>it seems like I had roughly the same idea as you, before I saw your post; >>but I used 0xffffffff instead of UINT_MAX. >> >>Which of the two is preferred? >> >> >>-- Peter. >> >> >>Sean McBride wrote: >>Index: core/vgl/vgl_triangle_3d.cxx >>=================================================================== >>--- core/vgl/vgl_triangle_3d.cxx (revision 34632) >>+++ core/vgl/vgl_triangle_3d.cxx (working copy) >>@@ -375,7 +375,7 @@ >> >> namespace >> { >>- static const unsigned calc_edge_index_lookup[8] = {-1, 0, 2, 0, -1, >>1, 2, 1}; >>+ static const unsigned calc_edge_index_lookup[8] = {UINT_MAX, 0, 2, 0, >>UINT_MAX, 1, 2, 1}; >> //: Given the [0,2] index of two vertices, in either order, return >>the edge index [0,2] >> // E.g. between vertices 2 and 0 is edge 2. >> // Use precalculated list lookup, probably fastest. > > > > > > > > > > > > > > > > > > > > > >------------------------------------------------------------------------------ >This SF email is sponsosred by: >Try Windows Azure free for 90 days Click Here >http://p.sf.net/sfu/sfd2d-msazure >_______________________________________________ >Vxl-users mailing list >Vxl...@li... >https://lists.sourceforge.net/lists/listinfo/vxl-users > > ------------------------------------------------------------------------------ This SF email is sponsosred by: Try Windows Azure free for 90 days Click Here http://p.sf.net/sfu/sfd2d-msazure _______________________________________________ Vxl-users mailing list Vxl...@li... https://lists.sourceforge.net/lists/listinfo/vxl-users |
From: Sean M. <se...@ro...> - 2012-04-18 00:46:47
|
On Wed, 28 Mar 2012 16:06:58 -0400, Sean McBride said: >Attached is a patch that fixes compiler errors building vxl as C++11 >(with clang). > >3 changes: >- replace a -1 with UINT_MAX >- conditionalize a Mac OS X 10.4 hack to be done only if using the 10.4 >headers >- in C++11 only, implement math wrappers like vnl_math_isnan using >std::isnan (new in C++11) > >I tested this only on OS X 10.7 with clang, but the changes are >straightforward. Please review. Peter, It seems you applied the first part of my patch, thanks! What about the other two parts? Did you have a chance to review them? Thanks, -- ____________________________________________________________ Sean McBride, B. Eng se...@ro... Rogue Research www.rogue-research.com Mac Software Developer Montréal, Québec, Canada |
From: Peter V. <pet...@ya...> - 2012-04-19 20:14:18
|
Sorry, I apparently forgot about that. I was not able to test the other two patches, since I don't have the necessary infrastructure, but if that's fine with everybody I'll just apply them and commit to SVN. -- Peter. ________________________________ Från: Sean McBride <se...@ro...> Till: vxl...@li... Skickat: onsdag, 18 april 2012 2:46 Ämne: Re: [Vxl-users] [PATCH] fix for compiler errors with clang with C++11 It seems you applied the first part of my patch, thanks! What about the other two parts? Did you have a chance to review them? >Attached is a patch that fixes compiler errors building vxl as C++11 (with clang). >3 changes: >- replace a -1 with UINT_MAX >- conditionalize a Mac OS X 10.4 hack to be done only if using the 10.4 headers >- in C++11 only, implement math wrappers like vnl_math_isnan using std::isnan (new in C++11) > >I tested this only on OS X 10.7 with clang, but the changes are straightforward. Please review. ____________________________________________________________ Sean McBride, B. Eng se...@ro... Rogue Research www.rogue-research.com Mac Software Developer Montréal, Québec, Canada -- |
From: Sean M. <se...@ro...> - 2012-04-19 20:39:50
|
On Thu, 19 Apr 2012 21:01:28 +0100, Peter Vanroose said: >Sorry, I apparently forgot about that. >I was not able to test the other two patches, since I don't have the >necessary infrastructure, but if that's fine with everybody I'll just >apply them and commit to SVN. I guess the only part people might not like is the "#if __cplusplus > 199711L" check for C++11. If some compiler passes that test yet does not provide std::isnan, there will be an error. Some people prefer to do 'try compile'-type tests of the compiler, but I don't know how to do that, nor do I know what the vxl preference is. However you want to do it is fine with me. :) Cheers, -- ____________________________________________________________ Sean McBride, B. Eng se...@ro... Rogue Research www.rogue-research.com Mac Software Developer Montréal, Québec, Canada |
From: Amitha P. <am...@th...> - 2012-04-20 11:07:26
|
On 4/19/2012 4:39 PM, Sean McBride wrote: > I guess the only part people might not like is the "#if __cplusplus > > 199711L" check for C++11. If some compiler passes that test yet does > not provide std::isnan, there will be an error. Some people prefer to > do 'try compile'-type tests of the compiler, but I don't know how to > do that, nor do I know what the vxl preference is. However you want to > do it is fine with me. :) Cheers, Sean, that macro test is somewhat inconsistent with the vxl way. It seems to me that the check should be #if VXL_CXX11 ... #endif You should then add to vxl/vcl_compiler.h something like #if __cplusplus > 199711L #define VXL_CXX11 1 #else #define VXL_CXX11 0 #endif if this check is expected to work across all compilers, including non-CXX11 compilers. (I.e. if it a standard-defined way to check for a C++11 compiler.) If it is a check for a specific compiler, like clang, then the stanzas in vcl_compiler.h should be more like // Assume the compiler is not CXX 11 by default #define VXL_CXX11 0 ... #if <check for clang> # if __cplusplus > 199711L #undef VXL_CXX11 #define VXL_CXX11 1 #endif #endif Since the intent seems to be to use this on a CXX 11 compiler, as opposed to on any compiler that happens to have std::nan, a test in vcl_compiler.h is probably the better way over a try-compile. Thanks, Amitha. |
From: Sean M. <se...@ro...> - 2012-04-20 15:39:22
|
On Fri, 20 Apr 2012 06:35:52 -0400, Amitha Perera said: >On 4/19/2012 4:39 PM, Sean McBride wrote: >> I guess the only part people might not like is the "#if __cplusplus > >> 199711L" check for C++11. If some compiler passes that test yet does >> not provide std::isnan, there will be an error. Some people prefer to >> do 'try compile'-type tests of the compiler, but I don't know how to >> do that, nor do I know what the vxl preference is. However you want to >> do it is fine with me. :) Cheers, > >Sean, that macro test is somewhat inconsistent with the vxl way. I know little of vxl, so I'll take your word. :) Peter, shall I update the patch as Amitha suggests? >seems to me that the check should be > #if VXL_CXX11 > ... > #endif > >You should then add to vxl/vcl_compiler.h something like > #if __cplusplus > 199711L > #define VXL_CXX11 1 > #else > #define VXL_CXX11 0 > #endif >if this check is expected to work across all compilers, including >non-CXX11 compilers. (I.e. if it a standard-defined way to check for a >C++11 compiler.) If it is a check for a specific compiler, like clang, >then the stanzas in vcl_compiler.h should be more like > > // Assume the compiler is not CXX 11 by default > #define VXL_CXX11 0 > ... > #if <check for clang> > # if __cplusplus > 199711L > #undef VXL_CXX11 > #define VXL_CXX11 1 > #endif > #endif > >Since the intent seems to be to use this on a CXX 11 compiler, as >opposed to on any compiler that happens to have std::nan, a test in >vcl_compiler.h is probably the better way over a try-compile. FYI, any C++11-conforming compiler must pass the "#if __cplusplus > 199711L" test, see : <http://www2.research.att.com/~bs/C++0xFAQ.html#0x> likewise std::isnan is part of the standard: <http://www2.research.att.com/~bs/C++0xFAQ.html#C99> nothing about my patch is specific to clang, though that's the only compiler I tested with. Cheers, -- ____________________________________________________________ Sean McBride, B. Eng se...@ro... Rogue Research www.rogue-research.com Mac Software Developer Montréal, Québec, Canada |
From: Peter V. <pet...@ya...> - 2012-04-20 16:48:02
|
> Peter, shall I update the patch as Amitha suggests? Certainly; sounds good to me. -- Peter. |
From: Sean M. <se...@ro...> - 2012-04-20 21:59:29
Attachments:
VXL_CXX11.patch
|
On Fri, 20 Apr 2012 17:47:55 +0100, Peter Vanroose said: >> Peter, shall I update the patch as Amitha suggests? > >Certainly; sounds good to me. See attached. Also fixes a little warning I happened to notice. -- ____________________________________________________________ Sean McBride, B. Eng se...@ro... Rogue Research www.rogue-research.com Mac Software Developer Montréal, Québec, Canada |
From: Peter V. <pet...@ya...> - 2012-04-21 12:09:25
|
Sean McBride wrote: > See attached. Also fixes a little warning I happened to notice. OK, done. -- Peter. -- |