erichkeane accepted this revision. erichkeane added a comment. Still want a test for E3, a release note, and updating the cxx_status.html. But those don't need review IMO.
================ 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}} + ---------------- 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. 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