diff mbox series

[pushed] c++: constexpr-evaluate more assumes

Message ID 20221025163750.96139-1-jason@redhat.com
State New
Headers show
Series [pushed] c++: constexpr-evaluate more assumes | expand

Commit Message

Jason Merrill Oct. 25, 2022, 4:37 p.m. UTC
Tested x86_64-pc-linux-gnu, applying to trunk.

-- >8 --

The initial [[assume]] support avoided evaluating assumes with
TREE_SIDE_EFFECTS set, such as calls, because we don't want any side-effects
that change the constexpr state.  This patch allows us to evaluate
expressions with that flag set by tracking which variables the evaluation is
allowed to modify, and giving up if it tries to touch any others.

I considered allowing changes to other variables and then rolling them back,
but that seems like a rare enough situation that it doesn't seem worth
working to handle nicely at this point.

gcc/cp/ChangeLog:

	* constexpr.cc (class constexpr_global_ctx): Add modifiable field,
	get_value, get_value_ptr, put_value, remove_value, flush_modifiable
	member functions.
	(class modifiable_tracker): New.
	(cxx_eval_internal_function): Use it.
	(diagnose_failing_condition): Strip CLEANUP_POINT_EXPR.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp23/attr-assume9.C: New test.
	* g++.dg/cpp23/attr-assume10.C: New test.
---
 gcc/cp/constexpr.cc                        | 136 ++++++++++++++-------
 gcc/testsuite/g++.dg/cpp23/attr-assume10.C |  22 ++++
 gcc/testsuite/g++.dg/cpp23/attr-assume9.C  |  19 +++
 3 files changed, 133 insertions(+), 44 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp23/attr-assume10.C
 create mode 100644 gcc/testsuite/g++.dg/cpp23/attr-assume9.C


base-commit: 3ee675724cb27eb2e8cfd82f6e234edc4a63f6be
prerequisite-patch-id: f5ac6dd54aa193cf71a1bd0526bebf7f43012b0e
diff mbox series

Patch

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index fc1bc53f68a..39bb023b79c 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1092,10 +1092,11 @@  enum constexpr_switch_state {
    cxx_eval_outermost_constant_expr invocation.  VALUES is a map of values of
    variables initialized within the expression.  */
 
-struct constexpr_global_ctx {
+class constexpr_global_ctx {
   /* Values for any temporaries or local variables within the
      constant-expression. */
   hash_map<tree,tree> values;
+public:
   /* Number of cxx_eval_constant_expression calls (except skipped ones,
      on simple constants or location wrappers) encountered during current
      cxx_eval_outermost_constant_expr call.  */
@@ -1105,11 +1106,61 @@  struct constexpr_global_ctx {
   auto_vec<tree, 16> heap_vars;
   /* Cleanups that need to be evaluated at the end of CLEANUP_POINT_EXPR.  */
   vec<tree> *cleanups;
+  /* If non-null, only allow modification of existing values of the variables
+     in this set.  Set by modifiable_tracker, below.  */
+  hash_set<tree> *modifiable;
   /* Number of heap VAR_DECL deallocations.  */
   unsigned heap_dealloc_count;
   /* Constructor.  */
   constexpr_global_ctx ()
-    : constexpr_ops_count (0), cleanups (NULL), heap_dealloc_count (0) {}
+    : constexpr_ops_count (0), cleanups (NULL), modifiable (nullptr),
+      heap_dealloc_count (0) {}
+
+ tree get_value (tree t)
+  {
+    if (tree *p = values.get (t))
+      return *p;
+    return NULL_TREE;
+  }
+  tree *get_value_ptr (tree t)
+  {
+    if (modifiable && !modifiable->contains (t))
+      return nullptr;
+    return values.get (t);
+  }
+  void put_value (tree t, tree v)
+  {
+    bool already_in_map = values.put (t, v);
+    if (!already_in_map && modifiable)
+      modifiable->add (t);
+  }
+  void remove_value (tree t) { values.remove (t); }
+};
+
+/* Helper class for constexpr_global_ctx.  In some cases we want to avoid
+   side-effects from evaluation of a particular subexpression of a
+   constant-expression.  In such cases we use modifiable_tracker to prevent
+   modification of variables created outside of that subexpression.
+
+   ??? We could change the hash_set to a hash_map, allow and track external
+   modifications, and roll them back in the destructor.  It's not clear to me
+   that this would be worthwhile.  */
+
+class modifiable_tracker
+{
+  hash_set<tree> set;
+  constexpr_global_ctx *global;
+public:
+  modifiable_tracker (constexpr_global_ctx *g): global(g)
+  {
+    global->modifiable = &set;
+  }
+  ~modifiable_tracker ()
+  {
+    for (tree t: set)
+      global->remove_value (t);
+    global->modifiable = nullptr;
+  }
 };
 
 /* The constexpr expansion context.  CALL is the current function
@@ -1653,7 +1704,7 @@  addr_of_non_const_var (tree *tp, int *walk_subtrees, void *data)
 	    return var;
 
 	  constexpr_global_ctx *global = (constexpr_global_ctx *) data;
-	  if (global->values.get (var))
+	  if (global->get_value (var))
 	    return var;
 	}
   if (TYPE_P (*tp))
@@ -1865,6 +1916,8 @@  diagnose_failing_condition (tree bad, location_t cloc, bool show_expr_p,
 {
   /* Nobody wants to see the artificial (bool) cast.  */
   bad = tree_strip_nop_conversions (bad);
+  if (TREE_CODE (bad) == CLEANUP_POINT_EXPR)
+    bad = TREE_OPERAND (bad, 0);
 
   /* Actually explain the failure if this is a concept check or a
      requires-expression.  */
@@ -1902,18 +1955,14 @@  cxx_eval_internal_function (const constexpr_ctx *ctx, tree t,
       return void_node;
 
     case IFN_ASSUME:
-      /* For now, restrict constexpr evaluation of [[assume (cond)]]
-	 only to the cases which don't have side-effects.  Evaluating
-	 it even when it does would mean we'd need to somehow undo
-	 all the side-effects e.g. in ctx->global->values.  */
-      if (!TREE_SIDE_EFFECTS (CALL_EXPR_ARG (t, 0))
-	  /* And it needs to be a potential constant expression.  */
-	  && potential_rvalue_constant_expression (CALL_EXPR_ARG (t, 0)))
+      if (potential_rvalue_constant_expression (CALL_EXPR_ARG (t, 0)))
 	{
 	  constexpr_ctx new_ctx = *ctx;
 	  new_ctx.quiet = true;
 	  tree arg = CALL_EXPR_ARG (t, 0);
 	  bool new_non_constant_p = false, new_overflow_p = false;
+	  /* Avoid modification of existing values.  */
+	  modifiable_tracker ms (new_ctx.global);
 	  arg = cxx_eval_constant_expression (&new_ctx, arg, vc_prvalue,
 					      &new_non_constant_p,
 					      &new_overflow_p);
@@ -2613,7 +2662,7 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	      // See PR98988 and PR99031.
 	      varpool_node::finalize_decl (var);
 	      ctx->global->heap_vars.safe_push (var);
-	      ctx->global->values.put (var, NULL_TREE);
+	      ctx->global->put_value (var, NULL_TREE);
 	      return fold_convert (ptr_type_node, build_address (var));
 	    }
 	  else
@@ -2641,7 +2690,7 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 			  return t;
 			}
 		      DECL_NAME (var) = heap_deleted_identifier;
-		      ctx->global->values.remove (var);
+		      ctx->global->remove_value (var);
 		      ctx->global->heap_dealloc_count++;
 		      return void_node;
 		    }
@@ -2663,7 +2712,7 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 			  return t;
 			}
 		      DECL_NAME (var) = heap_deleted_identifier;
-		      ctx->global->values.remove (var);
+		      ctx->global->remove_value (var);
 		      ctx->global->heap_dealloc_count++;
 		      return void_node;
 		    }
@@ -2726,7 +2775,7 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
       new_ctx.object = AGGR_INIT_EXPR_SLOT (t);
       tree ctor = new_ctx.ctor = build_constructor (DECL_CONTEXT (fun), NULL);
       CONSTRUCTOR_NO_CLEARING (ctor) = true;
-      ctx->global->values.put (new_ctx.object, ctor);
+      ctx->global->put_value (new_ctx.object, ctor);
       ctx = &new_ctx;
     }
 
@@ -2949,12 +2998,12 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 		  if (TREE_CODE (arg) == CONSTRUCTOR)
 		    vec_safe_push (ctors, arg);
 		}
-	      ctx->global->values.put (remapped, arg);
+	      ctx->global->put_value (remapped, arg);
 	      remapped = DECL_CHAIN (remapped);
 	    }
 	  /* Add the RESULT_DECL to the values map, too.  */
 	  gcc_assert (!DECL_BY_REFERENCE (res));
-	  ctx->global->values.put (res, NULL_TREE);
+	  ctx->global->put_value (res, NULL_TREE);
 
 	  /* Track the callee's evaluated SAVE_EXPRs and TARGET_EXPRs so that
 	     we can forget their values after the call.  */
@@ -3009,7 +3058,7 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	    result = void_node;
 	  else
 	    {
-	      result = *ctx->global->values.get (res);
+	      result = ctx->global->get_value (res);
 	      if (result == NULL_TREE && !*non_constant_p
 		  && !DECL_DESTRUCTOR_P (fun))
 		{
@@ -3031,15 +3080,15 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	  /* Forget the saved values of the callee's SAVE_EXPRs and
 	     TARGET_EXPRs.  */
 	  for (tree save_expr : save_exprs)
-	    ctx->global->values.remove (save_expr);
+	    ctx->global->remove_value (save_expr);
 
 	  /* Remove the parms/result from the values map.  Is it worth
 	     bothering to do this when the map itself is only live for
 	     one constexpr evaluation?  If so, maybe also clear out
 	     other vars from call, maybe in BIND_EXPR handling?  */
-	  ctx->global->values.remove (res);
+	  ctx->global->remove_value (res);
 	  for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
-	    ctx->global->values.remove (parm);
+	    ctx->global->remove_value (parm);
 
 	  /* Free any parameter CONSTRUCTORs we aren't returning directly.  */
 	  while (!ctors->is_empty ())
@@ -4876,7 +4925,7 @@  verify_ctor_sanity (const constexpr_ctx *ctx, tree type)
 			  (TREE_TYPE (type), TREE_TYPE (otype)))));
     }
   gcc_assert (!ctx->object || !DECL_P (ctx->object)
-	      || *(ctx->global->values.get (ctx->object)) == ctx->ctor);
+	      || ctx->global->get_value (ctx->object) == ctx->ctor);
 }
 
 /* Subroutine of cxx_eval_constant_expression.
@@ -5198,7 +5247,7 @@  cxx_eval_vec_init (const constexpr_ctx *ctx, tree t,
 	  new_ctx.object = VEC_INIT_EXPR_SLOT (t);
 	  tree ctor = new_ctx.ctor = build_constructor (atype, NULL);
 	  CONSTRUCTOR_NO_CLEARING (ctor) = true;
-	  ctx->global->values.put (new_ctx.object, ctor);
+	  ctx->global->put_value (new_ctx.object, ctor);
 	  ctx = &new_ctx;
 	}
       init = expand_vec_init_expr (ctx->object, t, complain);
@@ -5884,7 +5933,7 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
      we're initializing.  */
   tree *valp;
   if (DECL_P (object))
-    valp = ctx->global->values.get (object);
+    valp = ctx->global->get_value_ptr (object);
   else
     valp = NULL;
   if (!valp)
@@ -6116,7 +6165,7 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
       /* The hash table might have moved since the get earlier, and the
 	 initializer might have mutated the underlying CONSTRUCTORs, so we must
 	 recompute VALP. */
-      valp = ctx->global->values.get (object);
+      valp = ctx->global->get_value_ptr (object);
       for (unsigned i = 0; i < vec_safe_length (indexes); i++)
 	{
 	  ctors[i] = valp;
@@ -6546,7 +6595,7 @@  cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
 
       /* Forget saved values of SAVE_EXPRs and TARGET_EXPRs.  */
       for (tree save_expr : save_exprs)
-	ctx->global->values.remove (save_expr);
+	ctx->global->remove_value (save_expr);
       save_exprs.truncate (0);
 
       if (++count >= constexpr_loop_limit)
@@ -6568,7 +6617,7 @@  cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
 
   /* Forget saved values of SAVE_EXPRs and TARGET_EXPRs.  */
   for (tree save_expr : save_exprs)
-    ctx->global->values.remove (save_expr);
+    ctx->global->remove_value (save_expr);
 
   return NULL_TREE;
 }
@@ -6865,8 +6914,8 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
       /* We ask for an rvalue for the RESULT_DECL when indirecting
 	 through an invisible reference, or in named return value
 	 optimization.  */
-      if (tree *p = ctx->global->values.get (t))
-	return *p;
+      if (tree v = ctx->global->get_value (t))
+	return v;
       else
 	{
 	  if (!ctx->quiet)
@@ -6909,10 +6958,9 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
       else if (t == ctx->object)
 	return ctx->ctor;
       if (VAR_P (t))
-	if (tree *p = ctx->global->values.get (t))
-	  if (*p != NULL_TREE)
+	if (tree v = ctx->global->get_value (t))
 	    {
-	      r = *p;
+	      r = v;
 	      break;
 	    }
       if (ctx->manifestly_const_eval)
@@ -6955,8 +7003,8 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
     case PARM_DECL:
       if (lval && !TYPE_REF_P (TREE_TYPE (t)))
 	/* glvalue use.  */;
-      else if (tree *p = ctx->global->values.get (r))
-	r = *p;
+      else if (tree v = ctx->global->get_value (r))
+	r = v;
       else if (lval)
 	/* Defer in case this is only used for its type.  */;
       else if (COMPLETE_TYPE_P (TREE_TYPE (t))
@@ -7015,7 +7063,7 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	    new_ctx.object = r;
 	    new_ctx.ctor = build_constructor (TREE_TYPE (r), NULL);
 	    CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = true;
-	    ctx->global->values.put (r, new_ctx.ctor);
+	    ctx->global->put_value (r, new_ctx.ctor);
 	    ctx = &new_ctx;
 	  }
 
@@ -7030,12 +7078,12 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	    if (CLASS_TYPE_P (TREE_TYPE (r))
 		&& CP_TYPE_CONST_P (TREE_TYPE (r)))
 	      TREE_READONLY (init) = true;
-	    ctx->global->values.put (r, init);
+	    ctx->global->put_value (r, init);
 	  }
 	else if (ctx == &new_ctx)
 	  /* We gave it a CONSTRUCTOR above.  */;
 	else
-	  ctx->global->values.put (r, NULL_TREE);
+	  ctx->global->put_value (r, NULL_TREE);
       }
       break;
 
@@ -7058,11 +7106,11 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	gcc_checking_assert (!TARGET_EXPR_DIRECT_INIT_P (t));
 	/* Avoid evaluating a TARGET_EXPR more than once.  */
 	tree slot = TARGET_EXPR_SLOT (t);
-	if (tree *p = ctx->global->values.get (slot))
+	if (tree v = ctx->global->get_value (slot))
 	  {
 	    if (lval)
 	      return slot;
-	    r = *p;
+	    r = v;
 	    break;
 	  }
 	if ((AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type)))
@@ -7078,7 +7126,7 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	    new_ctx.ctor = build_constructor (type, NULL);
 	    CONSTRUCTOR_NO_CLEARING (new_ctx.ctor) = true;
 	    new_ctx.object = slot;
-	    ctx->global->values.put (new_ctx.object, new_ctx.ctor);
+	    ctx->global->put_value (new_ctx.object, new_ctx.ctor);
 	    ctx = &new_ctx;
 	  }
 	/* Pass vc_prvalue because this indicates
@@ -7092,7 +7140,7 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	if (TARGET_EXPR_CLEANUP (t) && !CLEANUP_EH_ONLY (t))
 	  ctx->global->cleanups->safe_push (TARGET_EXPR_CLEANUP (t));
 	r = unshare_constructor (r);
-	ctx->global->values.put (slot, r);
+	ctx->global->put_value (slot, r);
 	if (ctx->save_exprs)
 	  ctx->save_exprs->safe_push (slot);
 	if (lval)
@@ -7135,15 +7183,15 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 
     case SAVE_EXPR:
       /* Avoid evaluating a SAVE_EXPR more than once.  */
-      if (tree *p = ctx->global->values.get (t))
-	r = *p;
+      if (tree v = ctx->global->get_value (t))
+	r = v;
       else
 	{
 	  r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), vc_prvalue,
 					    non_constant_p, overflow_p);
 	  if (*non_constant_p)
 	    break;
-	  ctx->global->values.put (t, r);
+	  ctx->global->put_value (t, r);
 	  if (ctx->save_exprs)
 	    ctx->save_exprs->safe_push (t);
 	}
@@ -8078,7 +8126,7 @@  cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
 	gcc_assert (same_type_ignoring_top_level_qualifiers_p
 		    (type, TREE_TYPE (object)));
       if (object && DECL_P (object))
-	global_ctx.values.put (object, ctx.ctor);
+	global_ctx.put_value (object, ctx.ctor);
       if (TREE_CODE (r) == TARGET_EXPR)
 	/* Avoid creating another CONSTRUCTOR when we expand the
 	   TARGET_EXPR.  */
diff --git a/gcc/testsuite/g++.dg/cpp23/attr-assume10.C b/gcc/testsuite/g++.dg/cpp23/attr-assume10.C
new file mode 100644
index 00000000000..475555ac415
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp23/attr-assume10.C
@@ -0,0 +1,22 @@ 
+// Test that s.i is not modified by the assume.
+// { dg-do compile { target c++17 } }
+
+struct string
+{
+  const char *p;
+  int i;
+  constexpr string (const char *p): p(p), i(0) { }
+  constexpr int length () { ++i; return __builtin_strlen (p); }
+};
+
+constexpr int f()
+{
+  string s ("foobar");
+  [[assume (s.length () > 0)]];
+  if (s.i != 0) __builtin_abort();
+  int len = s.length ();
+  if (s.i != 1) __builtin_abort();
+  return len;
+}
+
+static_assert (f());
diff --git a/gcc/testsuite/g++.dg/cpp23/attr-assume9.C b/gcc/testsuite/g++.dg/cpp23/attr-assume9.C
new file mode 100644
index 00000000000..cbd68151692
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp23/attr-assume9.C
@@ -0,0 +1,19 @@ 
+// Diagnose failed assumptions involving a function call.
+// { dg-do compile { target c++17 } }
+
+struct string
+{
+  const char *p;
+  constexpr string (const char *p): p(p) { }
+  constexpr int length () { return __builtin_strlen (p); }
+};
+
+constexpr int f()
+{
+  string s ("foobar");
+  [[assume (s.length () == 0)]]; // { dg-error "assume" }
+  // { dg-message "6 == 0" "" { target *-*-* } .-1 }
+  return s.length ();
+}
+
+static_assert (f());		// { dg-error "non-constant" }