On 2/9/24 13:31, Marek Polacek wrote:
On Fri, Feb 09, 2024 at 10:07:33AM -0500, Jason Merrill wrote:
On 2/8/24 17:20, Marek Polacek wrote:
On Thu, Feb 08, 2024 at 04:53:45PM -0500, Jason Merrill wrote:
On 2/8/24 11:51, Marek Polacek wrote:
On Thu, Feb 08, 2024 at 08:49:28AM -0500, Patrick Palka wrote:
On Wed, 7 Feb 2024, Marek Polacek wrote:

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

-- >8 --
Here the problem is that we give hard errors while substituting
template parameters during overload resolution of is_throwable
which has an invalid throw in decltype.

The backtrace shows that fn_type_unification -> instantiate_template
-> tsubst* passes complain=0 as expected, but build_throw doesn't
have a complain parameter.  So let's add one.  Also remove a redundant
local variable which I should have removed in my P2266 patch.

But there's still something not quite clear to me.  I claim that 'b'
in the testcase should evaluate to false since the first overload ought
to have been discarded.  EDG 6.6 agrees, but clang++, msvc, and icx evaluate
it to true.  Who's right?

I think it should be true since P1155, which we implement in C++20 mode and
above (or rather, we implement the sequel P2266); since then we implicitly
move from the function parameter.

The patch looks good except that we should test this expected value.

I could add
#if __cplusplus >= 202002L
static_assert (b, "move from the function parameter");
#else
static_assert (!b, "no move from the function parameter");
#endif

but that's going to fail for C++20 and above.

Ah, because treat_lvalue_as_rvalue_p doesn't recognize t as being from the
current scope in the trailing return type.  But that shouldn't be necessary:

https://eel.is/c++draft/expr.prim.id.unqual#4.2 says it's move-eligible
"if the id-expression (possibly parenthesized) is the operand of a
throw-expression ([expr.throw]), 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."

here there is no enclosing lambda or try, so it seems move-eligible.

...and as a result, 't' will be an xvalue, therefore moveonly(moveonly&&)
should be used, therefore the first overload should be used, which means 'b'
is true.

Yep.

I wonder if this is the
second half of the problem in 113789?

I could comment the first static_assert and add a FIXME if that sounds good?

dg-bogus would be better than commenting it out.

Ok, in this patch the only change I made is:

+#if __cplusplus >= 202002L
+static_assert (b, "move from the function parameter"); // { dg-bogus "" 
"PR113853" { xfail c++20 } }
+#else
+static_assert (!b, "no move from the function parameter");
+#endif

OK.

Will you also look into fixing the treat_ bug?  That can be a separate
patch.

I'd like to.  I opened <https://gcc.gnu.org/PR113853>.

-- >8 --
Here the problem is that we give hard errors while substituting
template parameters during overload resolution of is_throwable
which has an invalid throw in decltype.

The backtrace shows that fn_type_unification -> instantiate_template
-> tsubst* passes complain=0 as expected, but build_throw doesn't
have a complain parameter.  So let's add one.  Also remove a redundant
local variable which I should have removed in my P2266 patch.

There's still one problem for which I opened <https://gcc.gnu.org/PR113853>.
We need to patch up treat_lvalue_as_rvalue_p and remove the dg-bogus.

Thanks to Patrick for notifying me of this PR.  This doesn't fully fix
113789; there I think I'll have to figure our why a candidate wasn't
discarded from the overload set.

gcc/cp/ChangeLog:

        * coroutines.cc (coro_rewrite_function_body): Pass tf_warning_or_error
        to build_throw.
        (morph_fn_to_coro): Likewise.
        * cp-tree.h (build_throw): Adjust.
        * except.cc (expand_end_catch_block): Pass tf_warning_or_error to
        build_throw.
        (build_throw): Add a tsubst_flags_t parameter.  Use it.  Remove
        redundant variable.  Guard an inform call.
        * parser.cc (cp_parser_throw_expression): Pass tf_warning_or_error
        to build_throw.
        * pt.cc (tsubst_expr) <case THROW_EXPR>: Pass complain to build_throw.

libcc1/ChangeLog:

        * libcp1plugin.cc (plugin_build_unary_expr): Pass tf_error to
        build_throw.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp0x/sfinae69.C: New test.
---
  gcc/cp/coroutines.cc                  |  4 ++--
  gcc/cp/cp-tree.h                      |  3 ++-
  gcc/cp/except.cc                      | 33 ++++++++++++---------------
  gcc/cp/parser.cc                      |  2 +-
  gcc/cp/pt.cc                          |  2 +-
  gcc/testsuite/g++.dg/cpp0x/sfinae69.C | 21 +++++++++++++++++
  libcc1/libcp1plugin.cc                |  4 ++--
  7 files changed, 43 insertions(+), 26 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae69.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 3194c911e8c..9b037edbd14 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4246,7 +4246,7 @@ coro_rewrite_function_body (location_t fn_start, tree 
fnbody, tree orig,
                                  boolean_type_node, i_a_r_c);
        finish_if_stmt_cond (not_iarc, not_iarc_if);
        /* If the initial await resume called value is false, rethrow...  */
-      tree rethrow = build_throw (fn_start, NULL_TREE);
+      tree rethrow = build_throw (fn_start, NULL_TREE, tf_warning_or_error);
        suppress_warning (rethrow);
        finish_expr_stmt (rethrow);
        finish_then_clause (not_iarc_if);
@@ -5151,7 +5151,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
        tree del_coro_fr = coro_get_frame_dtor (coro_fp, orig, frame_size,
                                              promise_type, fn_start);
        finish_expr_stmt (del_coro_fr);
-      tree rethrow = build_throw (fn_start, NULL_TREE);
+      tree rethrow = build_throw (fn_start, NULL_TREE, tf_warning_or_error);
        suppress_warning (rethrow);
        finish_expr_stmt (rethrow);
        finish_handler (handler);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 969c7239c97..334c11396c2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7185,7 +7185,8 @@ extern void init_exception_processing             (void);
  extern tree expand_start_catch_block          (tree);
  extern void expand_end_catch_block            (void);
  extern tree build_exc_ptr                     (void);
-extern tree build_throw                                (location_t, tree);
+extern tree build_throw                                (location_t, tree,
+                                                tsubst_flags_t);
  extern int nothrow_libfn_p                    (const_tree);
  extern void check_handlers                    (tree);
  extern tree finish_noexcept_expr              (tree, tsubst_flags_t);
diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
index d17a57d3fbc..ea3d6f57396 100644
--- a/gcc/cp/except.cc
+++ b/gcc/cp/except.cc
@@ -486,7 +486,8 @@ expand_end_catch_block (void)
          || DECL_DESTRUCTOR_P (current_function_decl))
        && !in_nested_catch ())
      {
-      tree rethrow = build_throw (input_location, NULL_TREE);
+      tree rethrow = build_throw (input_location, NULL_TREE,
+                                 tf_warning_or_error);
        /* Disable all warnings for the generated rethrow statement.  */
        suppress_warning (rethrow);
        finish_expr_stmt (rethrow);
@@ -607,7 +608,7 @@ wrap_cleanups_r (tree *tp, int *walk_subtrees, void * 
/*data*/)
  /* Build a throw expression.  */
tree
-build_throw (location_t loc, tree exp)
+build_throw (location_t loc, tree exp, tsubst_flags_t complain)
  {
    if (exp == error_mark_node)
      return exp;
@@ -621,7 +622,7 @@ build_throw (location_t loc, tree exp)
        return exp;
      }
- if (exp && null_node_p (exp))
+  if (exp && null_node_p (exp) && (complain & tf_warning))
      warning_at (loc, 0,
                "throwing NULL, which has integral, not pointer type");
@@ -642,8 +643,6 @@ build_throw (location_t loc, tree exp)
        tree object, ptr;
        tree allocate_expr;
- tsubst_flags_t complain = tf_warning_or_error;
-
        /* The CLEANUP_TYPE is the internal type of a destructor.  */
        if (!cleanup_type)
        {
@@ -714,7 +713,6 @@ build_throw (location_t loc, tree exp)
        if (CLASS_TYPE_P (temp_type))
        {
          int flags = LOOKUP_NORMAL | LOOKUP_ONLYCONVERTING;
-         bool converted = false;
          location_t exp_loc = cp_expr_loc_or_loc (exp, loc);
/* Under C++0x [12.8/16 class.copy], a thrown lvalue is sometimes
@@ -727,23 +725,20 @@ build_throw (location_t loc, tree exp)
            exp = moved;
/* Call the copy constructor. */
-         if (!converted)
-           {
-             releasing_vec exp_vec (make_tree_vector_single (exp));
-             exp = (build_special_member_call
-                    (object, complete_ctor_identifier, &exp_vec,
-                     TREE_TYPE (object), flags, tf_warning_or_error));
-           }
-
+         releasing_vec exp_vec (make_tree_vector_single (exp));
+         exp = build_special_member_call (object, complete_ctor_identifier,
+                                          &exp_vec, TREE_TYPE (object), flags,
+                                          complain);
          if (exp == error_mark_node)
            {
-             inform (exp_loc, "  in thrown expression");
+             if (complain & tf_error)
+               inform (exp_loc, "  in thrown expression");
              return error_mark_node;
            }
        }
        else
        {
-         tree tmp = decay_conversion (exp, tf_warning_or_error);
+         tree tmp = decay_conversion (exp, complain);
          if (tmp == error_mark_node)
            return error_mark_node;
          exp = cp_build_init_expr (object, tmp);
@@ -768,7 +763,7 @@ build_throw (location_t loc, tree exp)
          tree binfo = TYPE_BINFO (TREE_TYPE (object));
          tree dtor_fn = lookup_fnfields (binfo,
                                          complete_dtor_identifier, 0,
-                                         tf_warning_or_error);
+                                         complain);
          dtor_fn = BASELINK_FUNCTIONS (dtor_fn);
          if (!mark_used (dtor_fn)
              || !perform_or_defer_access_check (binfo, dtor_fn,
@@ -785,7 +780,7 @@ build_throw (location_t loc, tree exp)
        cleanup = build_int_cst (cleanup_type, 0);
/* ??? Indicate that this function call throws throw_type. */
-      tree tmp = cp_build_function_call_nary (throw_fn, tf_warning_or_error,
+      tree tmp = cp_build_function_call_nary (throw_fn, complain,
                                              ptr, throw_type, cleanup,
                                              NULL_TREE);
@@ -807,7 +802,7 @@ build_throw (location_t loc, tree exp) /* ??? Indicate that this function call allows exceptions of the type
         of the enclosing catch block (if known).  */
-      exp = cp_build_function_call_vec (rethrow_fn, NULL, tf_warning_or_error);
+      exp = cp_build_function_call_vec (rethrow_fn, NULL, complain);
      }
exp = build1_loc (loc, THROW_EXPR, void_type_node, exp);
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index c4292c49ce2..09ecfa23b5d 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -29310,7 +29310,7 @@ cp_parser_throw_expression (cp_parser* parser)
       the end at the end of the final token we consumed.  */
    location_t combined_loc = make_location (start_loc, start_loc,
                                           parser->lexer);
-  expression = build_throw (combined_loc, expression);
+  expression = build_throw (combined_loc, expression, tf_warning_or_error);
return expression;
  }
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index f9abb21a9a2..9c225c095c8 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -21180,7 +21180,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
case THROW_EXPR:
        RETURN (build_throw
-       (input_location, RECUR (TREE_OPERAND (t, 0))));
+       (input_location, RECUR (TREE_OPERAND (t, 0)), complain));
case CONSTRUCTOR:
        {
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae69.C 
b/gcc/testsuite/g++.dg/cpp0x/sfinae69.C
new file mode 100644
index 00000000000..361e0b79ba2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae69.C
@@ -0,0 +1,21 @@
+// PR c++/98388
+// { dg-do compile { target c++11 } }
+
+struct moveonly {
+    moveonly() = default;
+    moveonly(moveonly&&) = default;
+};
+
+template<class T>
+constexpr auto is_throwable(T t) -> decltype(throw t, true) {
+    return true;
+}
+template<class T>
+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 } }
+#else
+static_assert (!b, "no move from the function parameter");
+#endif
diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
index 7f4e4c98060..0eff7c68d29 100644
--- a/libcc1/libcp1plugin.cc
+++ b/libcc1/libcp1plugin.cc
@@ -2640,7 +2640,7 @@ plugin_build_unary_expr (cc1_plugin::connection *self,
        break;
case THROW_EXPR:
-      result = build_throw (input_location, op0);
+      result = build_throw (input_location, op0, tf_error);
        break;
case TYPEID_EXPR:
@@ -2664,7 +2664,7 @@ plugin_build_unary_expr (cc1_plugin::connection *self,
        result = make_pack_expansion (op0);
        break;
- // We're using this for sizeof...(pack). */
+      /* We're using this for sizeof...(pack).  */
      case TYPE_PACK_EXPANSION:
        result = make_pack_expansion (op0);
        PACK_EXPANSION_SIZEOF_P (result) = true;

base-commit: 41a6d2560500a202708e7b661b8b2ad432aee3a6

Reply via email to