aaron.ballman added a subscriber: cjdb.
aaron.ballman 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@-1 {{integer value 8 is outside the valid range of values
[-8, 8) for this enumeration type}}
+
----------------
tahonermann wrote:
> 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.
I'm fine with being more verbose in the diagnostic so long as it doesn't go
overboard. I don't really like the wording Erich suggested because it can be
misinterpreted as both values being inclusive. I can hold my nose at what we
have above. We're inconsistent in how we report this kind of information and it
seems like someday we should improve this whole class of diagnostics (ones with
ranges) to have a consistent display to the user. (CC @cjdb for awareness for
his project, nothing actionable though.)
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
