redefinition of dip::clamp_cast()

slokhorst commented 5 years ago

When compiling diplib with GCC 8.2.1:

[ 31%] Building CXX object src/CMakeFiles/DIP.dir/analysis/chord_length.cpp.o
In file included from /home/sebas/Projects/diplib/include/diplib.h:30,
                 from /home/sebas/Projects/diplib/src/analysis/chord_length.cpp:23:
/home/sebas/Projects/diplib/include/diplib/library/clamp_cast.h:225:20: error: redefinition of ‘template<class T> constexpr T dip::clamp_cast(dip::uint)’
 constexpr inline T clamp_cast( dip::uint v ) {
                    ^~~~~~~~~~
/home/sebas/Projects/diplib/include/diplib/library/clamp_cast.h:200:20: note: ‘template<class T> constexpr T dip::clamp_cast(dip::uint64)’ previously declared here
 constexpr inline T clamp_cast( uint64 v ) {
                    ^~~~~~~~~~
In file included from /home/sebas/Projects/diplib/include/diplib.h:30,
                 from /home/sebas/Projects/diplib/src/analysis/chord_length.cpp:23:
/home/sebas/Projects/diplib/include/diplib/library/clamp_cast.h:342:20: error: redefinition of ‘template<class T> constexpr T dip::clamp_cast(dip::sint)’
 constexpr inline T clamp_cast( dip::sint v ) {
                    ^~~~~~~~~~
/home/sebas/Projects/diplib/include/diplib/library/clamp_cast.h:313:20: note: ‘template<class T> constexpr T dip::clamp_cast(dip::sint64)’ previously declared here
 constexpr inline T clamp_cast( sint64 v ) {
                    ^~~~~~~~~~
make[2]: *** [src/CMakeFiles/DIP.dir/build.make:76: src/CMakeFiles/DIP.dir/analysis/chord_length.cpp.o] Error 1

This seems to be because dip::uint and dip::uint64 are the same type, and so are dip::sint and dip::sint64. E.g. for uint:

https://github.com/DIPlib/diplib/blob/c528265acad57a296c7a77b702e594f49e5baf8c/include/diplib/library/types.h#L67

https://github.com/DIPlib/diplib/blob/c528265acad57a296c7a77b702e594f49e5baf8c/include/diplib/library/types.h#L90

And of course, std::size_t == std::uint64_t on 64-bit systems.

Looks like this bug was introduced in a74647352d7be8a4250836b87916a52f42319c9f

crisluengo commented 5 years ago

I'm using GCC 8.3 on MacOS and don't have this issue. I also don't see it with GCC 5.4 on Linux. I actually needed to create the dip::uint and dip::uint64 overloads separately because the compiler couldn't resolve the one with the other.

I will have to add a conditional there to determine if those types are considered the same.

slokhorst commented 5 years ago

Out of curiosity, when do you need to work with dip::uint/std::size_t data? It's quite unpredictable as we see here (and you mention yourself: "it makes assumptions about the size of the pointer"). And you will usually know the exact pixel type.

I'm not familiar with the code and I'm sure it's there for a reason.. just curious.

crisluengo commented 5 years ago

It is for cases where you create an image with constant data or data from e.g. a dip::IntegerArray (by copy). This happens also when e.g. multiplying an image by a constant. For example img / img.NumberOfPixels().

slokhorst commented 5 years ago

Ah ok, thanks for the info.

I just set up Travis CI for testing, which uses GCC 5.4.0 on Ubuntu 16.04 by default, and it also fails there: https://travis-ci.org/slokhorst/diplib/builds/520934430

crisluengo commented 5 years ago

Indeed, I had not tested on Linux. I usually do. A CI setup would certainly be useful.

This seems to be a platform-dependent thing. I've re-written dip::clamp_cast and a few other things. This now works correctly on MacOS with GCC 8.3 and Apple Clang 10, and on Linux with GCC 5.4. I cannot readily test on Windows, unfortunately.

@slokhorst, could you help me set up Travis CI for this repository?

DIPlib / diplib

redefinition of dip::clamp_cast() #31