erichkeane added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:370 +def note_constexpr_unscoped_enum_out_of_range : Note< + "store of value outside of the range of unscoped enum, valid values %0 to %1">; def err_experimental_clang_interp_failed : Error< ---------------- This is a bit awkwardly phrased to me... though I don't have a better idea. Perhaps take another run, or get Aaron to look at it? ================ Comment at: clang/lib/AST/ExprConstant.cpp:13512 + if (Info.Ctx.getLangOpts().CPlusPlus20) + if (const EnumType *ET = dyn_cast<EnumType>(DestType)) { ---------------- Why the check for C++20? This is UB as a Defect Report, which applies to all versions of C++. ================ Comment at: clang/lib/AST/ExprConstant.cpp:13533 + + if (NumNegativeBits) { + unsigned NumBits = std::max(NumNegativeBits, NumPositiveBits + 1); ---------------- So my reading of 'within the range of the enumeration values' is different here. Given: `enum F { -4, 4};`, the valid values for assignment I would say are -4, -3, -2, -1, 0, 1, 2, 3, 4. This error seems to be a bit more permissive here by making sure it is represent-able by the number of bits required to represent the enum. ================ Comment at: clang/test/SemaCXX/constant-expression-cxx2a.cpp:1477 + +enum E { e1=-4, e2=4}; +void testValueInRangeOfEnumerationValues() { ---------------- Theres likely quite a few more tests that need to be done. We should have one where it validates against a fixed underlying type, AND probably a pair of ones to check against scoped enums. I wouldn't expect a scoped enum to diagnose, but a test to make sure we're checking the right thing is likely valuable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130058/new/ https://reviews.llvm.org/D130058 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits