aboutsummaryrefslogtreecommitdiff
path: root/gcc/cp/semantics.c
diff options
context:
space:
mode:
authorMarek Polacek <polacek@redhat.com>2020-11-06 15:21:13 -0500
committerMarek Polacek <polacek@redhat.com>2020-11-10 15:08:06 -0500
commit8c0c83feb04d7486ccf9cbe86dcd5668f0a21ef9 (patch)
tree83991e65e0c54d69e82e832bffd8031f93483356 /gcc/cp/semantics.c
parenta210d404d08e363af4b2e2a60986c3cb08f8ebc5 (diff)
downloadgcc-8c0c83feb04d7486ccf9cbe86dcd5668f0a21ef9.zip
gcc-8c0c83feb04d7486ccf9cbe86dcd5668f0a21ef9.tar.gz
gcc-8c0c83feb04d7486ccf9cbe86dcd5668f0a21ef9.tar.bz2
c++: Improve static_assert diagnostic [PR97518]
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: error: static assertion failed note: 'is_same<int*, int>::value' evaluates to false 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 "'false' evaluates to 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. In passing, I've cleaned up some things: * use iloc_sentinel when appropriate, * it's nicer to call contextual_conv_bool instead of the rather verbose perform_implicit_conversion_flags, * no need to check for INTEGER_CST before calling integer_zerop. gcc/cp/ChangeLog: PR c++/97518 * cp-tree.h (finish_static_assert): Adjust declaration. * parser.c (cp_parser_static_assert): Pass false to finish_static_assert. * pt.c (tsubst_expr): Pass true to finish_static_assert. * semantics.c (find_failing_clause_r): New function. (find_failing_clause): New function. (finish_static_assert): Add a bool parameter. Use iloc_sentinel. Call contextual_conv_bool instead of perform_implicit_conversion_flags. Don't check for INTEGER_CST before calling integer_zerop. Call find_failing_clause and maybe use its location. Print the original condition or the failing clause if SHOW_EXPR_P. gcc/testsuite/ChangeLog: PR c++/97518 * g++.dg/diagnostic/pr87386.C: Adjust expected output. * g++.dg/diagnostic/static_assert1.C: New test. * g++.dg/diagnostic/static_assert2.C: New test. libcc1/ChangeLog: PR c++/97518 * libcp1plugin.cc (plugin_add_static_assert): Pass false to finish_static_assert.
Diffstat (limited to 'gcc/cp/semantics.c')
-rw-r--r--gcc/cp/semantics.c73
1 files changed, 61 insertions, 12 deletions
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 33d715e..df698d5 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -9827,13 +9827,53 @@ init_cp_semantics (void)
{
}
+
+/* If we have a condition in conjunctive normal form (CNF), find the first
+ failing clause. In other words, given an expression like
+
+ true && true && false && true && false
+
+ return the first 'false'. EXPR is the expression. */
+
+static tree
+find_failing_clause_r (tree expr)
+{
+ if (TREE_CODE (expr) == TRUTH_ANDIF_EXPR)
+ {
+ /* First check the left side... */
+ tree e = find_failing_clause_r (TREE_OPERAND (expr, 0));
+ if (e == NULL_TREE)
+ /* ...if we didn't find a false clause, check the right side. */
+ e = find_failing_clause_r (TREE_OPERAND (expr, 1));
+ return e;
+ }
+ tree e = contextual_conv_bool (expr, tf_none);
+ e = fold_non_dependent_expr (e, tf_none, /*manifestly_const_eval=*/true);
+ if (integer_zerop (e))
+ /* This is the failing clause. */
+ return expr;
+ return NULL_TREE;
+}
+
+/* Wrapper for find_failing_clause_r. */
+
+static tree
+find_failing_clause (tree expr)
+{
+ if (TREE_CODE (expr) != TRUTH_ANDIF_EXPR)
+ return NULL_TREE;
+ return find_failing_clause_r (expr);
+}
+
/* Build a STATIC_ASSERT for a static assertion with the condition
CONDITION and the message text MESSAGE. LOCATION is the location
of the static assertion in the source code. When MEMBER_P, this
- static assertion is a member of a class. */
+ static assertion is a member of a class. If SHOW_EXPR_P is true,
+ print the condition (because it was instantiation-dependent). */
+
void
finish_static_assert (tree condition, tree message, location_t location,
- bool member_p)
+ bool member_p, bool show_expr_p)
{
tsubst_flags_t complain = tf_warning_or_error;
@@ -9871,8 +9911,7 @@ finish_static_assert (tree condition, tree message, location_t location,
tree orig_condition = condition;
/* Fold the expression and convert it to a boolean value. */
- condition = perform_implicit_conversion_flags (boolean_type_node, condition,
- complain, LOOKUP_NORMAL);
+ condition = contextual_conv_bool (condition, complain);
condition = fold_non_dependent_expr (condition, complain,
/*manifestly_const_eval=*/true);
@@ -9881,21 +9920,32 @@ finish_static_assert (tree condition, tree message, location_t location,
;
else
{
- location_t saved_loc = input_location;
+ iloc_sentinel ils (location);
- input_location = location;
- if (TREE_CODE (condition) == INTEGER_CST
- && integer_zerop (condition))
+ if (integer_zerop (condition))
{
int sz = TREE_INT_CST_LOW (TYPE_SIZE_UNIT
(TREE_TYPE (TREE_TYPE (message))));
int len = TREE_STRING_LENGTH (message) / sz - 1;
+
+ /* See if we can find which clause was failing (for logical AND). */
+ tree bad = find_failing_clause (orig_condition);
+ /* If not, or its location is unusable, fall back to the previous
+ location. */
+ location_t cloc = location;
+ if (cp_expr_location (bad) != UNKNOWN_LOCATION)
+ cloc = cp_expr_location (bad);
+
/* Report the error. */
if (len == 0)
- error ("static assertion failed");
+ error_at (cloc, "static assertion failed");
else
- error ("static assertion failed: %s",
- TREE_STRING_POINTER (message));
+ error_at (cloc, "static assertion failed: %s",
+ TREE_STRING_POINTER (message));
+ if (show_expr_p)
+ inform (cloc, "%qE evaluates to false",
+ /* Nobody wants to see the artificial (bool) cast. */
+ (bad ? tree_strip_nop_conversions (bad) : orig_condition));
/* Actually explain the failure if this is a concept check or a
requires-expression. */
@@ -9909,7 +9959,6 @@ finish_static_assert (tree condition, tree message, location_t location,
if (require_rvalue_constant_expression (condition))
cxx_constant_value (condition);
}
- input_location = saved_loc;
}
}