Laurent Rineau (GeometryFactory) wrote:
> On Wednesday 29 July 2009 14:05:41 Sylvain Pion wrote:
>> Laurent Rineau (GeometryFactory) wrote:
>>> One GF customer reported the following issue:
>>>
>>> "For 64bit compilation, we get the disturbing warning
>>>
>>> ..\Third_Party_Libraries\CGAL\include\CGAL/Handle.h(90) : warning C4311:
>>> 'reinterpret_cast' : pointer truncation from 'CGAL::Rep *const ' to
>>> 'unsigned long'
>>>
>>> which looks like a real 64bit issue."
>>>
>>> and he is right. Here is the code:
>>>
>>> inline
>>> bool
>>> identical(const Handle &h1, const Handle &h2)
>>> { return reinterpret_cast<unsigned long>(h1.PTR) ==
>>> reinterpret_cast<unsigned long>(h2.PTR); }
>>>
>>>
>>> "unsigned long" is not the right type! On 32 bits machine, that type is
>>> too long, and on x64 under Windows, that type is too short!
>> Indeed...
>>
>>> See http://en.wikipedia.org/wiki/LLP64#Specific_data_models
>>>
>>> We need uintptr_t from C99, because that type is defined as the fastest
>>> unsigned integral type that is convertible from void pointers and
>>> comvertible to void pointers, without truncation.
>>>
>>> However, that type is not yet in the C++ norm, and some platforms may not
>>> have it. We will probably need some macro stuff:
>>> - use C++0x if possible,
>>> - fallback to plain C99 headers,
>>> - then fallback to platform specific typedefs.
>>>
>>> Is that the correct solution?
>> For the identical() function above, why don't we simply compare
>> the pointers without doing the casts to unsigned long at all ?
>>
>> Nevertheless, the problem exists in the id() function (and this
>> function is probably less useful...).
>> What about returning std::ptrdiff_t with {return h.PTR -
>> static_cast<Ref*>(0);} ?
>
> Yes. That might be a good cross-platform solution for id(). I will commit that
> in the trunk. And identical() will be a.id()==b.id().
Why not just h1.PTR == h2.PTR for identical() ?
So, let's go and break the trunk! ;-)
I have a least compiled and run test/Kernel_23/Lazy_kernel.cpp
changing functions to a separate header, not included by default
(<CGAL/assertions_behaviour.h>). The motivation is to hide the enum
values with risky names (ABORT, EXIT, CONTINUE) to a header file which
is most probably not used by any user (or very few).
(breaks backward compatibility for an expected very few, for the sake
of erasing random surprises for "many" ?)
There were some differences :
- CGAL::Object has no template constructor, so using make_object() was required.
- CGAL::Object had no comparison with NULL to check emptyness. I added
such comparison operators to CGAL::Object as *deprecated*.
- C++0x's std:array from <array>
- TR1's std::tr1::array from <tr1/array>
- boost::array from <boost/array.hpp>
Motivation : GCC's std::array is faster than boost::array.
Move CGALi:make_array to namespace CGAL.
Document CGAL::array.
bool are_sorted(const T & a, const T & b, const T & c)
bool are_strictly_sorted(const T & a, const T & b, const T & c)
bool are_ordered(const T & a, const T & b, const T & c)
bool are_strictly_ordered(const T & a, const T & b, const T & c)
with a Compare functor as last argument.
bool are_sorted(const T & a, const T & b, const T & c)
bool are_strictly_sorted(const T & a, const T & b, const T & c)
bool are_ordered(const T & a, const T & b, const T & c)
bool are_strictly_ordered(const T & a, const T & b, const T & c)
Sylvain Pion
Laurent Rineau
Andreas Fabri
Sébastien Loriot