[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

Wed Aug 3 15:22:45 PDT 2022

smeenai added a comment.

Sorry for chiming in a bit late here.

I'm working on getting a large internal codebase to compile after this change. Most of the issues are easy enough to deal with, but there's one Boost error I'd like to highlight: P8289 <https://reviews.llvm.org/P8289>. (We might be using a slightly older Boost version, but upgrading it would be a pretty significant undertaking, unfortunately, and current Boost appears to have similar code: https://github.com/boostorg/mpl/blob/8f10d06b9637a67b23a3a11791de5bd6bcd2e112/include/boost/mpl/aux_/integral_wrapper.hpp#L72-L73.)

There's a couple of things that make this error hard to parse:

- The note about the integer value being out of range for the enumeration doesn't appear until towards the end of the error (line 55 in my paste), even though it's the key to understanding why the argument isn't considered a constant expression
- The note doesn't mention //which// enumeration type the value is out of the valid range for, and with the mix of macros and templates going on in this error message, that's non-trivial to decipher

After digging through the preprocessor output, it turns out that `AUX_WRAPPER_VALUE_TYPE` is defined as `T` here (after being undefined and redefined a bunch of times, which makes this even trickier), and coupled with the types in the templates in the error you can determine that `T` is boost::numeric::udt_builtin_mixture_enum <https://github.com/boostorg/numeric_conversion/blob/db44689f4f4f74d6572a868e13f523c82fca5a55/include/boost/numeric/conversion/udt_builtin_mixture_enum.hpp#L15>, but it'd be much easier if the diagnostic gave you the type directly and also appeared much earlier.

Diagnostic quality aside, I'm not really sure what to do about this error. It appears to ultimately originate from here <https://github.com/boostorg/numeric_conversion/blob/db44689f4f4f74d6572a868e13f523c82fca5a55/include/boost/numeric/conversion/detail/udt_builtin_mixture.hpp#L23>, which will attempt to compute a `prior` for the first enum value, which is incorrect. I can fix the issue by giving `udt_builtin_mixture_enum` and int_float_mixture_enum <https://github.com/boostorg/numeric_conversion/blob/db44689f4f4f74d6572a868e13f523c82fca5a55/include/boost/numeric/conversion/int_float_mixture_enum.hpp#L15> an underlying type of `int`, but I lack the Boost know-how to understand if that's reasonable.

Given that we have a non-obvious (at least to me) issue in a widely used third-party library, would we consider giving users some way to opt out of this error, at least as a transition tool?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130058/new/

https://reviews.llvm.org/D130058