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<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 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.

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index cfe5cf0..385bfcd 100644 (file)
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,37 @@
+2020-05-21  Patrick Palka  <ppalka@redhat.com>
+
+       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.
+
 2020-05-20  Marek Polacek  <polacek@redhat.com>
 
        DR 2237

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 04faabc..c1ab928 100644 (file)
--- 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<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 +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<tree, 16> 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<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 +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<tree, tree>::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;
 }
 

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index ede9892..2804958 100644 (file)
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2447,6 +2447,8 @@ cp_fold (tree x)
   if (tree *cached = fold_cache->get (x))
     return *cached;
 
+  uid_sensitive_constexpr_evaluation_checker c;
+
   code = TREE_CODE (x);
   switch (code)
     {
@@ -2929,10 +2931,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 (!c.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 27707ab..07c1614 100644 (file)
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7950,7 +7950,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,
@@ -7969,6 +7969,27 @@ 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_sentinel
+{
+  temp_override<bool> ovr;
+  uid_sensitive_constexpr_evaluation_sentinel ();
+};
+
+/* Used to determine whether uid_sensitive_constexpr_evaluation_p was
+   called and returned true, indicating that we've restricted 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_checker
+{
+  const unsigned saved_counter;
+  uid_sensitive_constexpr_evaluation_checker ();
+  bool evaluation_restricted_p () const;
+};
+
 /* 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 9b53570..a6ab96a 100644 (file)
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -399,6 +399,11 @@ fold_for_warn (tree x)
 {
   /* C++ implementation.  */
 
+  /* Prevent warning-dependent constexpr evaluation from changing
+     DECL_UID (which breaks -fcompare-debug) and from instantiating
+     templates.  */
+  uid_sensitive_constexpr_evaluation_sentinel 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 +415,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/ChangeLog b/gcc/testsuite/ChangeLog
index bcac1c6..6de0c5e 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-05-21  Patrick Palka  <ppalka@redhat.com>
+
+       PR c++/94038
+       * g++.dg/warn/pr94038-2.C: New test.
+
 2020-05-21  Martin Liska  <mliska@suse.cz>
 
        PR target/95229

diff --git a/gcc/testsuite/g++.dg/warn/pr94038-2.C b/gcc/testsuite/g++.dg/warn/pr94038-2.C
new file mode 100644 (file)
index 0000000..a468cc0
--- /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, "");