We would like to do constexpr evaluation to avoid false positives on
warnings, but constexpr evaluation can involve function body copying
that changes DECL_UID, which breaks -fcompare-debug. In the PR Jakub
suggested that one possibility would be to avoid operations that might
affect DECL_UID; this patch implements that suggestion.
The other three patches are to avoid regressions from this change.
We recently started unsharing values we find in cv_cache, but we
shouldn't do that if it's the same as the argument. No longer unsharing
regresses the diagnostic locations in decomp48.C. And the digest_init
patch avoids a regression on desig14.C.
All tested x86_64-pc-linux-gnu, applying to trunk.
>From 00c458ede4df6ff37d709b8a8d3135e2aa4c9d14 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Sun, 26 Jan 2020 22:58:32 -0500
Subject: [PATCH 4/4] c++: Use constexpr to avoid wrong -Wsign-compare
(PR90691).
To: gcc-patches@gcc.gnu.org
We would like to do constexpr evaluation to avoid false positives on
warnings, but constexpr evaluation can involve function body copying that
changes DECL_UID, which breaks -fcompare-debug. So let's remember
that we need to avoid that.
PR c++/90691
* expr.c (fold_for_warn): Call maybe_constant_value.
* constexpr.c (struct constexpr_ctx): Add uid_sensitive field.
(maybe_constant_value): Add uid_sensitive parm.
(get_fundef_copy): Don't copy if it's true.
(cxx_eval_call_expression): Don't instantiate if it's true.
(cxx_eval_outermost_constant_expr): Likewise.
---
gcc/cp/cp-tree.h | 2 +-
gcc/cp/constexpr.c | 42 +++++++++++++++------
gcc/cp/expr.c | 2 +
gcc/testsuite/g++.dg/warn/Walways-true-3.C | 4 +-
gcc/testsuite/g++.dg/warn/Wsign-compare-9.C | 22 +++++++++++
5 files changed, 58 insertions(+), 14 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wsign-compare-9.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c46d1e92b3b..037d3b64538 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7930,7 +7930,7 @@ extern bool require_potential_rvalue_constant_expression (tree);
extern tree cxx_constant_value (tree, tree = NULL_TREE);
extern void cxx_constant_dtor (tree, tree);
extern tree cxx_constant_init (tree, tree = NULL_TREE);
-extern tree maybe_constant_value (tree, tree = NULL_TREE, bool = false);
+extern tree maybe_constant_value (tree, tree = NULL_TREE, bool = false, bool = false);
extern tree maybe_constant_init (tree, tree = NULL_TREE, bool = false);
extern tree fold_non_dependent_expr (tree,
tsubst_flags_t = tf_warning_or_error,
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 152142f0b31..03ab9ae2045 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1074,13 +1074,18 @@ struct constexpr_ctx {
/* If inside SWITCH_EXPR. */
constexpr_switch_state *css_state;
- /* Whether we should error on a non-constant expression or fail quietly. */
+ /* Whether we should error on a non-constant expression or fail quietly.
+ This flag needs to be here, but some of the others could move to global
+ if they get larger than a word. */
bool quiet;
/* Whether we are strictly conforming to constant expression rules or
trying harder to get a constant value. */
bool strict;
/* Whether __builtin_is_constant_evaluated () should be true. */
bool manifestly_const_eval;
+ /* Whether we want to avoid doing anything that will cause extra DECL_UID
+ generation. */
+ bool uid_sensitive;
};
/* A table of all constexpr calls that have been evaluated by the
@@ -1145,7 +1150,7 @@ static GTY(()) hash_map<tree, tree> *fundef_copies_table;
is parms, TYPE is result. */
static tree
-get_fundef_copy (constexpr_fundef *fundef)
+get_fundef_copy (const constexpr_ctx *ctx, constexpr_fundef *fundef)
{
tree copy;
bool existed;
@@ -1162,6 +1167,9 @@ get_fundef_copy (constexpr_fundef *fundef)
}
else if (*slot == NULL_TREE)
{
+ if (ctx->uid_sensitive)
+ return NULL_TREE;
+
/* We've already used the function itself, so make a copy. */
copy = build_tree_list (NULL, NULL);
tree saved_body = DECL_SAVED_TREE (fundef->decl);
@@ -2232,6 +2240,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
/* We can't defer instantiating the function any longer. */
if (!DECL_INITIAL (fun)
+ && !ctx->uid_sensitive
&& DECL_TEMPLOID_INSTANTIATION (fun))
{
location_t save_loc = input_location;
@@ -2378,13 +2387,12 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
gcc_assert (at_eof >= 2 && ctx->quiet);
*non_constant_p = true;
}
- else
+ else if (tree copy = get_fundef_copy (ctx, new_call.fundef))
{
tree body, parms, res;
releasing_vec ctors;
/* Reuse or create a new unshared copy of this function's body. */
- tree copy = get_fundef_copy (new_call.fundef);
body = TREE_PURPOSE (copy);
parms = TREE_VALUE (copy);
res = TREE_TYPE (copy);
@@ -2522,6 +2530,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
}
}
}
+ else
+ /* Couldn't get a function copy to evaluate. */
+ *non_constant_p = true;
if (result == error_mark_node)
*non_constant_p = true;
@@ -6250,7 +6261,8 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
bool strict = true,
bool manifestly_const_eval = false,
bool constexpr_dtor = false,
- tree object = NULL_TREE)
+ tree object = NULL_TREE,
+ bool uid_sensitive = false)
{
auto_timevar time (TV_CONSTEXPR);
@@ -6260,7 +6272,8 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
constexpr_global_ctx global_ctx;
constexpr_ctx ctx = { &global_ctx, NULL, NULL, NULL, NULL, NULL,
allow_non_constant, strict,
- manifestly_const_eval || !allow_non_constant };
+ manifestly_const_eval || !allow_non_constant,
+ uid_sensitive };
tree type = initialized_type (t);
tree r = t;
@@ -6350,7 +6363,8 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
auto_vec<tree, 16> cleanups;
global_ctx.cleanups = &cleanups;
- instantiate_constexpr_fns (r);
+ if (!uid_sensitive)
+ instantiate_constexpr_fns (r);
r = cxx_eval_constant_expression (&ctx, r,
false, &non_constant_p, &overflow_p);
@@ -6598,12 +6612,14 @@ fold_simple (tree t)
Otherwise, if T does not have TREE_CONSTANT set, returns T.
Otherwise, returns a version of T without TREE_CONSTANT.
MANIFESTLY_CONST_EVAL is true if T is manifestly const-evaluated
- as per P0595. */
+ as per P0595. UID_SENSITIVE is true if we can't do anything that
+ would affect DECL_UID ordering. */
static GTY((deletable)) hash_map<tree, tree> *cv_cache;
tree
-maybe_constant_value (tree t, tree decl, bool manifestly_const_eval)
+maybe_constant_value (tree t, tree decl, bool manifestly_const_eval,
+ bool uid_sensitive)
{
tree r;
@@ -6621,7 +6637,8 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval)
return t;
if (manifestly_const_eval)
- return cxx_eval_outermost_constant_expr (t, true, true, true, false, decl);
+ return cxx_eval_outermost_constant_expr (t, true, true, true, false,
+ decl, uid_sensitive);
if (cv_cache == NULL)
cv_cache = hash_map<tree, tree>::create_ggc (101);
@@ -6633,7 +6650,10 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval)
r = unshare_expr_without_location (r);
protected_set_expr_location (r, EXPR_LOCATION (t));
}
- return r;
+ if (r != t || TREE_CONSTANT (t) || !manifestly_const_eval)
+ return r;
+ /* If we cached this as non-constant and we need a constant value, try
+ again; we might have failed before due to UID_SENSITIVE. */
}
r = cxx_eval_outermost_constant_expr (t, true, true, false, false, decl);
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index cfc1c7b64ca..04e4418c671 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -397,6 +397,8 @@ fold_for_warn (tree x)
else
return f;
}
+ else if (cxx_dialect >= cxx11)
+ x = maybe_constant_value (x, NULL_TREE, false, true);
return c_fully_fold (x, /*for_init*/false, /*maybe_constp*/NULL);
}
diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-3.C b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
index 0291328d415..dd02843857b 100644
--- a/gcc/testsuite/g++.dg/warn/Walways-true-3.C
+++ b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
@@ -17,7 +17,7 @@ bar (int &a)
if (&b) // { dg-warning "7:the compiler can assume that the address of" }
foo ();
- if (!&c) // { dg-warning "8:the compiler can assume that the address of" }
+ if (!&c) // { dg-warning "8:the address of" }
foo ();
if (!&(int &)(int &)a) // { dg-warning "8:the compiler can assume that the address of" }
@@ -29,7 +29,7 @@ bar (int &a)
if (&b != 0) // { dg-warning "10:the compiler can assume that the address of" }
foo ();
- if (0 == &(int &)(int &)c) // { dg-warning "9:the compiler can assume that the address of" }
+ if (0 == &(int &)(int &)c) // { dg-warning "9:the address of" }
foo ();
if (&a != (int *)0) // { dg-warning "10:the compiler can assume that the address of" }
diff --git a/gcc/testsuite/g++.dg/warn/Wsign-compare-9.C b/gcc/testsuite/g++.dg/warn/Wsign-compare-9.C
new file mode 100644
index 00000000000..bf3514c40ac
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wsign-compare-9.C
@@ -0,0 +1,22 @@
+// PR c++/90691
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Wsign-compare }
+
+struct S {
+ int a;
+
+ constexpr S();
+ explicit constexpr S(int a_) : a(a_) {}
+};
+
+constexpr S b = S(12);
+
+template <const S& e>
+bool c(unsigned int d) {
+ return d >= e.a;
+}
+
+bool test(unsigned int d);
+bool test(unsigned int d) {
+ return c<b>(d);
+}
--
2.18.1
>From 12b13c2f8bbc7f65bcb573ef6df18d66a08d37c6 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Fri, 7 Feb 2020 16:28:20 -0500
Subject: [PATCH 3/4] c++: Preserve location in maybe_constant_value.
To: gcc-patches@gcc.gnu.org
If cxx_eval_outermost_constant_expr doesn't change the argument, we really
shouldn't unshare it when we try to fold it again.
PR c++/92852
* constexpr.c (maybe_constant_value): Don't unshare if the cached
value is the same as the argument.
---
gcc/cp/constexpr.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 8a02c6e0713..152142f0b31 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -6626,7 +6626,15 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval)
if (cv_cache == NULL)
cv_cache = hash_map<tree, tree>::create_ggc (101);
if (tree *cached = cv_cache->get (t))
- return unshare_expr_without_location (*cached);
+ {
+ r = *cached;
+ if (r != t)
+ {
+ r = unshare_expr_without_location (r);
+ protected_set_expr_location (r, EXPR_LOCATION (t));
+ }
+ return r;
+ }
r = cxx_eval_outermost_constant_expr (t, true, true, false, false, decl);
gcc_checking_assert (r == t
--
2.18.1
>From c41d03823cb3746c533e612b0232f68b1537817f Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Fri, 7 Feb 2020 17:14:11 -0500
Subject: [PATCH 2/4] c++: Fix -Wreturn-local-addr location.
To: gcc-patches@gcc.gnu.org
* typeck.c (maybe_warn_about_returning_address_of_local): Add
location parameter.
---
gcc/cp/typeck.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 5964c34272e..c0c98dad980 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -58,7 +58,7 @@ static tree pointer_diff (location_t, tree, tree, tree, tsubst_flags_t, tree *);
static tree get_delta_difference (tree, tree, bool, bool, tsubst_flags_t);
static void casts_away_constness_r (tree *, tree *, tsubst_flags_t);
static bool casts_away_constness (tree, tree, tsubst_flags_t);
-static bool maybe_warn_about_returning_address_of_local (tree);
+static bool maybe_warn_about_returning_address_of_local (tree, location_t = UNKNOWN_LOCATION);
static void error_args_num (location_t, tree, bool);
static int convert_arguments (tree, vec<tree, va_gc> **, tree, int,
tsubst_flags_t);
@@ -9466,11 +9466,12 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
temporary give an appropriate warning and return true. */
static bool
-maybe_warn_about_returning_address_of_local (tree retval)
+maybe_warn_about_returning_address_of_local (tree retval, location_t loc)
{
tree valtype = TREE_TYPE (DECL_RESULT (current_function_decl));
tree whats_returned = fold_for_warn (retval);
- location_t loc = cp_expr_loc_or_input_loc (retval);
+ if (!loc)
+ loc = cp_expr_loc_or_input_loc (retval);
for (;;)
{
@@ -9504,7 +9505,7 @@ maybe_warn_about_returning_address_of_local (tree retval)
|| is_std_forward_p (whats_returned)))
{
tree arg = CALL_EXPR_ARG (whats_returned, 0);
- return maybe_warn_about_returning_address_of_local (arg);
+ return maybe_warn_about_returning_address_of_local (arg, loc);
}
if (TREE_CODE (whats_returned) != ADDR_EXPR)
@@ -9550,7 +9551,7 @@ maybe_warn_about_returning_address_of_local (tree retval)
if (TYPE_REF_P (TREE_TYPE (base)))
{
if (tree init = DECL_INITIAL (base))
- return maybe_warn_about_returning_address_of_local (init);
+ return maybe_warn_about_returning_address_of_local (init, loc);
else
return false;
}
@@ -10077,7 +10078,7 @@ check_return_expr (tree retval, bool *no_warning)
retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
TREE_OPERAND (retval, 0));
else if (!processing_template_decl
- && maybe_warn_about_returning_address_of_local (retval)
+ && maybe_warn_about_returning_address_of_local (retval, loc)
&& INDIRECT_TYPE_P (valtype))
retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
build_zero_cst (TREE_TYPE (retval)));
--
2.18.1
>From 77840fd9056d8cc9dd5913ac5b9b366da3cf1008 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Fri, 7 Feb 2020 16:10:18 -0500
Subject: [PATCH 1/4] c++: Fix TREE_SIDE_EFFECTS after digest_init.
To: gcc-patches@gcc.gnu.org
* typeck2.c (process_init_constructor): Also clear TREE_SIDE_EFFECTS
if appropriate.
---
gcc/cp/typeck2.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 371b203c29b..48920894b3b 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1929,11 +1929,15 @@ process_init_constructor (tree type, tree init, int nested, int flags,
TREE_SIDE_EFFECTS (init) = true;
}
else if (picflags & PICFLAG_NOT_ALL_CONSTANT)
- /* Make sure TREE_CONSTANT isn't set from build_constructor. */
- TREE_CONSTANT (init) = false;
+ {
+ /* Make sure TREE_CONSTANT isn't set from build_constructor. */
+ TREE_CONSTANT (init) = false;
+ TREE_SIDE_EFFECTS (init) = false;
+ }
else
{
TREE_CONSTANT (init) = 1;
+ TREE_SIDE_EFFECTS (init) = false;
if (!(picflags & PICFLAG_NOT_ALL_SIMPLE))
TREE_STATIC (init) = 1;
}
--
2.18.1
