simbody icon indicating copy to clipboard operation
simbody copied to clipboard

Clang10 /w C++20 error: SimTK indexes: use of overloaded operator '==' is ambiguous

Open adamkewley opened this issue 5 years ago • 1 comments

I'm compiling a downstream project that uses OpenSim, but (transitively) includes Simbody. Top-level compiler details:

# clang++
Debian clang version 10.0.1-++20200708124224+ef32c611aa2-1~exp1~20200707224822.188 
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
# clang
Debian clang version 10.0.1-++20200708124224+ef32c611aa2-1~exp1~20200707224822.188 
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Installed as a prebuilt binary from the llvm project site. Effectively, I unpacked it and then configured a project that uses it. Command-line looks something like this:

CXX=clang++-10 CC=clang-10 cmake -S ../osmv -B . -DCMAKE_PREFIX_PATH=$(readlink -f ~/Desktop/osc/master-RelWithDebInfo-install/lib/cmake) -DCMAKE_GENERATOR=Ninja
cmake --build . --target all

This compiles completely fine if my project is set to C++17, e.g. with:

target_compile_features(osmv PUBLIC cxx_std_17)
# broken: cxx_std_20

When compiling under C++20, it breaks with (gisted, because it's a big error dump):

https://gist.github.com/adamkewley/36c726f82afe89bffa087a5b9d02f50a

Excerpt of that gist:

/home/adam/Desktop/osc/master-RelWithDebInfo-install/include/simbody/simmath/internal/ContactGeometry.h:840:27: error: use of overloaded operator '==' is ambiguous (with operand types 'SimTK::ContactGeometryTypeId' and 'SimTK::ContactGeometryTypeId')
{   return geo.getTypeId()==classTypeId(); }
           ~~~~~~~~~~~~~~~^ ~~~~~~~~~~~~~
/home/adam/Desktop/osc/master-RelWithDebInfo-install/include/simbody/simmath/internal/ContactGeometry.h:44:1: note: candidate function
SimTK_DEFINE_UNIQUE_INDEX_TYPE(ContactGeometryTypeId);
^
/home/adam/Desktop/osc/master-RelWithDebInfo-install/include/simbody/SimTKcommon/internal/common.h:427:5: note: expanded from macro 'SimTK_DEFINE_UNIQUE_INDEX_TYPE'
    SimTK_DEFINE_AND_EXPORT_UNIQUE_LOCAL_INDEX_TYPE(,,,NAME)   \
    ^
/home/adam/Desktop/osc/master-RelWithDebInfo-install/include/simbody/SimTKcommon/internal/common.h:459:10: note: expanded from macro 'SimTK_DEFINE_AND_EXPORT_UNIQUE_LOCAL_INDEX_TYPE'
    bool operator==(int  i) const {assert(isValidExtended() && isValidExtended(i)); return ix==i;}    \
         ^
/home/adam/Desktop/osc/master-RelWithDebInfo-install/include/simbody/simmath/internal/ContactGeometry.h:44:1: note: candidate function (with reversed parameter order)
/home/adam/Desktop/osc/master-RelWithDebInfo-install/include/simbody/SimTKcommon/internal/common.h:427:5: note: expanded from macro 'SimTK_DEFINE_UNIQUE_INDEX_TYPE'
    SimTK_DEFINE_AND_EXPORT_UNIQUE_LOCAL_INDEX_TYPE(,,,NAME)   \
    ^
/home/adam/Desktop/osc/master-RelWithDebInfo-install/include/simbody/SimTKcommon/internal/common.h:459:10: note: expanded from macro 'SimTK_DEFINE_AND_EXPORT_UNIQUE_LOCAL_INDEX_TYPE'
    bool operator==(int  i) const {assert(isValidExtended() && isValidExtended(i)); return ix==i;}    \

The error seems to be because C++20 adds the spaceship operator (clang implementation coverage is here, search 'Consistent comparison').

Simbody (obviously) doesn't use this feature, but it seems to change the semantics of comparison operators such that they're now broken in Simbody. One quick fix that got rid of some of those errors was to add a comparison between the index type and itself, because (if I understand correctly) the index-type's current implementation relies on an implicit conversion to an int when comparing an index to its own type.

However, that doesn't fix all of the errors because some of them seem to be due to a change in the semantics of comparison. C++20 seems to require that comparison is reversible. That is, that a == b is equivalent to b == a at the source level, and an ambiguity can be generated if a == b is possible but b == a is not (which might, when combined in weird ways with implicit conversion, be part of the issue here).

Here's some related reading material:

  • Similar issue, different project: https://github.com/boostorg/date_time/issues/132
  • Overview of comparisons in C++20: https://brevzin.github.io/c++/2019/07/28/comparisons-cpp20/#reversing-primary-operators

I don't currently have time to fix this, PR it, deal with reviews, etc. but might have time in the future. I figured it's best to document the behavior now rather than the issue being undocumented and everyone seperately having to figure out what the issue is.

adamkewley avatar Dec 23 '20 10:12 adamkewley

Thanks, Adam!

sherm1 avatar Dec 23 '20 17:12 sherm1

@sherm1 @adamkewley I have attempted to solve this in pull request #748

EmilRosenquist avatar Nov 29 '22 15:11 EmilRosenquist

Thanks! I'll try it on some other platforms.

sherm1 avatar Nov 29 '22 17:11 sherm1

Closed by #748. @adamkewley please verify this fixed your problem and reopen if not.

sherm1 avatar Dec 02 '22 22:12 sherm1