Quuxplusone added inline comments.
================
Comment at: test/PCH/cxx-static_assert.cpp:17
-// expected-error@12 {{static_assert failed "N is not 2!"}}
+// expected-error@12 {{static_assert failed due to requirement '1 == 2' "N is
not 2!"}}
T<1> t1; // expected-note {{in instantiation of template class 'T<1>'
requested here}}
----------------
courbet wrote:
> aaron.ballman wrote:
> > I'm not certain how I feel about now printing the failure condition when
> > there's an explicit message provided. From what I understand, a fair amount
> > of code in the wild does `static_assert(some_condition, "some_condition")`
> > because of older language modes where the message was not optional. I worry
> > we're going to start seeing a lot of diagnostics like: `static_assert
> > failed due to requirement '1 == 2' "N == 2"`, which seems a bit
> > low-quality. See `DFAPacketizer::DFAPacketizer()` in LLVM as an example of
> > something similar.
> >
> > Given that the user entered a message, do we still want to show the
> > requirement? Do we feel the same way if the requirement is fairly large?
> The issue is that `"N == 2"` is a useless error message. Actually, since the
> error message has to be a string literal, there is no way for the user to
> print a debuggable output. So I really think we should print the failed
> condition.
FWIW, I also don't agree with Aaron's concern.
I do think there is a lot of code in the wild whose string literal was "phoned
in." After all, this is why C++17 allows us to omit the string literal: to
avoid boilerplate like this.
static_assert(some-condition, "some-condition");
static_assert(some-condition, "some-condition was not satisfied");
static_assert(some-condition, "some-condition must be satisfied");
static_assert(some-condition, "");
But should Clang go _out of its way_ to suppress such "redundant" string
literals? First of all, such suppression would be C++14-and-earlier: if a
C++17-native program contains a string literal, we should maybe assume it's on
purpose. Second, it's not clear how a machine could detect "redundant" literals
with high fidelity.
static_assert(std::is_integral<T>, "std::is_integral<T>");
// clearly redundant
static_assert(std::is_integral<T>, "T must be integral");
// redundant, but unlikely to be machine-detectable as such
static_assert((DFA_MAX_RESTERMS * DFA_MAX_RESOURCES) <= (8 *
sizeof(DFAInput)),
"(DFA_MAX_RESTERMS * DFA_MAX_RESOURCES) > (8 * sizeof(DFAInput))");
// redundant, but unlikely to be machine-detectable as such
// thanks to the substitution of > for <=
static_assert((DFA_MAX_RESTERMS * DFA_MAX_RESOURCES) <= (8 *
sizeof(DFAInput)),
"(DFA_MAX_RESTERMS * DFA_MAX_RESOURCES) too big for DFAInput");
// redundant, but unlikely to be machine-detectable as such
In any event, I agree with @courbet that Clang should print the expansion of
the failed condition in any case.
Also: One might argue that if the `static_assert` fits completely on one source
line, then we could omit the string-literal from our error message because the
string literal will be shown anyway as part of the offending source line — but
I believe IDE-users would complain about that, so, we shouldn't do that. And
even then, we'd still want to print the failed condition!
Checking my understanding: The `static_assert` above (taken from
`DFAPacketizer::DFAPacketizer()` in LLVM) would be unchanged by @courbet's
patches, because none of its subexpressions are template-dependent. Right?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55270/new/
https://reviews.llvm.org/D55270
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
