c++: make build_throw SFINAE-friendly [PR98388]

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                      | 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


base-commit: bfd72bb44eca83b0db2b0bab895f27a8a44247a2

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?
> 
> 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.

> @@ -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?

> +		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 55c23b642bc..1a8f9b20e30 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: bfd72bb44eca83b0db2b0bab895f27a8a44247a2
> -- 
> 2.43.0
> 
>