Re: [PATCH 1/2] c++: remove function_p parm from tsubst_copy_and_build

[Jason Merrill <jason@redhat.com>]

On 11/10/22 09:56, Patrick Palka wrote:
> The function_p parameter of tsubst_copy_and_build (added in r69316) is
> inspected only in its IDENTIFIER_NODE case, where it controls whether we
> diagnose unqualified name lookup failure for the given identifier.  But
> I think ever since r173965, we never substitute an IDENTIFIER_NODE with
> function_p=true for which the lookup can possibly fail, and therefore
> the flag is effectively unneeded.
> 
> Before that commit, we would incorrectly repeat unqualified lookup for
> an ADL-enabled CALL_EXPR at instantiation time, which naturally could
> fail and thus motivated the flag.  Afterwards, we no longer substitute
> an IDENTIFIER_NODE callee when koenig_p is true so the flag isn't needed
> for its original purpose.  What about when koenig_p=false?  Apparently
> we still may have an IDENTIFIER_NODE callee in this case, namely when
> unqualified name lookup found a dependent local function declaration,
> but repeating that lookup can't fail.  (It also can't fail for USING_DECL
> callees.)
> 
> So this patch removes this effectively unneeded parameter from
> tsubst_copy_and_build.  It also updates a outdated comment in the
> CALL_EXPR case about when we may see an IDENTIFIER_NODE callee with
> koenig_p=false.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?

OK.

> gcc/cp/ChangeLog:
> 
> 	* cp-lang.cc (objcp_tsubst_copy_and_build): Remove
> 	function_p parameter.
> 	* cp-objcp-common.h (objcp_tsubst_copy_and_build):
> 	Likewise.
> 	* cp-tree.h (tsubst_copy_and_build): Likewise.
> 	* init.cc (get_nsdmi): Adjust calls to tsubst_copy_and_build.
> 	* pt.cc (expand_integer_pack): Likewise.
> 	(instantiate_non_dependent_expr_internal): Likewise.
> 	(tsubst_function_decl): Likewise.
> 	(tsubst_arg_types): Likewise.
> 	(tsubst_exception_specification): Likewise.
> 	(tsubst): Likewise.
> 	(tsubst_copy_asm_operands): Likewise.
> 	(tsubst_expr): Likewise.
> 	(tsubst_non_call_postfix_expression): Likewise.
> 	(tsubst_lambda_expr): Likewise.
> 	(tsubst_copy_and_build_call_args): Likewise.
> 	(tsubst_copy_and_build): Remove function_p parameter
> 	and adjust function comment.  Adjust recursive calls.
> 	<case CALL_EXPR>: Update outdated comment about when
> 	we can see an IDENTIFIER_NODE callee with koenig_p=false.
> 	(maybe_instantiate_noexcept): Adjust calls to
> 	tsubst_copy_and_build.
> 
> gcc/objcp/ChangeLog:
> 
> 	* objcp-lang.cc (objcp_tsubst_copy_and_build): Remove
> 	function_p parameter.
> ---
>   gcc/cp/cp-lang.cc        |  3 +--
>   gcc/cp/cp-objcp-common.h |  3 +--
>   gcc/cp/cp-tree.h         |  2 +-
>   gcc/cp/init.cc           |  2 +-
>   gcc/cp/pt.cc             | 46 ++++++++++++----------------------------
>   gcc/objcp/objcp-lang.cc  |  5 ++---
>   6 files changed, 19 insertions(+), 42 deletions(-)
> 
> diff --git a/gcc/cp/cp-lang.cc b/gcc/cp/cp-lang.cc
> index c3cfde56cc6..a3f29eda0d6 100644
> --- a/gcc/cp/cp-lang.cc
> +++ b/gcc/cp/cp-lang.cc
> @@ -116,8 +116,7 @@ tree
>   objcp_tsubst_copy_and_build (tree /*t*/,
>   			     tree /*args*/,
>   			     tsubst_flags_t /*complain*/,
> -			     tree /*in_decl*/,
> -			     bool /*function_p*/)
> +			     tree /*in_decl*/)
>   {
>     return NULL_TREE;
>   }
> diff --git a/gcc/cp/cp-objcp-common.h b/gcc/cp/cp-objcp-common.h
> index 1a67f14d9b3..f4ba0c9e012 100644
> --- a/gcc/cp/cp-objcp-common.h
> +++ b/gcc/cp/cp-objcp-common.h
> @@ -24,8 +24,7 @@ along with GCC; see the file COPYING3.  If not see
>   /* In cp/objcp-common.c, cp/cp-lang.cc and objcp/objcp-lang.cc.  */
>   
>   extern tree cp_get_debug_type (const_tree);
> -extern tree objcp_tsubst_copy_and_build (tree, tree, tsubst_flags_t,
> -					 tree, bool);
> +extern tree objcp_tsubst_copy_and_build (tree, tree, tsubst_flags_t, tree);
>   
>   extern int cp_decl_dwarf_attribute (const_tree, int);
>   extern int cp_type_dwarf_attribute (const_tree, int);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index d13bb3d4c0e..40fd2e1ebb9 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7383,7 +7383,7 @@ extern tree tsubst_default_argument		(tree, int, tree, tree,
>   						 tsubst_flags_t);
>   extern tree tsubst (tree, tree, tsubst_flags_t, tree);
>   extern tree tsubst_copy_and_build		(tree, tree, tsubst_flags_t,
> -						 tree, bool = false, bool = false);
> +						 tree, bool = false);
>   extern tree tsubst_expr                         (tree, tree, tsubst_flags_t,
>                                                    tree, bool);
>   extern tree tsubst_pack_expansion		(tree, tree, tsubst_flags_t, tree);
> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> index 3d5d3904944..fee49090de7 100644
> --- a/gcc/cp/init.cc
> +++ b/gcc/cp/init.cc
> @@ -622,7 +622,7 @@ get_nsdmi (tree member, bool in_ctor, tsubst_flags_t complain)
>   	  /* Do deferred instantiation of the NSDMI.  */
>   	  init = (tsubst_copy_and_build
>   		  (init, DECL_TI_ARGS (member),
> -		   complain, member, /*function_p=*/false,
> +		   complain, member,
>   		   /*integral_constant_expression_p=*/false));
>   	  init = digest_nsdmi_init (member, init, complain);
>   
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index c3fc56a13ff..bcc275f8b6e 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -3773,7 +3773,7 @@ expand_integer_pack (tree call, tree args, tsubst_flags_t complain,
>   {
>     tree ohi = CALL_EXPR_ARG (call, 0);
>     tree hi = tsubst_copy_and_build (ohi, args, complain, in_decl,
> -				   false/*fn*/, true/*int_cst*/);
> +				   true/*int_cst*/);
>   
>     if (instantiation_dependent_expression_p (hi))
>       {
> @@ -6361,7 +6361,6 @@ instantiate_non_dependent_expr_internal (tree expr, tsubst_flags_t complain)
>   				/*args=*/NULL_TREE,
>   				complain,
>   				/*in_decl=*/NULL_TREE,
> -				/*function_p=*/false,
>   				/*integral_constant_expression_p=*/true);
>   }
>   
> @@ -14210,7 +14209,6 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain,
>       {
>         tree spec = lookup_explicit_specifier (t);
>         spec = tsubst_copy_and_build (spec, args, complain, in_decl,
> -				    /*function_p=*/false,
>   				    /*i_c_e_p=*/true);
>         spec = build_explicit_specifier (spec, complain);
>         if (spec == error_mark_node)
> @@ -15276,7 +15274,7 @@ tsubst_arg_types (tree arg_types,
>         || (in_decl && TREE_CODE (in_decl) == FUNCTION_DECL
>   	  && DECL_LOCAL_DECL_P (in_decl)))
>       default_arg = tsubst_copy_and_build (default_arg, args, complain, in_decl,
> -					 false/*fn*/, false/*constexpr*/);
> +					 false/*constexpr*/);
>   
>     tree remaining_arg_types = tsubst_arg_types (TREE_CHAIN (arg_types),
>   					       args, end, complain, in_decl);
> @@ -15458,7 +15456,7 @@ tsubst_exception_specification (tree fntype,
>   	      expr = DEFERRED_NOEXCEPT_PATTERN (expr);
>   	    }
>   	  new_specs = tsubst_copy_and_build
> -	    (expr, args, complain, in_decl, /*function_p=*/false,
> +	    (expr, args, complain, in_decl,
>   	     /*integral_constant_expression_p=*/true);
>   	}
>         new_specs = build_noexcept_spec (new_specs, complain);
> @@ -16383,7 +16381,6 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   
>   	type = tsubst_copy_and_build (DECLTYPE_TYPE_EXPR (t), args,
>   				      complain|tf_decltype, in_decl,
> -				      /*function_p*/false,
>   				      /*integral_constant_expression*/false);
>   
>   	--cp_unevaluated_operand;
> @@ -18049,7 +18046,6 @@ tsubst_copy_asm_operands (tree t, tree args, tsubst_flags_t complain,
>   
>     if (TREE_CODE (t) != TREE_LIST)
>       return tsubst_copy_and_build (t, args, complain, in_decl,
> -				  /*function_p=*/false,
>   				  /*integral_constant_expression_p=*/false);
>   
>     if (t == void_list_node)
> @@ -19636,7 +19632,6 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
>         gcc_assert (!STATEMENT_CODE_P (TREE_CODE (t)));
>   
>         RETURN (tsubst_copy_and_build (t, args, complain, in_decl,
> -				    /*function_p=*/false,
>   				    integral_constant_expression_p));
>       }
>   
> @@ -19729,7 +19724,6 @@ tsubst_non_call_postfix_expression (tree t, tree args,
>   			     /*done=*/false, /*address_p=*/false);
>     else
>       t = tsubst_copy_and_build (t, args, complain, in_decl,
> -			       /*function_p=*/false,
>   			       /*integral_constant_expression_p=*/false);
>   
>     return t;
> @@ -19802,7 +19796,7 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   	init = tsubst_pack_expansion (init, args, complain, in_decl);
>         else
>   	init = tsubst_copy_and_build (init, args, complain, in_decl,
> -				      /*fn*/false, /*constexpr*/false);
> +				      /*constexpr*/false);
>   
>         if (init == error_mark_node)
>   	return error_mark_node;
> @@ -20070,7 +20064,6 @@ tsubst_copy_and_build_call_args (tree t, tree args, tsubst_flags_t complain,
>         if (!PACK_EXPANSION_P (arg))
>   	vec_safe_push (call_args,
>   		       tsubst_copy_and_build (arg, args, complain, in_decl,
> -					      /*function_p=*/false,
>   					      integral_constant_expression_p));
>         else
>   	{
> @@ -20097,21 +20090,18 @@ tsubst_copy_and_build_call_args (tree t, tree args, tsubst_flags_t complain,
>   }
>   
>   /* Like tsubst but deals with expressions and performs semantic
> -   analysis.  FUNCTION_P is true if T is the "F" in "F (ARGS)" or
> -   "F<TARGS> (ARGS)".  */
> +   analysis.  */
>   
>   tree
>   tsubst_copy_and_build (tree t,
>   		       tree args,
>   		       tsubst_flags_t complain,
>   		       tree in_decl,
> -		       bool function_p,
>   		       bool integral_constant_expression_p)
>   {
>   #define RETURN(EXP) do { retval = (EXP); goto out; } while(0)
>   #define RECUR(NODE)						\
>     tsubst_copy_and_build (NODE, args, complain, in_decl, 	\
> -			 /*function_p=*/false,			\
>   			 integral_constant_expression_p)
>   
>     tree retval, op1;
> @@ -20168,7 +20158,7 @@ tsubst_copy_and_build (tree t,
>   				     input_location);
>   	if (error_msg)
>   	  error (error_msg);
> -	if (!function_p && identifier_p (decl))
> +	if (identifier_p (decl))
>   	  {
>   	    if (complain & tf_error)
>   	      unqualified_name_lookup_error (decl);
> @@ -20182,7 +20172,6 @@ tsubst_copy_and_build (tree t,
>   	tree object;
>   	tree templ = tsubst_copy_and_build (TREE_OPERAND (t, 0), args,
>   					    complain, in_decl,
> -					    function_p,
>   					    integral_constant_expression_p);
>   	tree targs = TREE_OPERAND (t, 1);
>   
> @@ -20565,7 +20554,6 @@ tsubst_copy_and_build (tree t,
>   	      op1 = tsubst (op1, args, complain, in_decl);
>   	    else
>   	      op1 = tsubst_copy_and_build (op1, args, complain, in_decl,
> -					   /*function_p=*/false,
>   					   /*integral_constant_expression_p=*/
>   					   false);
>   	    --cp_unevaluated_operand;
> @@ -20605,7 +20593,6 @@ tsubst_copy_and_build (tree t,
>   	++cp_unevaluated_operand;
>   	++c_inhibit_evaluation_warnings;
>   	op1 = tsubst_copy_and_build (op1, args, complain, in_decl,
> -				     /*function_p=*/false,
>   				     /*integral_constant_expression_p=*/false);
>   	--cp_unevaluated_operand;
>   	--c_inhibit_evaluation_warnings;
> @@ -20618,7 +20605,6 @@ tsubst_copy_and_build (tree t,
>         ++c_inhibit_evaluation_warnings;
>         ++cp_noexcept_operand;
>         op1 = tsubst_copy_and_build (op1, args, complain, in_decl,
> -				   /*function_p=*/false,
>   				   /*integral_constant_expression_p=*/false);
>         --cp_unevaluated_operand;
>         --c_inhibit_evaluation_warnings;
> @@ -20729,7 +20715,6 @@ tsubst_copy_and_build (tree t,
>         {
>   	tree op0 = tsubst_copy_and_build (TREE_OPERAND (t, 0), args,
>   					  complain & ~tf_decltype, in_decl,
> -					  /*function_p=*/false,
>   					  integral_constant_expression_p);
>   	RETURN (build_x_compound_expr (EXPR_LOCATION (t),
>   				       op0,
> @@ -20775,12 +20760,10 @@ tsubst_copy_and_build (tree t,
>   	       would incorrectly perform unqualified lookup again.
>   
>   	       Note that we can also have an IDENTIFIER_NODE if the earlier
> -	       unqualified lookup found a member function; in that case
> -	       koenig_p will be false and we do want to do the lookup
> -	       again to find the instantiated member function.
> -
> -	       FIXME but doing that causes c++/15272, so we need to stop
> -	       using IDENTIFIER_NODE in that situation.  */
> +	       unqualified lookup found a dependent local extern declaration
> +	       (as per finish_call_expr); in that case koenig_p will be false
> +	       and we do want to do the lookup again to find the substituted
> +	       declaration.  */
>   	    qualified_p = false;
>   
>   	    if (TREE_CODE (function) == TEMPLATE_ID_EXPR)
> @@ -20815,7 +20798,6 @@ tsubst_copy_and_build (tree t,
>   	      subcomplain |= tf_conv;
>   	    function = tsubst_copy_and_build (function, args, subcomplain,
>   					      in_decl,
> -					      !qualified_p,
>   					      integral_constant_expression_p);
>   
>   	    if (BASELINK_P (function))
> @@ -20919,7 +20901,7 @@ tsubst_copy_and_build (tree t,
>   		   the unqualified lookup again if we aren't in SFINAE
>   		   context.  */
>   		tree unq = (tsubst_copy_and_build
> -			    (function, args, complain, in_decl, true,
> +			    (function, args, complain, in_decl,
>   			     integral_constant_expression_p));
>   		if (unq == error_mark_node)
>   		  RETURN (error_mark_node);
> @@ -21469,7 +21451,7 @@ tsubst_copy_and_build (tree t,
>         {
>   	tree object_ptr
>   	  = tsubst_copy_and_build (TREE_OPERAND (t, 1), args, complain,
> -				   in_decl, /*function_p=*/false,
> +				   in_decl,
>   				   /*integral_constant_expression_p=*/false);
>   	RETURN (finish_offsetof (object_ptr,
>   				 RECUR (TREE_OPERAND (t, 0)),
> @@ -21571,8 +21553,7 @@ tsubst_copy_and_build (tree t,
>         /* Handle Objective-C++ constructs, if appropriate.  */
>         {
>   	tree subst
> -	  = objcp_tsubst_copy_and_build (t, args, complain,
> -					 in_decl, /*function_p=*/false);
> +	  = objcp_tsubst_copy_and_build (t, args, complain, in_decl);
>   	if (subst)
>   	  RETURN (subst);
>         }
> @@ -26389,7 +26370,6 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t complain)
>   	  noex = tsubst_copy_and_build (DEFERRED_NOEXCEPT_PATTERN (noex),
>   					DEFERRED_NOEXCEPT_ARGS (noex),
>   					tf_warning_or_error, fn,
> -					/*function_p=*/false,
>   					/*i_c_e_p=*/true);
>   
>   	  /* Build up the noexcept-specification.  */
> diff --git a/gcc/objcp/objcp-lang.cc b/gcc/objcp/objcp-lang.cc
> index 2e8809b865d..5f0e22942b8 100644
> --- a/gcc/objcp/objcp-lang.cc
> +++ b/gcc/objcp/objcp-lang.cc
> @@ -50,12 +50,11 @@ struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>      there should be very few (if any) routines below.  */
>   
>   tree
> -objcp_tsubst_copy_and_build (tree t, tree args, tsubst_flags_t complain,
> -			     tree in_decl, bool function_p ATTRIBUTE_UNUSED)
> +objcp_tsubst_copy_and_build (tree t, tree args, tsubst_flags_t complain,
> +			     tree in_decl)
>   {
>   #define RECURSE(NODE)							\
>     tsubst_copy_and_build (NODE, args, complain, in_decl, 		\
> -			 /*function_p=*/false,				\
>   			 /*integral_constant_expression_p=*/false)
>   
>     /* The following two can only occur in Objective-C++.  */