boost::mpl::integral_c broken in Clang trunk

[jyknight]

Clang has recently addressed an issue where it failed to diagnose the UB of casting an out-of-range integer to an enum in a constant expression.

Since undefined behavior is not allowed in a constant expression, Clang now prohibits enum test { zero }; constexpr int g = static_cast<test>(-1); with "error: constexpr variable 'g' must be initialized by a constant expression; note: integer value -1 is outside the valid range of values [0, 1] for this enumeration type"

This change breaks boost::mpl::integral_c, since it does exactly that. Demonstrated with example, below (and on godbolt)

#include <boost/mpl/integral_c.hpp>
enum test { zero, one };
boost::mpl::integral_c<test, zero> m;

Originally discovered with a program that was using the boost::numeric library, which in turn uses mpl like the above example.

#include <boost/numeric/conversion/udt_builtin_mixture.hpp>
boost::numeric::udt_builtin_mixture<int, int> m;

[dabrahams]

The fix looks like this.

[dabrahams]

Actually I'm back to believing there's no true fix if next and prior have to be of type integral_c<T, ...>.

[jyknight]

Perhaps the correct fix is to somehow cause the next/prior members to be omitted when they're impossible to represent, just like they're omitted for bool? I expect that's possible somehow via some template specialization magic.

Note that Clang has been changed since a week ago to no longer have this be an unconditional error. Now it's a controllable warning -Wenum-constexpr-conversion, which by default is enabled as an error. However, it's suppressed in system headers, and can be downgraded/disabled in user code as well. So, to see the error in the godbolt example you'll now need to add -Wsystem-headers to the compiler options. (e.g. https://godbolt.org/z/v4PPjW1n1)

Since most code using boost will be using it as a system header, they'll also not see the error, and anyone else may downgrade the error to a warning or disable it entirely if desired. However, it may become an unconditional error again in some future release of Clang, so it'd still be good to address it.

[dabrahams]

Perhaps the correct fix is to somehow cause the next/prior members to be omitted when they're impossible to represent, just like they're omitted for bool? I expect that's possible somehow via some template specialization magic.

Well, that's what I investigated, and my (ill-informed) conclusion is that it's not possible. The problem is that the the only expressions that try to create the representation (static_cast<test>(-1)) are not constant expressions, and they're well-typed. I hope somebody proves me wrong, but I'm certainly out of ideas.

[shafik]

If the intent is that the enum should be able to handle values outside the enumerator values then perhaps making it have a fixed underlying type better represents the intent?

[dabrahams]

I don't think that's the intent, but it is a decent workaround for the problem. I don't believe the intent is actually realizable, but like I said, I no longer really know C++, so I might be wrong.

[ecatmur]

the static const trick works: https://godbolt.org/z/KrhdzqYbx

enum test { zero, one };
template<class E, E a>
E const k = static_cast<E>(a - 1);
template<class T, T> struct S {};
template<class> int f();
int g() { return f<S<test, k<test, zero>>>(); }

this means that going all the way back to the EDG implementation/workaround actually "fixes" it: https://godbolt.org/z/8bTerz3WM

#define __EDG_VERSION__ 242
#include <boost/mpl/integral_c.hpp>
enum test { zero, one };
boost::mpl::integral_c<test, zero> m;

[smeenai]

Was there any progress here? Clang landed https://github.com/llvm/llvm-project/pull/67528 which makes this an error by default even in system headers. We can turn off the warning for now, but that option will be removed in a future Clang release, so this will need to be addressed before then.

[carlosgalvezp]

Hi! Any progress? What is the ETA for fixing? Next Clang release in about 6 months will turn the warning into a non-suppressable hard error.

[kencu]

we are running into this issue now on MacPorts. current boost builds are failing with the now-default compiler, clang-16.

various patches are being suggested, versions of the ones listed in this PR, but if at all possible, we'd like to go with the one that will be finally integrated into boost.

[EduPonz]

We have hit the same issue (see here. It seems that macports/macports-ports#21839 does not fix it for us.

[rpopescu]

Does anyone know what the latest status is, for example, which is the earliest version of boost where this is fixed?

tagging @pdimov with thanks!

[pdimov]

It will be fixed in the next release, 1.86.

[rpopescu]

Thank you!

[Artalus]

We are currently unable to update our Boost to 1.86 - but can patch and rebuild the version we currently use, and would like to backport the needed changes locally, to build our 1.75 with newer Clang. I checked the history of this issue, and it seems that the combination of

• this change in numeric: https://github.com/boostorg/numeric_conversion/commit/50a1eae942effb0a9b90724323ef8f2a67e7984a

• and this change in MPL: https://github.com/boostorg/mpl/pull/77

does fix the build error for us. Since we are using Conan to provide third-party libraries, I also thought it might be a good idea to share such patch with a bigger audience. @pdimov apologies, but could you maybe confirm if this patch is indeed sufficient for a fix, and does not miss anything not mentioned in this thread?

[pdimov]

I think that these two patches are sufficient, yes.