aaron.ballman 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}}
----------------
Quuxplusone wrote:
> 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?
> But should Clang go _out of its way_ to suppress such "redundant" string
> literals?
I wasn't suggesting it should; I was suggesting that Clang should be
conservative and suppress printing the conditional when a message is present,
not when they look to be redundant enough.
> if a C++17-native program contains a string literal, we should maybe assume
> it's on purpose.
This is the exact scenario I was envisioning.
It's a relatively weak preference, but I kind of prefer not displaying the
conditional information in the presence of a message (at least in C++17 and
above), especially as the conditional can be huge. I'm thinking of scenarios
where the user does something like:
```
static_assert(condition1 && condition2 && (condition3 || condition4), "Simple
explanation");
```
except that `condition` is replaced by `std::some_type_trait<Blah>` in various
interesting ways.
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
