On 2/16/24 10:58, Marek Polacek wrote:
On Thu, Feb 15, 2024 at 04:36:40PM -0500, Jason Merrill wrote:
On 2/15/24 10:19, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Here we have

    template<class T>
    auto is_throwable(T t) -> decltype(throw t, true) { ... }

where we didn't properly mark 't' as IMPLICIT_RVALUE_P, which caused
the wrong overload to have been chosen.  Jason figured out it's because
we don't correctly implement [expr.prim.id.unqual]#4.2, which post-P2266
says that an id-expression is move-eligible if

"the id-expression (possibly parenthesized) is the operand of
a throw-expression, and names an implicitly movable entity that belongs
to a scope that does not contain the compound-statement of the innermost
lambda-expression, try-block, or function-try-block (if any) whose
compound-statement or ctor-initializer contains the throw-expression."

I worked out that it's trying to say that given

    struct X {
      X();
      X(const X&);
      X(X&&) = delete;
    };

the following should fail: the scope of the throw is an sk_try, and it's
also x's scope S, and S "does not contain the compound-statement of the
*try-block" so x is move-eligible, so we move, so we fail.

    void f ()
    try {
      X x;
      throw x;  // use of deleted function
    } catch (...) {
    }

Whereas here:

    void g (X x)
    try {
      throw x;
    } catch (...) {
    }

the throw is again in an sk_try, but x's scope is an sk_function_parms
which *does* contain the {} of the *try-block, so x is not move-eligible,
so we don't move, so we use X(const X&), and the code is fine.

The current code also doesn't seem to handle

    void h (X x) {
      void z (decltype(throw x, true));
    }

where there's no enclosing lambda or sk_try so we should move.

I'm not doing anything about lambdas because we shouldn't reach the
code at the end of the function: the DECL_HAS_VALUE_EXPR_P check
shouldn't let us go further.

        PR c++/113789
        PR c++/113853

gcc/cp/ChangeLog:

        * typeck.cc (treat_lvalue_as_rvalue_p): Update code to better
        reflect [expr.prim.id.unqual]#4.2.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp0x/sfinae69.C: Remove dg-bogus.
        * g++.dg/cpp0x/sfinae70.C: New test.
        * g++.dg/cpp0x/sfinae71.C: New test.
        * g++.dg/cpp0x/sfinae72.C: New test.
        * g++.dg/cpp2a/implicit-move4.C: New test.
---
   gcc/cp/typeck.cc                            | 32 +++++------
   gcc/testsuite/g++.dg/cpp0x/sfinae69.C       |  2 +-
   gcc/testsuite/g++.dg/cpp0x/sfinae70.C       | 16 ++++++
   gcc/testsuite/g++.dg/cpp0x/sfinae71.C       | 17 ++++++
   gcc/testsuite/g++.dg/cpp0x/sfinae72.C       | 17 ++++++
   gcc/testsuite/g++.dg/cpp2a/implicit-move4.C | 59 +++++++++++++++++++++
   6 files changed, 126 insertions(+), 17 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae70.C
   create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae71.C
   create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae72.C
   create mode 100644 gcc/testsuite/g++.dg/cpp2a/implicit-move4.C

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 132c55cfc6d..0dc44cd87ca 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10837,37 +10837,37 @@ treat_lvalue_as_rvalue_p (tree expr, bool return_p)
        parenthesized) id-expression that names an implicitly movable entity
        declared in the body or parameter-declaration-clause of the innermost
        enclosing function or lambda-expression, */
-  if (DECL_CONTEXT (retval) != current_function_decl)
-    return NULL_TREE;
     if (return_p)
       {
+      if (DECL_CONTEXT (retval) != current_function_decl)
+       return NULL_TREE;
         expr = move (expr);
         if (expr == error_mark_node)
        return NULL_TREE;
         return set_implicit_rvalue_p (expr);
       }
-  /* if the operand of a throw-expression is a (possibly parenthesized)
-     id-expression that names an implicitly movable entity whose scope does not
-     extend beyond the compound-statement of the innermost try-block or
-     function-try-block (if any) whose compound-statement or ctor-initializer
-     encloses the throw-expression, */
+  /* if the id-expression (possibly parenthesized) is the operand of
+     a throw-expression, and names an implicitly movable entity that belongs
+     to a scope that does not contain the compound-statement of the innermost
+     lambda-expression, try-block, or function-try-block (if any) whose
+     compound-statement or ctor-initializer contains the throw-expression.  */

Please also mention here why you aren't checking for lambdas.

Done.

     /* C++20 added move on throw of parms.  */
     if (TREE_CODE (retval) == PARM_DECL && cxx_dialect < cxx20)
       return NULL_TREE;
     for (cp_binding_level *b = current_binding_level;
-       ; b = b->level_chain)
-    {
-      for (tree decl = b->names; decl; decl = TREE_CHAIN (decl))
-       if (decl == retval)
-         return set_implicit_rvalue_p (move (expr));
-      if (b->kind == sk_function_parms
-         || b->kind == sk_try
-         || b->kind == sk_namespace)
+       b->kind != sk_namespace; b = b->level_chain)
+    if (b->kind == sk_try)
+      {
+       for (tree decl = b->names; decl; decl = TREE_CHAIN (decl))
+         if (decl == retval)
+           return set_implicit_rvalue_p (move (expr));
        return NULL_TREE;
-    }

What if the variable is in a block nested within the try-block?  I think we
still want to walk b->names for every inner binding level.

Oy, that's a bug.  Fixed by moving the == sk_try check below.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

-- >8 --
Here we have

   template<class T>
   auto is_throwable(T t) -> decltype(throw t, true) { ... }

where we didn't properly mark 't' as IMPLICIT_RVALUE_P, which caused
the wrong overload to have been chosen.  Jason figured out it's because
we don't correctly implement [expr.prim.id.unqual]#4.2, which post-P2266
says that an id-expression is move-eligible if

"the id-expression (possibly parenthesized) is the operand of
a throw-expression, and names an implicitly movable entity that belongs
to a scope that does not contain the compound-statement of the innermost
lambda-expression, try-block, or function-try-block (if any) whose
compound-statement or ctor-initializer contains the throw-expression."

I worked out that it's trying to say that given

   struct X {
     X();
     X(const X&);
     X(X&&) = delete;
   };

the following should fail: the scope of the throw is an sk_try, and it's
also x's scope S, and S "does not contain the compound-statement of the
*try-block" so x is move-eligible, so we move, so we fail.

   void f ()
   try {
     X x;
     throw x;  // use of deleted function
   } catch (...) {
   }

Whereas here:

   void g (X x)
   try {
     throw x;
   } catch (...) {
   }

the throw is again in an sk_try, but x's scope is an sk_function_parms
which *does* contain the {} of the *try-block, so x is not move-eligible,
so we don't move, so we use X(const X&), and the code is fine.

The current code also doesn't seem to handle

   void h (X x) {
     void z (decltype(throw x, true));
   }

where there's no enclosing lambda or sk_try so we should move.

I'm not doing anything about lambdas because we shouldn't reach the
code at the end of the function: the DECL_HAS_VALUE_EXPR_P check
shouldn't let us go further.

        PR c++/113789
        PR c++/113853

gcc/cp/ChangeLog:

        * typeck.cc (treat_lvalue_as_rvalue_p): Update code to better
        reflect [expr.prim.id.unqual]#4.2.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp0x/sfinae69.C: Remove dg-bogus.
        * g++.dg/cpp0x/sfinae70.C: New test.
        * g++.dg/cpp0x/sfinae71.C: New test.
        * g++.dg/cpp0x/sfinae72.C: New test.
        * g++.dg/cpp2a/implicit-move4.C: New test.
---
  gcc/cp/typeck.cc                            | 24 +++---
  gcc/testsuite/g++.dg/cpp0x/sfinae69.C       |  2 +-
  gcc/testsuite/g++.dg/cpp0x/sfinae70.C       | 16 ++++
  gcc/testsuite/g++.dg/cpp0x/sfinae71.C       | 17 ++++
  gcc/testsuite/g++.dg/cpp0x/sfinae72.C       | 17 ++++
  gcc/testsuite/g++.dg/cpp2a/implicit-move4.C | 88 +++++++++++++++++++++
  6 files changed, 152 insertions(+), 12 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae70.C
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae71.C
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae72.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/implicit-move4.C

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 132c55cfc6d..f5a0a2273be 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10837,37 +10837,39 @@ treat_lvalue_as_rvalue_p (tree expr, bool return_p)
       parenthesized) id-expression that names an implicitly movable entity
       declared in the body or parameter-declaration-clause of the innermost
       enclosing function or lambda-expression, */
-  if (DECL_CONTEXT (retval) != current_function_decl)
-    return NULL_TREE;
    if (return_p)
      {
+      if (DECL_CONTEXT (retval) != current_function_decl)
+       return NULL_TREE;
        expr = move (expr);
        if (expr == error_mark_node)
        return NULL_TREE;
        return set_implicit_rvalue_p (expr);
      }
- /* if the operand of a throw-expression is a (possibly parenthesized)
-     id-expression that names an implicitly movable entity whose scope does not
-     extend beyond the compound-statement of the innermost try-block or
-     function-try-block (if any) whose compound-statement or ctor-initializer
-     encloses the throw-expression, */
+  /* if the id-expression (possibly parenthesized) is the operand of
+     a throw-expression, and names an implicitly movable entity that belongs
+     to a scope that does not contain the compound-statement of the innermost
+     lambda-expression, try-block, or function-try-block (if any) whose
+     compound-statement or ctor-initializer contains the throw-expression.  */
/* C++20 added move on throw of parms. */
    if (TREE_CODE (retval) == PARM_DECL && cxx_dialect < cxx20)
      return NULL_TREE;
+ /* We don't check for lambda-expression here, because we should not get past
+     the DECL_HAS_VALUE_EXPR_P check above.  */
    for (cp_binding_level *b = current_binding_level;
-       ; b = b->level_chain)
+       b->kind != sk_namespace; b = b->level_chain)
      {
        for (tree decl = b->names; decl; decl = TREE_CHAIN (decl))
        if (decl == retval)
          return set_implicit_rvalue_p (move (expr));
-      if (b->kind == sk_function_parms
-         || b->kind == sk_try
-         || b->kind == sk_namespace)
+      if (b->kind == sk_try)
        return NULL_TREE;
      }
+
+  return set_implicit_rvalue_p (move (expr));
  }
/* Warn about dubious usage of std::move (in a return statement, if RETURN_P
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae69.C 
b/gcc/testsuite/g++.dg/cpp0x/sfinae69.C
index 361e0b79ba2..60eba61165d 100644
--- a/gcc/testsuite/g++.dg/cpp0x/sfinae69.C
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae69.C
@@ -15,7 +15,7 @@ constexpr bool is_throwable(...) { return false; }
constexpr bool b = is_throwable<moveonly>(moveonly{});
  #if __cplusplus >= 202002L
-static_assert (b, "move from the function parameter"); // { dg-bogus "" 
"PR113853" { xfail c++20 } }
+static_assert (b, "move from the function parameter");
  #else
  static_assert (!b, "no move from the function parameter");
  #endif
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae70.C 
b/gcc/testsuite/g++.dg/cpp0x/sfinae70.C
new file mode 100644
index 00000000000..48ea70a61a4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae70.C
@@ -0,0 +1,16 @@
+// PR c++/113789
+// { dg-do compile { target c++11 } }
+
+struct AutoPtr {
+    AutoPtr() = default;
+    AutoPtr(AutoPtr&) {}
+};
+
+template<class T> auto f(T p, int) -> decltype(throw p, 1) = delete;
+template<class T> void f(T p, long);
+
+void
+g ()
+{
+  f (AutoPtr (), 42); // { dg-error "use of deleted function" "" { target 
c++20_down } }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae71.C 
b/gcc/testsuite/g++.dg/cpp0x/sfinae71.C
new file mode 100644
index 00000000000..2fefe5f70e0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae71.C
@@ -0,0 +1,17 @@
+// PR c++/113789
+// { dg-do compile { target c++11 } }
+// Like sfinae70.C but T&&.
+
+struct AutoPtr {
+    AutoPtr() = default;
+    AutoPtr(AutoPtr&) {}
+};
+
+template<class T> auto f(T&& p, int) -> decltype(throw p, 1) = delete;
+template<class T> void f(T p, long);
+
+void
+g ()
+{
+  f (AutoPtr (), 42); // { dg-error "use of deleted function" "" { target 
c++20_down } }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae72.C 
b/gcc/testsuite/g++.dg/cpp0x/sfinae72.C
new file mode 100644
index 00000000000..397cdbd8c23
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae72.C
@@ -0,0 +1,17 @@
+// PR c++/113789
+// { dg-do compile { target c++11 } }
+// Like sfinae70.C but ().
+
+struct AutoPtr {
+    AutoPtr() = default;
+    AutoPtr(AutoPtr&) {}
+};
+
+template<class T> auto f(T p, int) -> decltype(throw (p), 1) = delete;
+template<class T> void f(T p, long);
+
+void
+g ()
+{
+  f (AutoPtr (), 42); // { dg-error "use of deleted function" "" { target 
c++20_down } }
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/implicit-move4.C 
b/gcc/testsuite/g++.dg/cpp2a/implicit-move4.C
new file mode 100644
index 00000000000..09ac5c93ab9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/implicit-move4.C
@@ -0,0 +1,88 @@
+// PR c++/113853
+// { dg-do compile { target c++20 } }
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&) = delete;
+};
+
+void
+f1 ()
+{
+  try {
+    ;
+  } catch (X x) {
+    throw x; // { dg-error "use of deleted function" }
+  }
+}
+
+void
+f2 (X x)
+try {
+  ;
+} catch (...) {
+  throw x;  // { dg-error "use of deleted function" }
+}
+
+void
+f2b (X x)
+try {
+  ;
+} catch (...) {
+  {
+    throw x;  // { dg-error "use of deleted function" }
+  }
+}
+
+void
+f3 ()
+try {
+  X x;
+  throw x;  // { dg-error "use of deleted function" }
+} catch (...) {
+}
+
+void
+f3b ()
+try {
+  {
+    X x;
+    throw x;  // { dg-error "use of deleted function" }
+  }
+} catch (...) {
+}
+
+void
+f4 (X x)
+try {
+  throw x;
+} catch (...) {
+}
+
+void
+f4b (X x)
+try {
+  {
+    throw x;
+  }
+} catch (...) {
+}
+
+void
+f5 (X x)
+{
+  void g (decltype(throw x, true));    // { dg-error "use of deleted 
function|expected" }
+}
+
+// The "expected" shouldn't be here, c++/113924.
+void
+f6 (X x, int = decltype(throw x, true){}) // { dg-error "use of deleted 
function|expected" }
+{
+}
+
+void
+f7 (X x)
+{
+  [&] { throw x; };
+}

base-commit: 7c6071a66f32f43cea7aa4aa32d89b338e768307

Reply via email to