On 11/10/20 2:28 PM, Marek Polacek wrote:
On Tue, Nov 10, 2020 at 02:15:56PM -0500, Jason Merrill wrote:
On 11/10/20 1:59 PM, Marek Polacek wrote:
On Tue, Nov 10, 2020 at 11:32:04AM -0500, Jason Merrill wrote:
On 11/9/20 7:21 PM, Marek Polacek wrote:
Currently, when a static_assert fails, we only say "static assertion failed".
It would be more useful if we could also print the expression that
evaluated to false; this is especially useful when the condition uses
template parameters. Consider the motivating example, in which we have
this line:
static_assert(is_same<X, Y>::value);
if this fails, the user has to play dirty games to get the compiler to
print the template arguments. With this patch, we say:
static assertion failed due to requirement 'is_same<int*, int>::value'
I'd rather avoid the word "requirement" here, to avoid confusion with
Concepts.
Maybe have the usual failed error, and if we're showing the expression, have
a second inform to say e.g. "%qE evaluates to false"?
Works for me.
Also, if we've narrowed the problem down to a particular subexpression,
let's only print that one.
Done.
which I think is much better. However, always printing the condition that
evaluated to 'false' wouldn't be very useful: e.g. noexcept(fn) is
always parsed to true/false, so we would say "static assertion failed due
to requirement 'false'" which doesn't help. So I wound up only printing
the condition when it was instantiation-dependent, that is, we called
finish_static_assert from tsubst_expr.
Moreover, this patch also improves the diagnostic when the condition
consists of a logical AND. Say you have something like this:
static_assert(fn1() && fn2() && fn3() && fn4() && fn5());
where fn4() evaluates to false and the other ones to true. Highlighting
the whole thing is not that helpful because it won't say which clause
evaluated to false. With the find_failing_clause tweak in this patch
we emit:
error: static assertion failed
6 | static_assert(fn1() && fn2() && fn3() && fn4() && fn5());
| ~~~^~
so you know right away what's going on. Unfortunately, when you combine
both things, that is, have an instantiation-dependent expr and && in
a static_assert, we can't yet quite point to the clause that failed. It
is because when we tsubstitute something like is_same<X, Y>::value, we
generate a VAR_DECL that doesn't have any location. It would be awesome
if we could wrap it with a location wrapper, but I didn't see anything
obvious.
Hmm, I vaguely remember that at first we were using location wrappers less
in templates, but I thought that was fixed. I don't see anything setting
suppress_location_wrappers, and it looks like tsubst_copy_and_build should
preserve a location wrapper.
The problem here is that tsubst_qualified_id produces a VAR_DECL and for those
CAN_HAVE_LOCATION_P is false.
Ah, then perhaps tsubst_qualified_id should call maybe_wrap_with_location to
preserve the location of the SCOPE_REF.
The SCOPE_REF's location is fine, we just throw it away and return a VAR_DECL.
Yes, I'm saying that tsubst_qualified_id should extract the location
from the SCOPE_REF and pass it to maybe_wrap_with_location to give the
VAR_DECL a location wrapper.
Jason
