From f65a3299a521a44522c1e01724f75e36af22f40b Mon Sep 17 00:00:00 2001 From: Patrick Palka Date: Thu, 21 May 2020 09:03:43 -0400 Subject: c++: template instantiation during fold_for_warn [PR94038] 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. 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() 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. 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 returned true, and we use this to determine whether the result of constexpr evaluation was restricted by the flag. This lets us safely update the cv_cache and fold_cache from fold_for_warn in the most common case where the flag did not restrict constexpr evaluation. 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_sentinel): Define its constructor. (uid_sensitive_constexpr_evaluation_checker): 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. (maybe_constant_value): Remove 'uid_sensitive' parameter and adjust function body accordingly. Set up a uid_sensitive_constexpr_evaluation_checker, and use it to conditionally update the cv_cache. * cp-gimplify.c (cp_fold): Set up a uid_sensitive_constexpr_evaluation_checker, and use it to conditionally update the fold_cache. * cp-tree.h (maybe_constant_value): Update declaration. (struct uid_sensitive_constexpr_evaluation_sentinel): Define. (struct sensitive_constexpr_evaluation_checker): Define. * expr.c (fold_for_warn): Set up a uid_sensitive_constexpr_evaluation_sentinel 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 | 100 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 76 insertions(+), 24 deletions(-) (limited to 'gcc/cp/constexpr.c') diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 04faabc..c1ab928 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1089,11 +1089,67 @@ 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 body 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; +} + +/* The default constructor for uid_sensitive_constexpr_evaluation_sentinel + enables the internal flag for uid_sensitive_constexpr_evaluation_p + during the lifetime of the sentinel object. Upon its destruction, the + previous value of uid_sensitive_constexpr_evaluation_p is restored. */ + +uid_sensitive_constexpr_evaluation_sentinel +::uid_sensitive_constexpr_evaluation_sentinel () + : ovr (uid_sensitive_constexpr_evaluation_value, true) +{ +} + +/* The default constructor for uid_sensitive_constexpr_evaluation_checker + records the current number of times that uid_sensitive_constexpr_evaluation_p + has been called and returned true. */ + +uid_sensitive_constexpr_evaluation_checker +::uid_sensitive_constexpr_evaluation_checker () + : saved_counter (uid_sensitive_constexpr_evaluation_true_counter) +{ +} + +/* Returns true iff uid_sensitive_constexpr_evaluation_p is true, and + some constexpr evaluation was restricted due to u_s_c_e_p being called + and returning true during the lifetime of this checker object. */ + +bool +uid_sensitive_constexpr_evaluation_checker::evaluation_restricted_p () const +{ + return (uid_sensitive_constexpr_evaluation_value + && 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 +1212,7 @@ static GTY(()) hash_map *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 +1229,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 +2348,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 +2510,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 +6541,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 +6609,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 +6625,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,8 +6715,7 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant, auto_vec cleanups; global_ctx.cleanups = &cleanups; - if (!uid_sensitive) - instantiate_constexpr_fns (r); + instantiate_constexpr_fns (r); r = cxx_eval_constant_expression (&ctx, r, false, &non_constant_p, &overflow_p); @@ -6909,14 +6963,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 *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 +6986,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::create_ggc (101); @@ -6950,14 +7001,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_checker c; + 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 (!c.evaluation_restricted_p ()) + cv_cache->put (t, r); return r; } -- cgit v1.1