On Tue, 5 May 2020, Patrick Palka wrote:
> Unfortunately, the previous fix to PR94038 is fragile. When the
> argument to fold_for_warn is a bare CALL_EXPR, then all is well: the
> result of maybe_constant_value from fold_for_warn (with
> uid_sensitive=true) is reused via the cv_cache in the subsequent call to
> maybe_constant_value from cp_fold (with uid_sensitive=false), so we
> avoid instantiating bar<int>.
>
> But when the argument to fold_for_warn is more complex, e.g. an
> INDIRECT_REF of a CALL_EXPR, as in the testcase below (due to bar<int>()
> returning const int& which we need to decay to int) then from
> fold_for_warn we call maybe_constant_value on the INDIRECT_REF, and from
> cp_fold we call it on the CALL_EXPR, so there is no reuse via the
> cv_cache and we therefore end up instantiating bar<int>.
>
> So for a more robust solution to this general issue of warning flags
> affecting code generation, it seems that we need a way to globally avoid
> template instantiation during constexpr evaluation whenever we're
> performing warning-dependent folding.
>
> To that end, this patch replaces the flag constexpr_ctx::uid_sensitive
> with a global flag uid_sensitive_constexpr_evaluation_p, and enables it
> during fold_for_warn using an RAII helper.
>
> The patch also adds a counter that keeps track of the number of times
> uid_sensitive_constexpr_evaluation_p is called, and we use this to
> determine whether the result of constexpr evaluation depended on the
> flag's value. This lets us safely update the cv_cache and fold_cache
> from fold_for_warn in the most common case where the flag's value was
> irrelevant during constexpr evaluation.
Whoops, shortly after I sent the patch it occurred to me that we only
want to keep track of the number of times
uid_sensitive_constexpr_evaluation_p was called _and returned true_.
If the predicate was called and it returned false, then constexpr
evaluation was not restricted, so we could safely cache the result.
Please consider this patch instead, which also defines the various
helper classse out-of-line in constexpr.c for improved encapsulation:
-- >8 --
gcc/cp/ChangeLog:
PR c++/94038
* constexpr.c (constexpr_ctx::uid_sensitive): Remove field.
(uid_sensitive_constexpr_evaluation_value): Define.
(uid_sensitive_constexpr_evaluation_true_counter): Define.
(uid_sensitive_constexpr_evaluation_p): Define.
(uid_sensitive_constexpr_evaluation): Define its constructor and
destructor.
(uid_sensitive_constexpr_evaluation_marker): Define its
constructor and its evaluation_restricted_p method.
(get_fundef_copy): Remove 'ctx' parameter. Use u_s_c_e_p
instead of constexpr_ctx::uid_sensitive.
(cxx_eval_call_expression): Use u_s_c_e_p instead, and test it
last. Adjust call to get_fundef_copy.
(instantiate_cx_fn_r): Test u_s_c_e_p so that we increment the
counter if necessary.
(cxx_eval_outermost_constant_expr): Remove 'uid_sensitive'
parameter. Adjust function body accordingly. Use
uid_sensitive_constexpr_evaluation::value.
(maybe_constant_value): Remove 'uid_sensitive' parameter and
adjust function body accordingly. Set up a
uid_sensitive_constexpr_evaluation_marker, and use it to
conditionally update the cv_cache.
* cp-gimplify.h (cp_fold): Set up a
uid_sensitive_constexpr_evaluation_marker, and use it to
conditionally update the fold_cache.
* cp-tree.h (maybe_constant_value): Update declaration.
(struct uid_sensitive_constexpr_evaluation): Define.
(struct sensitive_constexpr_evaluation_marker): Define.
* expr.c (fold_for_warn): Make constexpr evaluation
uid-sensitive before calling the folding subroutines. Drop
all but the first argument to maybe_constant_value.
gcc/testsuite/ChangeLog:
PR c++/94038
* g++.dg/warn/pr94038-2.C: New test.
---
gcc/cp/constexpr.c | 96 ++++++++++++++++++++-------
gcc/cp/cp-gimplify.c | 13 ++--
gcc/cp/cp-tree.h | 24 ++++++-
gcc/cp/expr.c | 4 +-
gcc/testsuite/g++.dg/warn/pr94038-2.C | 28 ++++++++
5 files changed, 136 insertions(+), 29 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/pr94038-2.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 637cb746576..668e8b5d1b3 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1089,11 +1089,64 @@ struct constexpr_ctx {
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;
};
+/* This internal flag controls whether we should avoid doing anything during
+ constexpr evaluation that would cause extra DECL_UID generation, such as
+ template instantiation and function copying. */
+
+static bool uid_sensitive_constexpr_evaluation_value;
+
+/* An internal counter that keeps track of the number of times
+ uid_sensitive_constexpr_evaluation_p returned true. */
+
+static unsigned uid_sensitive_constexpr_evaluation_true_counter;
+
+/* The accessor for uid_sensitive_constexpr_evaluation::value which also
+ increments the corresponding counter. */
+
+static bool
+uid_sensitive_constexpr_evaluation_p ()
+{
+ if (uid_sensitive_constexpr_evaluation_value)
+ {
+ ++uid_sensitive_constexpr_evaluation_true_counter;
+ return true;
+ }
+ else
+ return false;
+}
+
+uid_sensitive_constexpr_evaluation
+::uid_sensitive_constexpr_evaluation ()
+ : saved_value (uid_sensitive_constexpr_evaluation_value)
+{
+ uid_sensitive_constexpr_evaluation_value = true;
+}
+
+uid_sensitive_constexpr_evaluation
+::~uid_sensitive_constexpr_evaluation ()
+{
+ uid_sensitive_constexpr_evaluation_value = saved_value;
+}
+
+uid_sensitive_constexpr_evaluation_marker
+::uid_sensitive_constexpr_evaluation_marker ()
+ : saved_counter (uid_sensitive_constexpr_evaluation_true_counter)
+{
+}
+
+/* Returns true iff some constexpr evaluation was restricted due to
+ the value of uid_sensitive_constexpr_evaluation_p since this marker
+ object was constructed. */
+
+bool
+uid_sensitive_constexpr_evaluation_marker::evaluation_restricted_p ()
+{
+ return saved_counter != uid_sensitive_constexpr_evaluation_true_counter;
+}
+
+
/* A table of all constexpr calls that have been evaluated by the
compiler in this translation unit. */
@@ -1156,7 +1209,7 @@ static GTY(()) hash_map<tree, tree> *fundef_copies_table;
is parms, TYPE is result. */
static tree
-get_fundef_copy (const constexpr_ctx *ctx, constexpr_fundef *fundef)
+get_fundef_copy (constexpr_fundef *fundef)
{
tree copy;
bool existed;
@@ -1173,7 +1226,7 @@ get_fundef_copy (const constexpr_ctx *ctx,
constexpr_fundef *fundef)
}
else if (*slot == NULL_TREE)
{
- if (ctx->uid_sensitive)
+ if (uid_sensitive_constexpr_evaluation_p ())
return NULL_TREE;
/* We've already used the function itself, so make a copy. */
@@ -2292,8 +2345,8 @@ 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))
+ && DECL_TEMPLOID_INSTANTIATION (fun)
+ && !uid_sensitive_constexpr_evaluation_p ())
{
location_t save_loc = input_location;
input_location = loc;
@@ -2454,7 +2507,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
gcc_assert (at_eof >= 2 && ctx->quiet);
*non_constant_p = true;
}
- else if (tree copy = get_fundef_copy (ctx, new_call.fundef))
+ else if (tree copy = get_fundef_copy (new_call.fundef))
{
tree body, parms, res;
releasing_vec ctors;
@@ -6485,7 +6538,8 @@ instantiate_cx_fn_r (tree *tp, int *walk_subtrees, void
*/*data*/)
&& DECL_DECLARED_CONSTEXPR_P (*tp)
&& !DECL_INITIAL (*tp)
&& !trivial_fn_p (*tp)
- && DECL_TEMPLOID_INSTANTIATION (*tp))
+ && DECL_TEMPLOID_INSTANTIATION (*tp)
+ && !uid_sensitive_constexpr_evaluation_p ())
{
++function_depth;
instantiate_decl (*tp, /*defer_ok*/false, /*expl_inst*/false);
@@ -6552,8 +6606,7 @@ 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,
- bool uid_sensitive = false)
+ tree object = NULL_TREE)
{
auto_timevar time (TV_CONSTEXPR);
@@ -6569,8 +6622,7 @@ 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, NULL,
allow_non_constant, strict,
- manifestly_const_eval || !allow_non_constant,
- uid_sensitive };
+ manifestly_const_eval || !allow_non_constant };
tree type = initialized_type (t);
tree r = t;
@@ -6660,7 +6712,7 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
auto_vec<tree, 16> cleanups;
global_ctx.cleanups = &cleanups;
- if (!uid_sensitive)
+ if (!uid_sensitive_constexpr_evaluation_value)
instantiate_constexpr_fns (r);
r = cxx_eval_constant_expression (&ctx, r,
false, &non_constant_p, &overflow_p);
@@ -6909,14 +6961,12 @@ 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. UID_SENSITIVE is true if we can't do anything that
- would affect DECL_UID ordering. */
+ as per P0595. */
static GTY((deletable)) hash_map<tree, tree> *cv_cache;
tree
-maybe_constant_value (tree t, tree decl, bool manifestly_const_eval,
- bool uid_sensitive)
+maybe_constant_value (tree t, tree decl, bool manifestly_const_eval)
{
tree r;
@@ -6934,8 +6984,7 @@ 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, uid_sensitive);
+ return cxx_eval_outermost_constant_expr (t, true, true, true, false, decl);
if (cv_cache == NULL)
cv_cache = hash_map<tree, tree>::create_ggc (101);
@@ -6950,14 +6999,15 @@ maybe_constant_value (tree t, tree decl, bool
manifestly_const_eval,
return r;
}
- r = cxx_eval_outermost_constant_expr (t, true, true, false, false,
- decl, uid_sensitive);
+ uid_sensitive_constexpr_evaluation_marker m;
+ r = cxx_eval_outermost_constant_expr (t, true, true, false, false, decl);
gcc_checking_assert (r == t
|| CONVERT_EXPR_P (t)
|| TREE_CODE (t) == VIEW_CONVERT_EXPR
|| (TREE_CONSTANT (t) && !TREE_CONSTANT (r))
|| !cp_tree_equal (r, t));
- cv_cache->put (t, r);
+ if (!m.evaluation_restricted_p ())
+ cv_cache->put (t, r);
return r;
}
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index fc26a85f43a..3bcad665e8d 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2443,6 +2443,8 @@ cp_fold (tree x)
if (tree *cached = fold_cache->get (x))
return *cached;
+ uid_sensitive_constexpr_evaluation_marker m;
+
code = TREE_CODE (x);
switch (code)
{
@@ -2925,10 +2927,13 @@ cp_fold (tree x)
return org_x;
}
- fold_cache->put (org_x, x);
- /* Prevent that we try to fold an already folded result again. */
- if (x != org_x)
- fold_cache->put (x, x);
+ if (!m.evaluation_restricted_p ())
+ {
+ fold_cache->put (org_x, x);
+ /* Prevent that we try to fold an already folded result again. */
+ if (x != org_x)
+ fold_cache->put (x, x);
+ }
return x;
}
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f4328403bc7..7f1c1ee8908 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7949,7 +7949,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, bool = false);
+extern tree maybe_constant_value (tree, tree = NULL_TREE, 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,
@@ -7968,6 +7968,28 @@ extern tree fold_sizeof_expr (tree);
extern void clear_cv_and_fold_caches (bool = true);
extern tree unshare_constructor (tree
CXX_MEM_STAT_INFO);
+/* An RAII sentinel used to restrict constexpr evaluation so that it
+ doesn't do anything that causes extra DECL_UID generation. */
+
+struct uid_sensitive_constexpr_evaluation
+{
+ const bool saved_value;
+ uid_sensitive_constexpr_evaluation ();
+ ~uid_sensitive_constexpr_evaluation ();
+};
+
+/* Used to determine whether uid_sensitive_constexpr_evaluation_p was
+ called and returned true, indicating that we've inhibited constexpr
+ evaluation in order to avoid UID generation. We use this to control
+ updates to the fold_cache and cv_cache. */
+
+struct uid_sensitive_constexpr_evaluation_marker
+{
+ const unsigned saved_counter;
+ uid_sensitive_constexpr_evaluation_marker ();
+ bool evaluation_restricted_p ();
+};
+
/* In cp-ubsan.c */
extern void cp_ubsan_maybe_instrument_member_call (tree);
extern void cp_ubsan_instrument_member_accesses (tree *);
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 9b535708c57..97671edf800 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -399,6 +399,8 @@ fold_for_warn (tree x)
{
/* C++ implementation. */
+ uid_sensitive_constexpr_evaluation s;
+
/* It's not generally safe to fully fold inside of a template, so
call fold_non_dependent_expr instead. */
if (processing_template_decl)
@@ -410,7 +412,7 @@ fold_for_warn (tree x)
return f;
}
else if (cxx_dialect >= cxx11)
- x = maybe_constant_value (x, NULL_TREE, false, true);
+ x = maybe_constant_value (x);
return c_fully_fold (x, /*for_init*/false, /*maybe_constp*/NULL);
}
diff --git a/gcc/testsuite/g++.dg/warn/pr94038-2.C
b/gcc/testsuite/g++.dg/warn/pr94038-2.C
new file mode 100644
index 00000000000..a468cc055eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr94038-2.C
@@ -0,0 +1,28 @@
+// PR c++/94038
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-O -Wall" }
+
+static constexpr int x = 0;
+
+template<typename T>
+constexpr const int&
+foo()
+{
+ static_assert(T(1) == 0, "");
+ return x;
+}
+
+template<typename T>
+constexpr const int&
+bar()
+{
+ return foo<T>();
+}
+
+constexpr int
+baz(int a)
+{
+ return a;
+}
+
+static_assert(decltype(baz(bar<int>())){} == 0, "");
--
2.26.2.447.gd61d20c9b4
