From aaa26bf496a646778ac861aed124d960b5bf549f Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Sun, 26 Jan 2020 22:58:32 -0500 Subject: [PATCH] c++: Use constexpr to avoid wrong -Wsign-compare (PR90691). 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/ChangeLog | 8 ++++ gcc/cp/constexpr.c | 42 +++++++++++++++------ gcc/cp/cp-tree.h | 2 +- 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 +++++++++++ 6 files changed, 66 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wsign-compare-9.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 3e5e547fd87..dff6a170580 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,5 +1,13 @@ 2020-02-08 Jason Merrill + PR c++/90691 + * expr.c (fold_for_warn): Call maybe_constant_value. + * constexpr.c (struct constexpr_ctx): Add uid_sensitive bit-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. + PR c++/92852 * constexpr.c (maybe_constant_value): Don't unshare if the cached value is the same as the argument. diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 1b35bc107e9..192ba566427 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 *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; @@ -6275,7 +6286,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); @@ -6285,7 +6297,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; @@ -6375,7 +6388,8 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant, auto_vec 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); @@ -6623,12 +6637,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 *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; @@ -6646,7 +6662,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::create_ggc (101); @@ -6658,7 +6675,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/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/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 +bool c(unsigned int d) { + return d >= e.a; +} + +bool test(unsigned int d); +bool test(unsigned int d) { + return c(d); +} -- 2.30.2