diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c index dddb1e26202..f192da37b41 100644 --- a/gdb/unittests/enum-flags-selftests.c +++ b/gdb/unittests/enum-flags-selftests.c @@ -235,33 +235,6 @@ CHECK_VALID (true, UnsignedEnumFlag, ~UnsignedEnumFlag ()) CHECK_VALID (true, EnumFlag, true ? EnumFlag () : RawEnum ()) CHECK_VALID (true, EnumFlag, true ? RawEnum () : EnumFlag ()) -/* These are valid, but it's not a big deal since you won't be able to - assign the resulting integer to an enum or an enum_flags without a - cast. - - The latter two tests are disabled on older GCCs because they - incorrectly fail with gcc 4.8 and 4.9 at least. Running the test - outside a SFINAE context shows: - - invalid user-defined conversion from ‘EF’ to ‘RE2’ - - They've been confirmed to compile/pass with gcc 5.3, gcc 7.1 and - clang 3.7. */ - -CHECK_VALID (true, int, true ? EnumFlag () : EnumFlag2 ()) -CHECK_VALID (true, int, true ? EnumFlag2 () : EnumFlag ()) -CHECK_VALID (true, int, true ? EnumFlag () : RawEnum2 ()) -CHECK_VALID (true, int, true ? RawEnum2 () : EnumFlag ()) - -/* Same, but with an unsigned enum. */ - -using uns = unsigned int; - -CHECK_VALID (true, uns, true ? EnumFlag () : UnsignedEnumFlag ()) -CHECK_VALID (true, uns, true ? UnsignedEnumFlag () : EnumFlag ()) -CHECK_VALID (true, uns, true ? EnumFlag () : UnsignedRawEnum ()) -CHECK_VALID (true, uns, true ? UnsignedRawEnum () : EnumFlag ()) - /* Unfortunately this can't work due to the way C++ computes the return type of the ternary conditional operator. int isn't implicitly convertible to the raw enum type, so the type of the diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h index d3291e95d7e..71109bb8c99 100644 --- a/gdbsupport/enum-flags.h +++ b/gdbsupport/enum-flags.h @@ -73,30 +73,6 @@ namespace. The compiler finds the corresponding is_enum_flags_enum_type function via ADL. */ -/* Note that std::underlying_type is not what we want here, - since that returns unsigned int even when the enum decays to signed - int. */ -template class integer_for_size { using type = void; }; -template<> struct integer_for_size<1, 0> { using type = uint8_t; }; -template<> struct integer_for_size<2, 0> { using type = uint16_t; }; -template<> struct integer_for_size<4, 0> { using type = uint32_t; }; -template<> struct integer_for_size<8, 0> { using type = uint64_t; }; -template<> struct integer_for_size<1, 1> { using type = int8_t; }; -template<> struct integer_for_size<2, 1> { using type = int16_t; }; -template<> struct integer_for_size<4, 1> { using type = int32_t; }; -template<> struct integer_for_size<8, 1> { using type = int64_t; }; - -template -struct enum_underlying_type -{ - DIAGNOSTIC_PUSH - DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION - using type - = typename integer_for_size(T (-1) < T (0))>::type; - DIAGNOSTIC_POP -}; - namespace enum_flags_detail { @@ -117,10 +93,61 @@ struct zero_type; /* gdb::Requires trait helpers. */ template using EnumIsUnsigned - = std::is_unsigned::type>; + = std::is_unsigned::type>; + +/* Helper to detect whether an enum has a fixed underlying type. This can be + achieved by using a scoped enum (in which case the type is "int") or + an explicit underlying type. C-style enums that are unscoped or do not + have an explicit underlying type have an implementation-defined underlying + type. + + https://timsong-cpp.github.io/cppwp/n4659/dcl.enum#5 + + We need this trait in order to ensure that operator~ below does NOT + operate on old-style enums. This is because we apply operator~ on + the value and then cast the result to the enum_type. This is however + Undefined Behavior if the result does not fit in the range of possible + values for the enum. For enums with fixed underlying type, the entire + range of the integer is available. However, for old-style enums, the range + is only the smallest bit-field that can hold all the values of the + enumeration, typically much smaller than the underlying integer: + + https://timsong-cpp.github.io/cppwp/n4659/expr.static.cast#10 + https://timsong-cpp.github.io/cppwp/n4659/dcl.enum#8 + + To implement this, we leverage the fact that, since C++17, enums with + fixed underlying type can be list-initialized from an integer: + https://timsong-cpp.github.io/cppwp/n4659/dcl.init.list#3.7 + + Old-style enums cannot be initialized like that, leading to ill-formed + code. + + We then use this together with SFINAE to create the desired trait. + +*/ +template +struct EnumHasFixedUnderlyingType : std::false_type +{ + static_assert(std::is_enum::value); +}; + +/* Specialization that is active only if enum_type can be + list-initialized from an integer (0). Only enums with fixed + underlying type satisfy this property in C++17. */ template -using EnumIsSigned - = std::is_signed::type>; +struct EnumHasFixedUnderlyingType> : std::true_type +{ + static_assert(std::is_enum::value); +}; + +template +using EnumIsSafeForBitwiseComplement = std::conjunction< + EnumIsUnsigned, + EnumHasFixedUnderlyingType +>; + +template +using EnumIsUnsafeForBitwiseComplement = std::negation>; } @@ -129,7 +156,7 @@ class enum_flags { public: using enum_type = E; - using underlying_type = typename enum_underlying_type::type; + using underlying_type = typename std::underlying_type::type; /* For to_string. Maps one enumerator of E to a string. */ struct string_mapping @@ -392,33 +419,41 @@ ENUM_FLAGS_GEN_COMP (operator!=, !=) template , typename - = gdb::Requires>> + = gdb::Requires>> constexpr enum_type operator~ (enum_type e) { using underlying = typename enum_flags::underlying_type; - return (enum_type) ~underlying (e); + /* Cast to ULONGEST first, to prevent integer promotions from enums + with fixed underlying type std::uint8_t or std::uint16_t to + signed int. This ensures we apply the bitwise complement on an + unsigned type. */ + return (enum_type)(underlying) ~ULONGEST (e); } template , - typename = gdb::Requires>> + typename = gdb::Requires>> constexpr void operator~ (enum_type e) = delete; template , typename - = gdb::Requires>> + = gdb::Requires>> constexpr enum_flags operator~ (enum_flags e) { using underlying = typename enum_flags::underlying_type; - return (enum_type) ~underlying (e); + /* Cast to ULONGEST first, to prevent integer promotions from enums + with fixed underlying type std::uint8_t or std::uint16_t to + signed int. This ensures we apply the bitwise complement on an + unsigned type. */ + return (enum_type)(underlying) ~ULONGEST (e); } template , - typename = gdb::Requires>> + typename = gdb::Requires>> constexpr void operator~ (enum_flags e) = delete; /* Delete operator<< and operator>>. */ diff --git a/include/diagnostics.h b/include/diagnostics.h index 97e30ab807f..2a1d9b92fad 100644 --- a/include/diagnostics.h +++ b/include/diagnostics.h @@ -76,11 +76,6 @@ # define DIAGNOSTIC_ERROR_SWITCH \ DIAGNOSTIC_ERROR ("-Wswitch") -# if __has_warning ("-Wenum-constexpr-conversion") -# define DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION \ - DIAGNOSTIC_IGNORE ("-Wenum-constexpr-conversion") -# endif - #elif defined (__GNUC__) /* GCC */ # define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS \ @@ -159,8 +154,4 @@ # define DIAGNOSTIC_ERROR_SWITCH #endif -#ifndef DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION -# define DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION -#endif - #endif /* DIAGNOSTICS_H */