Hi!

For arguments passed by invisible reference, in the IL until genericization
we have the source types on the callee side and while on the caller side
we already pass references to the actual argument slot in the caller, we
undo that in cxx_bind_parameters_in_call's
      if (TREE_ADDRESSABLE (type))
        /* Undo convert_for_arg_passing work here.  */
        x = convert_from_reference (x);
This works fine most of the time, except when the type also has constexpr
destructor; in that case the destructor is invoked in the caller and thus
the unsharing we do to make sure that the callee doesn't modify caller's
values is in that case undesirable, it prevents the changes done in the
callee propagating to the caller which should see them for the constexpr
dtor evaluation.

The following patch fixes that.  While it could be perhaps done for all
TREE_ADDRESSABLE types, I don't see the need to change the behavior
if there is no constexpr non-trivial dtor.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-10-20  Jakub Jelinek  <ja...@redhat.com>

        PR c++/97388
        * constexpr.c (cxx_eval_call_expression): Don't unshare arguments
        if they have TREE_ADDRESSABLE type with non-trivial dtor.

        * g++.dg/cpp2a/constexpr-dtor5.C: New test.
        * g++.dg/cpp2a/constexpr-dtor6.C: New test.
        * g++.dg/cpp2a/constexpr-dtor7.C: New test.

--- gcc/cp/constexpr.c.jj       2020-10-19 09:32:28.000000000 +0200
+++ gcc/cp/constexpr.c  2020-10-19 15:34:17.707431525 +0200
@@ -2579,6 +2579,7 @@ cxx_eval_call_expression (const constexp
          for (int i = 0; i < TREE_VEC_LENGTH (bound); ++i)
            {
              tree arg = TREE_VEC_ELT (bound, i);
+             tree orig_arg = arg;
              if (entry)
                {
                  /* Unshare args going into the hash table to separate them
@@ -2587,13 +2588,24 @@ cxx_eval_call_expression (const constexp
                  arg = unshare_expr_without_location (arg);
                  TREE_VEC_ELT (bound, i) = arg;
                }
-             /* Don't share a CONSTRUCTOR that might be changed.  This is not
-                redundant with the unshare just above; we also don't want to
-                change the argument values in the hash table.  XXX Could we
-                unshare lazily in cxx_eval_store_expression?  */
-             arg = unshare_constructor (arg);
-             if (TREE_CODE (arg) == CONSTRUCTOR)
-               vec_safe_push (ctors, arg);
+             /* For arguments passed by invisible reference, if they have
+                non-trivial dtors, use the original argument without any
+                unsharing, because changes in the function call should be
+                reflected in the caller.  */
+             if (TREE_ADDRESSABLE (TREE_TYPE (arg))
+                 && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (arg)))
+               arg = orig_arg;
+             else
+               {
+                 /* Don't share a CONSTRUCTOR that might be changed.  This is
+                    not redundant with the unshare just above; we also don't
+                    want to change the argument values in the hash table.
+                    XXX Could we unshare lazily in
+                    cxx_eval_store_expression?  */
+                 arg = unshare_constructor (arg);
+                 if (TREE_CODE (arg) == CONSTRUCTOR)
+                   vec_safe_push (ctors, arg);
+               }
              ctx->global->values.put (remapped, arg);
              remapped = DECL_CHAIN (remapped);
            }
--- gcc/testsuite/g++.dg/cpp2a/constexpr-dtor5.C.jj     2020-10-19 
15:48:19.460200816 +0200
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor5.C        2020-10-19 
15:47:47.715662068 +0200
@@ -0,0 +1,26 @@
+// PR c++/97388
+// { dg-do compile { target c++20 } }
+
+struct S {
+  int m;
+  constexpr S () : m(1) {}
+  constexpr ~S () noexcept (false) { if (m == 1) { throw; } }
+};
+
+constexpr bool
+foo (S v)
+{
+  v.m = 2;
+  return true;
+}
+
+constexpr bool
+bar ()
+{
+  return foo (S ());
+}
+
+static_assert (foo (S ()));
+static_assert (bar ());
+constexpr bool x = foo (S ());
+constexpr bool y = bar ();
--- gcc/testsuite/g++.dg/cpp2a/constexpr-dtor6.C.jj     2020-10-19 
15:48:22.475157012 +0200
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor6.C        2020-10-19 
15:47:56.204538722 +0200
@@ -0,0 +1,27 @@
+// PR c++/97388
+// { dg-do compile { target c++20 } }
+
+struct S {
+  int *s;
+  constexpr S () : s(new int ()) {}
+  constexpr S (S &&x) noexcept : s(x.s) { x.s = nullptr; }
+  constexpr ~S () noexcept { delete s; }
+};
+
+constexpr bool
+foo (S v)
+{
+  auto x = static_cast<S &&> (v);
+  return true;
+}
+
+constexpr bool
+bar ()
+{
+  return foo (S ());
+}
+
+static_assert (foo (S ()));
+static_assert (bar ());
+constexpr bool x = foo (S ());
+constexpr bool y = bar ();
--- gcc/testsuite/g++.dg/cpp2a/constexpr-dtor7.C.jj     2020-10-19 
15:48:25.316115731 +0200
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor7.C        2020-10-19 
15:48:00.907470388 +0200
@@ -0,0 +1,19 @@
+// PR c++/97388
+// { dg-do compile { target c++20 } }
+
+struct S {
+  int *s;
+  constexpr S () : s(new int) {}       // { dg-error "is not a constant 
expression because allocated storage has not been deallocated" }
+  S (const S &) = delete;
+  S &operator= (const S &) = delete;
+  constexpr ~S () { delete s; }
+};
+
+constexpr bool
+foo (S v)
+{
+  v.s = nullptr;
+  return true;
+}
+
+static_assert (foo (S ()));    // { dg-error "non-constant condition for 
static assertion" }

        Jakub

Reply via email to