c++: constexpr-evaluate more assumes

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.

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index fc1bc53..39bb023 100644 (file)
--- 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 (file)
index 0000000..475555a
--- /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 (file)
index 0000000..cbd6815
--- /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" }