Repository navigation
Conversation
|
Once this one is merged, I'll tag a release so that we can add vcpkg and Conan support |
|
Failures are unrelated to the changes |
eisenwave
left a comment
There was a problem hiding this comment.
This design is clever, but it's not yet complete.
We still have compiler identification that's not covered here. Isn't it possible to install something built with GCC and to then get a different outcome when building that installation with Clang?
Another problem is that lots of our feature detection bypasses the standard feature-test macros entirely by using __has_builtin instead. So things like #if BEMAN_BIG_INT_HAS_BUILTIN(__builtin_add_overflow) presumably violate the Beman standard by doing flag forking as well; these are just compiler version detections under the hood. I don't actually mind merging this now and incrementally fixing the rest of the flag forking though.
Theoretically as no version of Clang supports say
I can pull all of those into config before merge. |
|
It's probably best to rip off the bandaid once and fix all the flag forking, including the I assume that's pretty straight forward. The cleanest solution may be to convert code like #if BEMAN_BIG_INT_HAS_BUILTIN(__builtin_add_overflow)
__builtin_add_overflow(x, y)
#endifto #ifdef BEMAN_BIG_INT_BUILTIN_ADD_OVERFLOW
BEMAN_BIG_INT_BUILTIN_ADD_OVERFLOW(x, y)
#endifwhere #if BEMAN_BIG_INT_HAS_BUILTIN(__builtin_add_overflow)
#define BEMAN_BIG_INT_BUILTIN_ADD_OVERFLOW(...) __builtin_add_overflow(__VA_ARGS__)
#endif |
As requested by @ednolan