The VXL Project

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.

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.

OK, I'll change it now.

 
--	Peter.



--


________________________________
 Från: Sean McBride <sean@...>

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.

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
<peter_vanroose@...:

> OK, I'll change it now.
>
> -- Peter.
>
>
> --
>   ------------------------------
> *Från:* Sean McBride <sean@...>
> **
> 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-users@...
> https://lists.sourceforge.net/lists/listinfo/vxl-users
>
>

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 <yanggehua@...>
Till: Peter Vanroose <p.vanroose@...> 
Kopia: "vxl-users@..." <vxl-users@...> 
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 <peter_vanroose@...> wrote:

OK, I'll change it now.
>
> 
>--	Peter.
>
>
>
>
>--
>
>
>________________________________
> Från: Sean McBride <sean@...>
>
>
>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-users@...
>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-users@...
https://lists.sourceforge.net/lists/listinfo/vxl-users

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                 sean@...
Rogue Research                        http://www.rogue-research.com 
Mac Software Developer              Montréal, Québec, Canada

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 <sean@...>
Till: vxl-users@... 
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                sean@...
Rogue Research                        http://www.rogue-research.com 
Mac Software Developer              Montréal, Québec, Canada










--

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                 sean@...
Rogue Research                        http://www.rogue-research.com 
Mac Software Developer              Montréal, Québec, Canada

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.

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                 sean@...
Rogue Research                        http://www.rogue-research.com 
Mac Software Developer              Montréal, Québec, Canada

> Peter, shall I update the patch as Amitha suggests?


Certainly; sounds good to me.

--      Peter.

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                 sean@...
Rogue Research                        http://www.rogue-research.com 
Mac Software Developer              Montréal, Québec, Canada

Sean McBride wrote:

> See attached.  Also fixes a little warning I happened to notice.

OK, done.

 
--	Peter.


















--