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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.


================
Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2420
+  constexpr E1 x2 = static_cast<E1>(8); // expected-error {{must be initialized by a constant expression}}
+  // expected-note at -1 {{integer value 8 is outside the valid range of values [-8, 8) for this enumeration type}}
+
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > erichkeane wrote:
> > > > > Are we ok with how subtle the `[N, M)` syntax is here?
> > > > FWIW, I pulled this from diagnostics like: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticSemaKinds.td#L9904 and https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticSemaKinds.td#L11541
> > > Those aren't particularly high quality diagnostics, the first is for builtin ranges (and builtins have notoriously bad diagnostics), the 2nd is for the matrix type, which is only slightly better.
> > > 
> > > That said, if you are ok with it, I'm ok, just somewhat afraid it'll be a touch confusing.
> > Yeah, it's not the best diagnostic, to be sure. The trouble is that spelling it out makes it worse IMO: `integer value %0 is outside the valid range of values %1 (inclusive) and %2 (exclusive) for this enumeration type`
> Ok then, I can't think of anything better really (PERHAPS something that says, `integer value %0 is outside of the valid range of values (%1 - %2 inclusive) for this enumeration type`, so I'm ok living with it until someone proposes better in a followup patch.
> 
> 
I've never cared for the `[` vs `(` notation to indicate inclusivity vs exclusivity. All I see are unbalanced tokens and I can never remember which brace means what; I have to look it up every time and it isn't an easy search, especially for people that aren't already somewhat familiar with the notation; you have to know to search for something like "range inclusive exclusive notation". I urge use of the more elaborate diagnostic.


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

https://reviews.llvm.org/D130058