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

Reply via email to