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.

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                      | 31 +++++++++++----------------
  gcc/cp/parser.cc                      |  2 +-
  gcc/cp/pt.cc                          |  2 +-
  gcc/testsuite/g++.dg/cpp0x/sfinae69.C | 16 ++++++++++++++
  libcc1/libcp1plugin.cc                |  4 ++--
  7 files changed, 37 insertions(+), 25 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..ed704b6c28a 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;

The "throwing NULL, which has integral, not pointer type" warning near
the top of build_throw should probably be guarded by tf_warning now.

Ah, I missed that.  Fixed.
@@ -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_warning)

tf_error instead of tf_warning for this note?

Not sure it makes a difference in practice, but I've changed it.  Thanks,

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?

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 | 16 +++++++++++++
  libcc1/libcp1plugin.cc                |  4 ++--
  7 files changed, 38 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..c4896050b9d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae69.C
@@ -0,0 +1,16 @@
+// 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{});
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: 5fb204aaf34b68c427f5b2bfb933fed72fe3eafb

Reply via email to