c++: equivalence of non-dependent calls [PR107461]

  After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of FUNCTION_DECL.
This innocent change revealed that cp_tree_equal doesn't first check
dependentness of a CALL_EXPR before treating the callee as a dependent
name, which manifests as us incorrectly accepting the first two
testcases below and rejecting the third:

 * In the first testcase, cp_tree_equal incorrectly returns true for
   the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
   are different FUNCTION_DECLs) and so we treat #2 as a redeclaration
   of #1.

 * Same issue in the second testcase, for f<int*>() and f<char>().

 * In the third testcase, cp_tree_equal incorrectly returns true for
   f<int>() and f<void(*)(int)>() which causes us to conflate the two
   dependent specializations A<decltype(f<int>()(U()))> and
   A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.

This patch fixes this by making called_fns_equal treat two callees as
dependent names only if the CALL_EXPRs in question are dependent.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk/12?  Patch generated with -w to ignore noisy whitespace changes.

	PR c++/107461

gcc/cp/ChangeLog:

	* pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Treat
	the callee as a dependent name only if the CALL_EXPR is
	dependent.
	* tree.cc (called_fns_equal): Take two CALL_EXPRs instead of
	CALL_EXPR_FNs thereof.  As above.
	(cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/overload5.C: New test.
	* g++.dg/cpp0x/overload5a.C: New test.
	* g++.dg/cpp0x/overload6.C: New test.
---
 gcc/cp/pt.cc                            |  1 +
 gcc/cp/tree.cc                          | 33 ++++++++++++++-----------
 gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 +++++++++
 gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++
 gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++
 5 files changed, 58 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C
  

On 2/4/23 15:31, Patrick Palka wrote:
> After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
> CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of FUNCTION_DECL.
> This innocent change revealed that cp_tree_equal doesn't first check
> dependentness of a CALL_EXPR before treating the callee as a dependent
> name, which manifests as us incorrectly accepting the first two
> testcases below and rejecting the third:
> 
>   * In the first testcase, cp_tree_equal incorrectly returns true for
>     the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
>     are different FUNCTION_DECLs) and so we treat #2 as a redeclaration
>     of #1.
> 
>   * Same issue in the second testcase, for f<int*>() and f<char>().
> 
>   * In the third testcase, cp_tree_equal incorrectly returns true for
>     f<int>() and f<void(*)(int)>() which causes us to conflate the two
>     dependent specializations A<decltype(f<int>()(U()))> and
>     A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.
> 
> This patch fixes this by making called_fns_equal treat two callees as
> dependent names only if the CALL_EXPRs in question are dependent.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/12?  Patch generated with -w to ignore noisy whitespace changes.
> 
> 	PR c++/107461
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Treat
> 	the callee as a dependent name only if the CALL_EXPR is
> 	dependent.
> 	* tree.cc (called_fns_equal): Take two CALL_EXPRs instead of
> 	CALL_EXPR_FNs thereof.  As above.
> 	(cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/overload5.C: New test.
> 	* g++.dg/cpp0x/overload5a.C: New test.
> 	* g++.dg/cpp0x/overload6.C: New test.
> ---
>   gcc/cp/pt.cc                            |  1 +
>   gcc/cp/tree.cc                          | 33 ++++++++++++++-----------
>   gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 +++++++++
>   gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++
>   gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++
>   5 files changed, 58 insertions(+), 14 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 255332dc0c1..c9360240cd2 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -1841,6 +1841,7 @@ iterative_hash_template_arg (tree arg, hashval_t val)
>       case CALL_EXPR:
>         {
>   	tree fn = CALL_EXPR_FN (arg);
> +	if (TREE_TYPE (arg) == NULL_TREE)

How about changing dependent_name to take the CALL_EXPR rather than the 
CALL_EXPR_FN?  That would mean some changes to write_expression to move 
the dependent_name handling into the CALL_EXPR handling, but that 
doesn't seem like a bad thing.  Other callers seem like a trivial change.

>   	  if (tree name = dependent_name (fn))
>   	    {
>   	      if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index c1da868732b..3a57e71b76e 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -3870,16 +3870,21 @@ decl_internal_context_p (const_tree decl)
>     return !TREE_PUBLIC (decl);
>   }
>   
> -/* Subroutine of cp_tree_equal: t1 and t2 are the CALL_EXPR_FNs of two
> -   CALL_EXPRS.  Return whether they are equivalent.  */
> +/* Subroutine of cp_tree_equal: t1 and t2 are two CALL_EXPRs.
> +   Return whether their CALL_EXPR_FNs are equivalent.  */
>   
>   static bool
>   called_fns_equal (tree t1, tree t2)
> +{
> +  tree fn1 = CALL_EXPR_FN (t1);
> +  tree fn2 = CALL_EXPR_FN (t2);
> +  if (TREE_TYPE (t1) == NULL_TREE
> +      && TREE_TYPE (t2) == NULL_TREE)
>       {
>         /* Core 1321: dependent names are equivalent even if the overload sets
>   	 are different.  But do compare explicit template arguments.  */
> -  tree name1 = dependent_name (t1);
> -  tree name2 = dependent_name (t2);
> +      tree name1 = dependent_name (fn1);
> +      tree name2 = dependent_name (fn2);
>         if (name1 || name2)
>   	{
>   	  tree targs1 = NULL_TREE, targs2 = NULL_TREE;
> @@ -3891,19 +3896,19 @@ called_fns_equal (tree t1, tree t2)
>   	     of whether the function was named with a qualified- or unqualified-id.
>   	     Until that's fixed, check that we aren't looking at overload sets from
>   	     different scopes.  */
> -      if (is_overloaded_fn (t1) && is_overloaded_fn (t2)
> -	  && (DECL_CONTEXT (get_first_fn (t1))
> -	      != DECL_CONTEXT (get_first_fn (t2))))
> +	  if (is_overloaded_fn (fn1) && is_overloaded_fn (fn2)
> +	      && (DECL_CONTEXT (get_first_fn (fn1))
> +		  != DECL_CONTEXT (get_first_fn (fn2))))
>   	    return false;
>   
> -      if (TREE_CODE (t1) == TEMPLATE_ID_EXPR)
> -	targs1 = TREE_OPERAND (t1, 1);
> -      if (TREE_CODE (t2) == TEMPLATE_ID_EXPR)
> -	targs2 = TREE_OPERAND (t2, 1);
> +	  if (TREE_CODE (fn1) == TEMPLATE_ID_EXPR)
> +	    targs1 = TREE_OPERAND (fn1, 1);
> +	  if (TREE_CODE (fn2) == TEMPLATE_ID_EXPR)
> +	    targs2 = TREE_OPERAND (fn2, 1);
>   	  return cp_tree_equal (targs1, targs2);
>   	}
> -  else
> -    return cp_tree_equal (t1, t2);
> +    }
> +  return cp_tree_equal (fn1, fn2);
>   }
>   
>   bool comparing_override_contracts;
> @@ -4037,7 +4042,7 @@ cp_tree_equal (tree t1, tree t2)
>   	if (KOENIG_LOOKUP_P (t1) != KOENIG_LOOKUP_P (t2))
>   	  return false;
>   
> -	if (!called_fns_equal (CALL_EXPR_FN (t1), CALL_EXPR_FN (t2)))
> +	if (!called_fns_equal (t1, t2))
>   	  return false;
>   
>   	call_expr_arg_iterator iter1, iter2;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5.C b/gcc/testsuite/g++.dg/cpp0x/overload5.C
> new file mode 100644
> index 00000000000..e05b1594f51
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/overload5.C
> @@ -0,0 +1,12 @@
> +// PR c++/107461
> +// { dg-do compile { target c++11 } }
> +
> +int f(...);
> +template<class T> decltype(T() + f(0)) g(); // #1
> +
> +char f(int);
> +template<class T> decltype(T() + f(0)) g(); // #2, distinct from #1
> +
> +int main() {
> +  g<int>(); // { dg-error "ambiguous" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5a.C b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
> new file mode 100644
> index 00000000000..037114f199c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
> @@ -0,0 +1,10 @@
> +// PR c++/107461
> +// { dg-do compile { target c++11 } }
> +
> +template<class T> T f();
> +template<class T> decltype(T() + f<int*>()) g(); // #1
> +template<class T> decltype(T() + f<char>()) g(); // #2, distinct from #1
> +
> +int main() {
> +  g<int>(); // { dg-error "ambiguous" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload6.C b/gcc/testsuite/g++.dg/cpp0x/overload6.C
> new file mode 100644
> index 00000000000..1fbee0501de
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/overload6.C
> @@ -0,0 +1,16 @@
> +// PR c++/107461
> +// { dg-do compile { target c++11 } }
> +
> +template<class T> T f();
> +
> +template<class> struct A { };
> +
> +template<class T> struct B {
> +  template<class U, class = A<decltype(f<T>()(U()))>>
> +  static void g(U);
> +};
> +
> +int main() {
> +  B<int> b;
> +  B<void(*)(int)>::g(0); // { dg-bogus "no match" }
> +}
  

On Sat, 4 Feb 2023, Jason Merrill wrote:

> On 2/4/23 15:31, Patrick Palka wrote:
> > After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
> > CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of FUNCTION_DECL.
> > This innocent change revealed that cp_tree_equal doesn't first check
> > dependentness of a CALL_EXPR before treating the callee as a dependent
> > name, which manifests as us incorrectly accepting the first two
> > testcases below and rejecting the third:
> > 
> >   * In the first testcase, cp_tree_equal incorrectly returns true for
> >     the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
> >     are different FUNCTION_DECLs) and so we treat #2 as a redeclaration
> >     of #1.
> > 
> >   * Same issue in the second testcase, for f<int*>() and f<char>().
> > 
> >   * In the third testcase, cp_tree_equal incorrectly returns true for
> >     f<int>() and f<void(*)(int)>() which causes us to conflate the two
> >     dependent specializations A<decltype(f<int>()(U()))> and
> >     A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.
> > 
> > This patch fixes this by making called_fns_equal treat two callees as
> > dependent names only if the CALL_EXPRs in question are dependent.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk/12?  Patch generated with -w to ignore noisy whitespace changes.
> > 
> > 	PR c++/107461
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Treat
> > 	the callee as a dependent name only if the CALL_EXPR is
> > 	dependent.
> > 	* tree.cc (called_fns_equal): Take two CALL_EXPRs instead of
> > 	CALL_EXPR_FNs thereof.  As above.
> > 	(cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp0x/overload5.C: New test.
> > 	* g++.dg/cpp0x/overload5a.C: New test.
> > 	* g++.dg/cpp0x/overload6.C: New test.
> > ---
> >   gcc/cp/pt.cc                            |  1 +
> >   gcc/cp/tree.cc                          | 33 ++++++++++++++-----------
> >   gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 +++++++++
> >   gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++
> >   gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++
> >   5 files changed, 58 insertions(+), 14 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C
> > 
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 255332dc0c1..c9360240cd2 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -1841,6 +1841,7 @@ iterative_hash_template_arg (tree arg, hashval_t val)
> >       case CALL_EXPR:
> >         {
> >   	tree fn = CALL_EXPR_FN (arg);
> > +	if (TREE_TYPE (arg) == NULL_TREE)
> 
> How about changing dependent_name to take the CALL_EXPR rather than the
> CALL_EXPR_FN?  That would mean some changes to write_expression to move the
> dependent_name handling into the CALL_EXPR handling, but that doesn't seem
> like a bad thing.  Other callers seem like a trivial change.

Indeed changing dependent_name seems best, but I'm worried about such a
refactoring to write_expression causing unintended mangling changes at
this stage.  Because it seems the CALL_EXPR case of write_expression
isn't the user of the dependent_name branch of write_expression, at
least according to the following patch which causes us to ICE on
mangle{37,57,58,76}.C:

diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index f2cda3be2cf..700857f8f3c 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -3450,6 +3450,7 @@ write_expression (tree expr)
     }
   else if (dependent_name (expr))
     {
+      gcc_unreachable ();
       tree name = dependent_name (expr);
       if (IDENTIFIER_ANY_OP_P (name))
 	{
@@ -3554,7 +3555,19 @@ write_expression (tree expr)
 		&& type_dependent_expression_p_push (expr))
 	      fn = OVL_NAME (fn);
 
-	    write_expression (fn);
+	    if (tree name = dependent_name (fn))
+	      {
+		if (IDENTIFIER_ANY_OP_P (name))
+		  {
+		    if (abi_version_at_least (16))
+		      write_string ("on");
+		    if (abi_warn_or_compat_version_crosses (16))
+		      G.need_abi_warning = 1;
+		  }
+		write_unqualified_id (name);
+	      }
+	    else
+	      write_expression (fn);
 	  }
 
 	  for (i = 0; i < call_expr_nargs (expr); ++i)

And since the CALL_EXPR case of write_expression looks through an
ADDR_EXPR callee before recursing, IIUC the refactoring would need to
make dependent_name look through an ADDR_EXPR callee as well, which
seems like a desirable/correct change but I'm worried that might have
unintended consequences as well.

> 
> >   	  if (tree name = dependent_name (fn))
> >   	    {
> >   	      if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
> > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > index c1da868732b..3a57e71b76e 100644
> > --- a/gcc/cp/tree.cc
> > +++ b/gcc/cp/tree.cc
> > @@ -3870,16 +3870,21 @@ decl_internal_context_p (const_tree decl)
> >     return !TREE_PUBLIC (decl);
> >   }
> >   -/* Subroutine of cp_tree_equal: t1 and t2 are the CALL_EXPR_FNs of two
> > -   CALL_EXPRS.  Return whether they are equivalent.  */
> > +/* Subroutine of cp_tree_equal: t1 and t2 are two CALL_EXPRs.
> > +   Return whether their CALL_EXPR_FNs are equivalent.  */
> >     static bool
> >   called_fns_equal (tree t1, tree t2)
> > +{
> > +  tree fn1 = CALL_EXPR_FN (t1);
> > +  tree fn2 = CALL_EXPR_FN (t2);
> > +  if (TREE_TYPE (t1) == NULL_TREE
> > +      && TREE_TYPE (t2) == NULL_TREE)
> >       {
> >         /* Core 1321: dependent names are equivalent even if the overload
> > sets
> >   	 are different.  But do compare explicit template arguments.  */
> > -  tree name1 = dependent_name (t1);
> > -  tree name2 = dependent_name (t2);
> > +      tree name1 = dependent_name (fn1);
> > +      tree name2 = dependent_name (fn2);
> >         if (name1 || name2)
> >   	{
> >   	  tree targs1 = NULL_TREE, targs2 = NULL_TREE;
> > @@ -3891,19 +3896,19 @@ called_fns_equal (tree t1, tree t2)
> >   	     of whether the function was named with a qualified- or
> > unqualified-id.
> >   	     Until that's fixed, check that we aren't looking at overload sets
> > from
> >   	     different scopes.  */
> > -      if (is_overloaded_fn (t1) && is_overloaded_fn (t2)
> > -	  && (DECL_CONTEXT (get_first_fn (t1))
> > -	      != DECL_CONTEXT (get_first_fn (t2))))
> > +	  if (is_overloaded_fn (fn1) && is_overloaded_fn (fn2)
> > +	      && (DECL_CONTEXT (get_first_fn (fn1))
> > +		  != DECL_CONTEXT (get_first_fn (fn2))))
> >   	    return false;
> >   -      if (TREE_CODE (t1) == TEMPLATE_ID_EXPR)
> > -	targs1 = TREE_OPERAND (t1, 1);
> > -      if (TREE_CODE (t2) == TEMPLATE_ID_EXPR)
> > -	targs2 = TREE_OPERAND (t2, 1);
> > +	  if (TREE_CODE (fn1) == TEMPLATE_ID_EXPR)
> > +	    targs1 = TREE_OPERAND (fn1, 1);
> > +	  if (TREE_CODE (fn2) == TEMPLATE_ID_EXPR)
> > +	    targs2 = TREE_OPERAND (fn2, 1);
> >   	  return cp_tree_equal (targs1, targs2);
> >   	}
> > -  else
> > -    return cp_tree_equal (t1, t2);
> > +    }
> > +  return cp_tree_equal (fn1, fn2);
> >   }
> >     bool comparing_override_contracts;
> > @@ -4037,7 +4042,7 @@ cp_tree_equal (tree t1, tree t2)
> >   	if (KOENIG_LOOKUP_P (t1) != KOENIG_LOOKUP_P (t2))
> >   	  return false;
> >   -	if (!called_fns_equal (CALL_EXPR_FN (t1), CALL_EXPR_FN (t2)))
> > +	if (!called_fns_equal (t1, t2))
> >   	  return false;
> >     	call_expr_arg_iterator iter1, iter2;
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5.C
> > b/gcc/testsuite/g++.dg/cpp0x/overload5.C
> > new file mode 100644
> > index 00000000000..e05b1594f51
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/overload5.C
> > @@ -0,0 +1,12 @@
> > +// PR c++/107461
> > +// { dg-do compile { target c++11 } }
> > +
> > +int f(...);
> > +template<class T> decltype(T() + f(0)) g(); // #1
> > +
> > +char f(int);
> > +template<class T> decltype(T() + f(0)) g(); // #2, distinct from #1
> > +
> > +int main() {
> > +  g<int>(); // { dg-error "ambiguous" }
> > +}
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5a.C
> > b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
> > new file mode 100644
> > index 00000000000..037114f199c
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
> > @@ -0,0 +1,10 @@
> > +// PR c++/107461
> > +// { dg-do compile { target c++11 } }
> > +
> > +template<class T> T f();
> > +template<class T> decltype(T() + f<int*>()) g(); // #1
> > +template<class T> decltype(T() + f<char>()) g(); // #2, distinct from #1
> > +
> > +int main() {
> > +  g<int>(); // { dg-error "ambiguous" }
> > +}
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/overload6.C
> > b/gcc/testsuite/g++.dg/cpp0x/overload6.C
> > new file mode 100644
> > index 00000000000..1fbee0501de
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/overload6.C
> > @@ -0,0 +1,16 @@
> > +// PR c++/107461
> > +// { dg-do compile { target c++11 } }
> > +
> > +template<class T> T f();
> > +
> > +template<class> struct A { };
> > +
> > +template<class T> struct B {
> > +  template<class U, class = A<decltype(f<T>()(U()))>>
> > +  static void g(U);
> > +};
> > +
> > +int main() {
> > +  B<int> b;
> > +  B<void(*)(int)>::g(0); // { dg-bogus "no match" }
> > +}
> 
>
  

On 2/4/23 20:08, Patrick Palka wrote:
> On Sat, 4 Feb 2023, Jason Merrill wrote:
> 
>> On 2/4/23 15:31, Patrick Palka wrote:
>>> After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
>>> CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of FUNCTION_DECL.
>>> This innocent change revealed that cp_tree_equal doesn't first check
>>> dependentness of a CALL_EXPR before treating the callee as a dependent
>>> name, which manifests as us incorrectly accepting the first two
>>> testcases below and rejecting the third:
>>>
>>>    * In the first testcase, cp_tree_equal incorrectly returns true for
>>>      the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
>>>      are different FUNCTION_DECLs) and so we treat #2 as a redeclaration
>>>      of #1.
>>>
>>>    * Same issue in the second testcase, for f<int*>() and f<char>().
>>>
>>>    * In the third testcase, cp_tree_equal incorrectly returns true for
>>>      f<int>() and f<void(*)(int)>() which causes us to conflate the two
>>>      dependent specializations A<decltype(f<int>()(U()))> and
>>>      A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.
>>>
>>> This patch fixes this by making called_fns_equal treat two callees as
>>> dependent names only if the CALL_EXPRs in question are dependent.
>>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk/12?  Patch generated with -w to ignore noisy whitespace changes.
>>>
>>> 	PR c++/107461
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Treat
>>> 	the callee as a dependent name only if the CALL_EXPR is
>>> 	dependent.
>>> 	* tree.cc (called_fns_equal): Take two CALL_EXPRs instead of
>>> 	CALL_EXPR_FNs thereof.  As above.
>>> 	(cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp0x/overload5.C: New test.
>>> 	* g++.dg/cpp0x/overload5a.C: New test.
>>> 	* g++.dg/cpp0x/overload6.C: New test.
>>> ---
>>>    gcc/cp/pt.cc                            |  1 +
>>>    gcc/cp/tree.cc                          | 33 ++++++++++++++-----------
>>>    gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 +++++++++
>>>    gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++
>>>    gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++
>>>    5 files changed, 58 insertions(+), 14 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C
>>>
>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>> index 255332dc0c1..c9360240cd2 100644
>>> --- a/gcc/cp/pt.cc
>>> +++ b/gcc/cp/pt.cc
>>> @@ -1841,6 +1841,7 @@ iterative_hash_template_arg (tree arg, hashval_t val)
>>>        case CALL_EXPR:
>>>          {
>>>    	tree fn = CALL_EXPR_FN (arg);
>>> +	if (TREE_TYPE (arg) == NULL_TREE)
>>
>> How about changing dependent_name to take the CALL_EXPR rather than the
>> CALL_EXPR_FN?  That would mean some changes to write_expression to move the
>> dependent_name handling into the CALL_EXPR handling, but that doesn't seem
>> like a bad thing.  Other callers seem like a trivial change.
> 
> Indeed changing dependent_name seems best, but I'm worried about such a
> refactoring to write_expression causing unintended mangling changes at
> this stage.  Because it seems the CALL_EXPR case of write_expression
> isn't the user of the dependent_name branch of write_expression, at
> least according to the following patch which causes us to ICE on
> mangle{37,57,58,76}.C:

Yeah, I tried the same thing.  Maybe for GCC 13 better to add a new 
function rather than change the current one.

> diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
> index f2cda3be2cf..700857f8f3c 100644
> --- a/gcc/cp/mangle.cc
> +++ b/gcc/cp/mangle.cc
> @@ -3450,6 +3450,7 @@ write_expression (tree expr)
>       }
>     else if (dependent_name (expr))
>       {
> +      gcc_unreachable ();
>         tree name = dependent_name (expr);
>         if (IDENTIFIER_ANY_OP_P (name))
>   	{
> @@ -3554,7 +3555,19 @@ write_expression (tree expr)
>   		&& type_dependent_expression_p_push (expr))
>   	      fn = OVL_NAME (fn);
>   
> -	    write_expression (fn);
> +	    if (tree name = dependent_name (fn))
> +	      {
> +		if (IDENTIFIER_ANY_OP_P (name))
> +		  {
> +		    if (abi_version_at_least (16))
> +		      write_string ("on");
> +		    if (abi_warn_or_compat_version_crosses (16))
> +		      G.need_abi_warning = 1;
> +		  }
> +		write_unqualified_id (name);
> +	      }
> +	    else
> +	      write_expression (fn);
>   	  }
>   
>   	  for (i = 0; i < call_expr_nargs (expr); ++i)
> 
> And since the CALL_EXPR case of write_expression looks through an
> ADDR_EXPR callee before recursing, IIUC the refactoring would need to
> make dependent_name look through an ADDR_EXPR callee as well, which
> seems like a desirable/correct change but I'm worried that might have
> unintended consequences as well.
> 
>>
>>>    	  if (tree name = dependent_name (fn))
>>>    	    {
>>>    	      if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
>>> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
>>> index c1da868732b..3a57e71b76e 100644
>>> --- a/gcc/cp/tree.cc
>>> +++ b/gcc/cp/tree.cc
>>> @@ -3870,16 +3870,21 @@ decl_internal_context_p (const_tree decl)
>>>      return !TREE_PUBLIC (decl);
>>>    }
>>>    -/* Subroutine of cp_tree_equal: t1 and t2 are the CALL_EXPR_FNs of two
>>> -   CALL_EXPRS.  Return whether they are equivalent.  */
>>> +/* Subroutine of cp_tree_equal: t1 and t2 are two CALL_EXPRs.
>>> +   Return whether their CALL_EXPR_FNs are equivalent.  */
>>>      static bool
>>>    called_fns_equal (tree t1, tree t2)
>>> +{
>>> +  tree fn1 = CALL_EXPR_FN (t1);
>>> +  tree fn2 = CALL_EXPR_FN (t2);
>>> +  if (TREE_TYPE (t1) == NULL_TREE
>>> +      && TREE_TYPE (t2) == NULL_TREE)
>>>        {
>>>          /* Core 1321: dependent names are equivalent even if the overload
>>> sets
>>>    	 are different.  But do compare explicit template arguments.  */
>>> -  tree name1 = dependent_name (t1);
>>> -  tree name2 = dependent_name (t2);
>>> +      tree name1 = dependent_name (fn1);
>>> +      tree name2 = dependent_name (fn2);
>>>          if (name1 || name2)
>>>    	{
>>>    	  tree targs1 = NULL_TREE, targs2 = NULL_TREE;
>>> @@ -3891,19 +3896,19 @@ called_fns_equal (tree t1, tree t2)
>>>    	     of whether the function was named with a qualified- or
>>> unqualified-id.
>>>    	     Until that's fixed, check that we aren't looking at overload sets
>>> from
>>>    	     different scopes.  */
>>> -      if (is_overloaded_fn (t1) && is_overloaded_fn (t2)
>>> -	  && (DECL_CONTEXT (get_first_fn (t1))
>>> -	      != DECL_CONTEXT (get_first_fn (t2))))
>>> +	  if (is_overloaded_fn (fn1) && is_overloaded_fn (fn2)
>>> +	      && (DECL_CONTEXT (get_first_fn (fn1))
>>> +		  != DECL_CONTEXT (get_first_fn (fn2))))
>>>    	    return false;
>>>    -      if (TREE_CODE (t1) == TEMPLATE_ID_EXPR)
>>> -	targs1 = TREE_OPERAND (t1, 1);
>>> -      if (TREE_CODE (t2) == TEMPLATE_ID_EXPR)
>>> -	targs2 = TREE_OPERAND (t2, 1);
>>> +	  if (TREE_CODE (fn1) == TEMPLATE_ID_EXPR)
>>> +	    targs1 = TREE_OPERAND (fn1, 1);
>>> +	  if (TREE_CODE (fn2) == TEMPLATE_ID_EXPR)
>>> +	    targs2 = TREE_OPERAND (fn2, 1);
>>>    	  return cp_tree_equal (targs1, targs2);
>>>    	}
>>> -  else
>>> -    return cp_tree_equal (t1, t2);
>>> +    }
>>> +  return cp_tree_equal (fn1, fn2);
>>>    }
>>>      bool comparing_override_contracts;
>>> @@ -4037,7 +4042,7 @@ cp_tree_equal (tree t1, tree t2)
>>>    	if (KOENIG_LOOKUP_P (t1) != KOENIG_LOOKUP_P (t2))
>>>    	  return false;
>>>    -	if (!called_fns_equal (CALL_EXPR_FN (t1), CALL_EXPR_FN (t2)))
>>> +	if (!called_fns_equal (t1, t2))
>>>    	  return false;
>>>      	call_expr_arg_iterator iter1, iter2;
>>> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5.C
>>> b/gcc/testsuite/g++.dg/cpp0x/overload5.C
>>> new file mode 100644
>>> index 00000000000..e05b1594f51
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp0x/overload5.C
>>> @@ -0,0 +1,12 @@
>>> +// PR c++/107461
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +int f(...);
>>> +template<class T> decltype(T() + f(0)) g(); // #1
>>> +
>>> +char f(int);
>>> +template<class T> decltype(T() + f(0)) g(); // #2, distinct from #1
>>> +
>>> +int main() {
>>> +  g<int>(); // { dg-error "ambiguous" }
>>> +}
>>> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>> b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>> new file mode 100644
>>> index 00000000000..037114f199c
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>> @@ -0,0 +1,10 @@
>>> +// PR c++/107461
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +template<class T> T f();
>>> +template<class T> decltype(T() + f<int*>()) g(); // #1
>>> +template<class T> decltype(T() + f<char>()) g(); // #2, distinct from #1
>>> +
>>> +int main() {
>>> +  g<int>(); // { dg-error "ambiguous" }
>>> +}
>>> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload6.C
>>> b/gcc/testsuite/g++.dg/cpp0x/overload6.C
>>> new file mode 100644
>>> index 00000000000..1fbee0501de
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp0x/overload6.C
>>> @@ -0,0 +1,16 @@
>>> +// PR c++/107461
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +template<class T> T f();
>>> +
>>> +template<class> struct A { };
>>> +
>>> +template<class T> struct B {
>>> +  template<class U, class = A<decltype(f<T>()(U()))>>
>>> +  static void g(U);
>>> +};
>>> +
>>> +int main() {
>>> +  B<int> b;
>>> +  B<void(*)(int)>::g(0); // { dg-bogus "no match" }
>>> +}
>>
>>
>
  

On 2/4/23 20:41, Jason Merrill wrote:
> On 2/4/23 20:08, Patrick Palka wrote:
>> On Sat, 4 Feb 2023, Jason Merrill wrote:
>>
>>> On 2/4/23 15:31, Patrick Palka wrote:
>>>> After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
>>>> CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of 
>>>> FUNCTION_DECL.
>>>> This innocent change revealed that cp_tree_equal doesn't first check
>>>> dependentness of a CALL_EXPR before treating the callee as a dependent
>>>> name, which manifests as us incorrectly accepting the first two
>>>> testcases below and rejecting the third:
>>>>
>>>>    * In the first testcase, cp_tree_equal incorrectly returns true for
>>>>      the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
>>>>      are different FUNCTION_DECLs) and so we treat #2 as a 
>>>> redeclaration
>>>>      of #1.
>>>>
>>>>    * Same issue in the second testcase, for f<int*>() and f<char>().
>>>>
>>>>    * In the third testcase, cp_tree_equal incorrectly returns true for
>>>>      f<int>() and f<void(*)(int)>() which causes us to conflate the two
>>>>      dependent specializations A<decltype(f<int>()(U()))> and
>>>>      A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.
>>>>
>>>> This patch fixes this by making called_fns_equal treat two callees as
>>>> dependent names only if the CALL_EXPRs in question are dependent.
>>>>
>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK 
>>>> for
>>>> trunk/12?  Patch generated with -w to ignore noisy whitespace changes.
>>>>
>>>>     PR c++/107461
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>>     * pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Treat
>>>>     the callee as a dependent name only if the CALL_EXPR is
>>>>     dependent.
>>>>     * tree.cc (called_fns_equal): Take two CALL_EXPRs instead of
>>>>     CALL_EXPR_FNs thereof.  As above.
>>>>     (cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>     * g++.dg/cpp0x/overload5.C: New test.
>>>>     * g++.dg/cpp0x/overload5a.C: New test.
>>>>     * g++.dg/cpp0x/overload6.C: New test.
>>>> ---
>>>>    gcc/cp/pt.cc                            |  1 +
>>>>    gcc/cp/tree.cc                          | 33 
>>>> ++++++++++++++-----------
>>>>    gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 +++++++++
>>>>    gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++
>>>>    gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++
>>>>    5 files changed, 58 insertions(+), 14 deletions(-)
>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C
>>>>
>>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>>> index 255332dc0c1..c9360240cd2 100644
>>>> --- a/gcc/cp/pt.cc
>>>> +++ b/gcc/cp/pt.cc
>>>> @@ -1841,6 +1841,7 @@ iterative_hash_template_arg (tree arg, 
>>>> hashval_t val)
>>>>        case CALL_EXPR:
>>>>          {
>>>>        tree fn = CALL_EXPR_FN (arg);
>>>> +    if (TREE_TYPE (arg) == NULL_TREE)
>>>
>>> How about changing dependent_name to take the CALL_EXPR rather than the
>>> CALL_EXPR_FN?  That would mean some changes to write_expression to 
>>> move the
>>> dependent_name handling into the CALL_EXPR handling, but that doesn't 
>>> seem
>>> like a bad thing.  Other callers seem like a trivial change.
>>
>> Indeed changing dependent_name seems best, but I'm worried about such a
>> refactoring to write_expression causing unintended mangling changes at
>> this stage.  Because it seems the CALL_EXPR case of write_expression
>> isn't the user of the dependent_name branch of write_expression, at
>> least according to the following patch which causes us to ICE on
>> mangle{37,57,58,76}.C:
> 
> Yeah, I tried the same thing.  Maybe for GCC 13 better to add a new 
> function rather than change the current one.

mangle76 seems like a bug where we're producing (and testing for) the 
wrong mangling -- mangling (*this). that doesn't exist in the source. 
clang gets it right.

mangle5{7,8} has the right mangling, we're just using dependent_name to 
mangle function names that aren't dependent names (because they're 
template arguments in both cases, and qualified in the latter).

>> diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
>> index f2cda3be2cf..700857f8f3c 100644
>> --- a/gcc/cp/mangle.cc
>> +++ b/gcc/cp/mangle.cc
>> @@ -3450,6 +3450,7 @@ write_expression (tree expr)
>>       }
>>     else if (dependent_name (expr))
>>       {
>> +      gcc_unreachable ();
>>         tree name = dependent_name (expr);
>>         if (IDENTIFIER_ANY_OP_P (name))
>>       {
>> @@ -3554,7 +3555,19 @@ write_expression (tree expr)
>>           && type_dependent_expression_p_push (expr))
>>             fn = OVL_NAME (fn);
>> -        write_expression (fn);
>> +        if (tree name = dependent_name (fn))
>> +          {
>> +        if (IDENTIFIER_ANY_OP_P (name))
>> +          {
>> +            if (abi_version_at_least (16))
>> +              write_string ("on");
>> +            if (abi_warn_or_compat_version_crosses (16))
>> +              G.need_abi_warning = 1;
>> +          }
>> +        write_unqualified_id (name);
>> +          }
>> +        else
>> +          write_expression (fn);
>>         }
>>         for (i = 0; i < call_expr_nargs (expr); ++i)
>>
>> And since the CALL_EXPR case of write_expression looks through an
>> ADDR_EXPR callee before recursing, IIUC the refactoring would need to
>> make dependent_name look through an ADDR_EXPR callee as well, which
>> seems like a desirable/correct change but I'm worried that might have
>> unintended consequences as well.
>>
>>>
>>>>          if (tree name = dependent_name (fn))
>>>>            {
>>>>              if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
>>>> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
>>>> index c1da868732b..3a57e71b76e 100644
>>>> --- a/gcc/cp/tree.cc
>>>> +++ b/gcc/cp/tree.cc
>>>> @@ -3870,16 +3870,21 @@ decl_internal_context_p (const_tree decl)
>>>>      return !TREE_PUBLIC (decl);
>>>>    }
>>>>    -/* Subroutine of cp_tree_equal: t1 and t2 are the CALL_EXPR_FNs 
>>>> of two
>>>> -   CALL_EXPRS.  Return whether they are equivalent.  */
>>>> +/* Subroutine of cp_tree_equal: t1 and t2 are two CALL_EXPRs.
>>>> +   Return whether their CALL_EXPR_FNs are equivalent.  */
>>>>      static bool
>>>>    called_fns_equal (tree t1, tree t2)
>>>> +{
>>>> +  tree fn1 = CALL_EXPR_FN (t1);
>>>> +  tree fn2 = CALL_EXPR_FN (t2);
>>>> +  if (TREE_TYPE (t1) == NULL_TREE
>>>> +      && TREE_TYPE (t2) == NULL_TREE)
>>>>        {
>>>>          /* Core 1321: dependent names are equivalent even if the 
>>>> overload
>>>> sets
>>>>         are different.  But do compare explicit template arguments.  */
>>>> -  tree name1 = dependent_name (t1);
>>>> -  tree name2 = dependent_name (t2);
>>>> +      tree name1 = dependent_name (fn1);
>>>> +      tree name2 = dependent_name (fn2);
>>>>          if (name1 || name2)
>>>>        {
>>>>          tree targs1 = NULL_TREE, targs2 = NULL_TREE;
>>>> @@ -3891,19 +3896,19 @@ called_fns_equal (tree t1, tree t2)
>>>>             of whether the function was named with a qualified- or
>>>> unqualified-id.
>>>>             Until that's fixed, check that we aren't looking at 
>>>> overload sets
>>>> from
>>>>             different scopes.  */
>>>> -      if (is_overloaded_fn (t1) && is_overloaded_fn (t2)
>>>> -      && (DECL_CONTEXT (get_first_fn (t1))
>>>> -          != DECL_CONTEXT (get_first_fn (t2))))
>>>> +      if (is_overloaded_fn (fn1) && is_overloaded_fn (fn2)
>>>> +          && (DECL_CONTEXT (get_first_fn (fn1))
>>>> +          != DECL_CONTEXT (get_first_fn (fn2))))
>>>>            return false;
>>>>    -      if (TREE_CODE (t1) == TEMPLATE_ID_EXPR)
>>>> -    targs1 = TREE_OPERAND (t1, 1);
>>>> -      if (TREE_CODE (t2) == TEMPLATE_ID_EXPR)
>>>> -    targs2 = TREE_OPERAND (t2, 1);
>>>> +      if (TREE_CODE (fn1) == TEMPLATE_ID_EXPR)
>>>> +        targs1 = TREE_OPERAND (fn1, 1);
>>>> +      if (TREE_CODE (fn2) == TEMPLATE_ID_EXPR)
>>>> +        targs2 = TREE_OPERAND (fn2, 1);
>>>>          return cp_tree_equal (targs1, targs2);
>>>>        }
>>>> -  else
>>>> -    return cp_tree_equal (t1, t2);
>>>> +    }
>>>> +  return cp_tree_equal (fn1, fn2);
>>>>    }
>>>>      bool comparing_override_contracts;
>>>> @@ -4037,7 +4042,7 @@ cp_tree_equal (tree t1, tree t2)
>>>>        if (KOENIG_LOOKUP_P (t1) != KOENIG_LOOKUP_P (t2))
>>>>          return false;
>>>>    -    if (!called_fns_equal (CALL_EXPR_FN (t1), CALL_EXPR_FN (t2)))
>>>> +    if (!called_fns_equal (t1, t2))
>>>>          return false;
>>>>          call_expr_arg_iterator iter1, iter2;
>>>> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5.C
>>>> b/gcc/testsuite/g++.dg/cpp0x/overload5.C
>>>> new file mode 100644
>>>> index 00000000000..e05b1594f51
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/g++.dg/cpp0x/overload5.C
>>>> @@ -0,0 +1,12 @@
>>>> +// PR c++/107461
>>>> +// { dg-do compile { target c++11 } }
>>>> +
>>>> +int f(...);
>>>> +template<class T> decltype(T() + f(0)) g(); // #1
>>>> +
>>>> +char f(int);
>>>> +template<class T> decltype(T() + f(0)) g(); // #2, distinct from #1
>>>> +
>>>> +int main() {
>>>> +  g<int>(); // { dg-error "ambiguous" }
>>>> +}
>>>> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>>> b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>>> new file mode 100644
>>>> index 00000000000..037114f199c
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>>> @@ -0,0 +1,10 @@
>>>> +// PR c++/107461
>>>> +// { dg-do compile { target c++11 } }
>>>> +
>>>> +template<class T> T f();
>>>> +template<class T> decltype(T() + f<int*>()) g(); // #1
>>>> +template<class T> decltype(T() + f<char>()) g(); // #2, distinct 
>>>> from #1
>>>> +
>>>> +int main() {
>>>> +  g<int>(); // { dg-error "ambiguous" }
>>>> +}
>>>> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload6.C
>>>> b/gcc/testsuite/g++.dg/cpp0x/overload6.C
>>>> new file mode 100644
>>>> index 00000000000..1fbee0501de
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/g++.dg/cpp0x/overload6.C
>>>> @@ -0,0 +1,16 @@
>>>> +// PR c++/107461
>>>> +// { dg-do compile { target c++11 } }
>>>> +
>>>> +template<class T> T f();
>>>> +
>>>> +template<class> struct A { };
>>>> +
>>>> +template<class T> struct B {
>>>> +  template<class U, class = A<decltype(f<T>()(U()))>>
>>>> +  static void g(U);
>>>> +};
>>>> +
>>>> +int main() {
>>>> +  B<int> b;
>>>> +  B<void(*)(int)>::g(0); // { dg-bogus "no match" }
>>>> +}
>>>
>>>
>>
>
  

On Sat, 4 Feb 2023, Jason Merrill wrote:

> On 2/4/23 20:41, Jason Merrill wrote:
> > On 2/4/23 20:08, Patrick Palka wrote:
> > > On Sat, 4 Feb 2023, Jason Merrill wrote:
> > > 
> > > > On 2/4/23 15:31, Patrick Palka wrote:
> > > > > After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
> > > > > CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of
> > > > > FUNCTION_DECL.
> > > > > This innocent change revealed that cp_tree_equal doesn't first check
> > > > > dependentness of a CALL_EXPR before treating the callee as a dependent
> > > > > name, which manifests as us incorrectly accepting the first two
> > > > > testcases below and rejecting the third:
> > > > > 
> > > > >    * In the first testcase, cp_tree_equal incorrectly returns true for
> > > > >      the two non-dependent CALL_EXPRs f(0) and f(0) (whose
> > > > > CALL_EXPR_FN
> > > > >      are different FUNCTION_DECLs) and so we treat #2 as a
> > > > > redeclaration
> > > > >      of #1.
> > > > > 
> > > > >    * Same issue in the second testcase, for f<int*>() and f<char>().
> > > > > 
> > > > >    * In the third testcase, cp_tree_equal incorrectly returns true for
> > > > >      f<int>() and f<void(*)(int)>() which causes us to conflate the
> > > > > two
> > > > >      dependent specializations A<decltype(f<int>()(U()))> and
> > > > >      A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.
> > > > > 
> > > > > This patch fixes this by making called_fns_equal treat two callees as
> > > > > dependent names only if the CALL_EXPRs in question are dependent.
> > > > > 
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > > > > for
> > > > > trunk/12?  Patch generated with -w to ignore noisy whitespace changes.
> > > > > 
> > > > >     PR c++/107461
> > > > > 
> > > > > gcc/cp/ChangeLog:
> > > > > 
> > > > >     * pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Treat
> > > > >     the callee as a dependent name only if the CALL_EXPR is
> > > > >     dependent.
> > > > >     * tree.cc (called_fns_equal): Take two CALL_EXPRs instead of
> > > > >     CALL_EXPR_FNs thereof.  As above.
> > > > >     (cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
> > > > > 
> > > > > gcc/testsuite/ChangeLog:
> > > > > 
> > > > >     * g++.dg/cpp0x/overload5.C: New test.
> > > > >     * g++.dg/cpp0x/overload5a.C: New test.
> > > > >     * g++.dg/cpp0x/overload6.C: New test.
> > > > > ---
> > > > >    gcc/cp/pt.cc                            |  1 +
> > > > >    gcc/cp/tree.cc                          | 33
> > > > > ++++++++++++++-----------
> > > > >    gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 +++++++++
> > > > >    gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++
> > > > >    gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++
> > > > >    5 files changed, 58 insertions(+), 14 deletions(-)
> > > > >    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
> > > > >    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
> > > > >    create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C
> > > > > 
> > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > > > index 255332dc0c1..c9360240cd2 100644
> > > > > --- a/gcc/cp/pt.cc
> > > > > +++ b/gcc/cp/pt.cc
> > > > > @@ -1841,6 +1841,7 @@ iterative_hash_template_arg (tree arg, hashval_t
> > > > > val)
> > > > >        case CALL_EXPR:
> > > > >          {
> > > > >        tree fn = CALL_EXPR_FN (arg);
> > > > > +    if (TREE_TYPE (arg) == NULL_TREE)
> > > > 
> > > > How about changing dependent_name to take the CALL_EXPR rather than the
> > > > CALL_EXPR_FN?  That would mean some changes to write_expression to move
> > > > the
> > > > dependent_name handling into the CALL_EXPR handling, but that doesn't
> > > > seem
> > > > like a bad thing.  Other callers seem like a trivial change.
> > > 
> > > Indeed changing dependent_name seems best, but I'm worried about such a
> > > refactoring to write_expression causing unintended mangling changes at
> > > this stage.  Because it seems the CALL_EXPR case of write_expression
> > > isn't the user of the dependent_name branch of write_expression, at
> > > least according to the following patch which causes us to ICE on
> > > mangle{37,57,58,76}.C:
> > 
> > Yeah, I tried the same thing.  Maybe for GCC 13 better to add a new function
> > rather than change the current one.

Sounds good, like so?  Only regtested so far.  Full bootstrap and
regtest running on x86_64-pc-linux-gnu.

-- >8 --

Subject: [PATCH] c++: equivalence of non-dependent calls [PR107461]

After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of FUNCTION_DECL.
This innocent change revealed that cp_tree_equal doesn't first check
dependentness of a CALL_EXPR before treating a FUNCTION_DECL callee as a
dependent name, which manifests as us incorrectly accepting the first
two testcases below and rejecting the third:

 * In the first testcase, cp_tree_equal incorrectly returns true for
   the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
   are different FUNCTION_DECLs) and so we treat #2 as a redeclaration
   of #1.

 * Same issue in the second testcase, for f<int*>() and f<char>().

 * In the third testcase, cp_tree_equal incorrectly returns true for
   f<int>() and f<void(*)(int)>() which causes us to conflate the two
   dependent specializations A<decltype(f<int>()(U()))> and
   A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.

This patch fixes this by making called_fns_equal treat two callees as
dependent names only if the overall CALL_EXPRs are dependent, via a new
convenience function call_expr_dependent_name that is like dependent_name
but also checks dependence of the overall CALL_EXPR.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk/12?  Patch generated with -w to ignore noisy whitespace changes.

	PR c++/107461

gcc/cp/ChangeLog:

	* cp-tree.h (call_expr_dependent_name): Declare.
	* pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Use
	call_expr_dependent_name instead of dependent_name.
	* tree.cc (call_expr_dependent_name): Define.
	(called_fns_equal): Adjust to take two CALL_EXPRs instead of
	CALL_EXPR_FNs thereof.  Use call_expr_dependent_name instead
	of dependent_name.
	(cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/overload5.C: New test.
	* g++.dg/cpp0x/overload5a.C: New test.
	* g++.dg/cpp0x/overload6.C: New test.
---
 gcc/cp/cp-tree.h                        |  1 +
 gcc/cp/pt.cc                            |  2 +-
 gcc/cp/tree.cc                          | 24 +++++++++++++++++++-----
 gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 ++++++++++++
 gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++++
 gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++++++
 6 files changed, 59 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 00b2bffc85c..ef601182d4b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7902,6 +7902,7 @@ extern tree lookup_maybe_add			(tree fns, tree lookup,
 extern int is_overloaded_fn			(tree) ATTRIBUTE_PURE;
 extern bool really_overloaded_fn		(tree) ATTRIBUTE_PURE;
 extern tree dependent_name			(tree);
+extern tree call_expr_dependent_name		(tree);
 extern tree maybe_get_fns			(tree) ATTRIBUTE_PURE;
 extern tree get_fns				(tree) ATTRIBUTE_PURE;
 extern tree get_first_fn			(tree) ATTRIBUTE_PURE;
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 255332dc0c1..9f3fc1fa089 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -1841,7 +1841,7 @@ iterative_hash_template_arg (tree arg, hashval_t val)
     case CALL_EXPR:
       {
 	tree fn = CALL_EXPR_FN (arg);
-	if (tree name = dependent_name (fn))
+	if (tree name = call_expr_dependent_name (arg))
 	  {
 	    if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
 	      val = iterative_hash_template_arg (TREE_OPERAND (fn, 1), val);
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index c1da868732b..880bd4f9bcf 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -2608,6 +2608,18 @@ dependent_name (tree x)
   return NULL_TREE;
 }
 
+/* Like dependent_name, but takes the overall CALL_EXPR and checks its
+   dependence.  */
+
+tree
+call_expr_dependent_name (tree x)
+{
+  if (TREE_TYPE (x) != NULL_TREE)
+    /* X isn't dependent, so its callee isn't a dependent name.  */
+    return NULL_TREE;
+  return dependent_name (CALL_EXPR_FN (x));
+}
+
 /* Returns true iff X is an expression for an overloaded function
    whose type cannot be known without performing overload
    resolution.  */
@@ -3870,16 +3882,18 @@ decl_internal_context_p (const_tree decl)
   return !TREE_PUBLIC (decl);
 }
 
-/* Subroutine of cp_tree_equal: t1 and t2 are the CALL_EXPR_FNs of two
-   CALL_EXPRS.  Return whether they are equivalent.  */
+/* Subroutine of cp_tree_equal: t1 and t2 are two CALL_EXPRs.
+   Return whether their CALL_EXPR_FNs are equivalent.  */
 
 static bool
 called_fns_equal (tree t1, tree t2)
 {
   /* Core 1321: dependent names are equivalent even if the overload sets
      are different.  But do compare explicit template arguments.  */
-  tree name1 = dependent_name (t1);
-  tree name2 = dependent_name (t2);
+  tree name1 = call_expr_dependent_name (t1);
+  tree name2 = call_expr_dependent_name (t2);
+  t1 = CALL_EXPR_FN (t1);
+  t2 = CALL_EXPR_FN (t2);
   if (name1 || name2)
     {
       tree targs1 = NULL_TREE, targs2 = NULL_TREE;
@@ -4037,7 +4051,7 @@ cp_tree_equal (tree t1, tree t2)
 	if (KOENIG_LOOKUP_P (t1) != KOENIG_LOOKUP_P (t2))
 	  return false;
 
-	if (!called_fns_equal (CALL_EXPR_FN (t1), CALL_EXPR_FN (t2)))
+	if (!called_fns_equal (t1, t2))
 	  return false;
 
 	call_expr_arg_iterator iter1, iter2;
diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5.C b/gcc/testsuite/g++.dg/cpp0x/overload5.C
new file mode 100644
index 00000000000..e05b1594f51
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/overload5.C
@@ -0,0 +1,12 @@
+// PR c++/107461
+// { dg-do compile { target c++11 } }
+
+int f(...);
+template<class T> decltype(T() + f(0)) g(); // #1
+
+char f(int);
+template<class T> decltype(T() + f(0)) g(); // #2, distinct from #1
+
+int main() {
+  g<int>(); // { dg-error "ambiguous" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5a.C b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
new file mode 100644
index 00000000000..037114f199c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
@@ -0,0 +1,10 @@
+// PR c++/107461
+// { dg-do compile { target c++11 } }
+
+template<class T> T f();
+template<class T> decltype(T() + f<int*>()) g(); // #1
+template<class T> decltype(T() + f<char>()) g(); // #2, distinct from #1
+
+int main() {
+  g<int>(); // { dg-error "ambiguous" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/overload6.C b/gcc/testsuite/g++.dg/cpp0x/overload6.C
new file mode 100644
index 00000000000..1fbee0501de
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/overload6.C
@@ -0,0 +1,16 @@
+// PR c++/107461
+// { dg-do compile { target c++11 } }
+
+template<class T> T f();
+
+template<class> struct A { };
+
+template<class T> struct B {
+  template<class U, class = A<decltype(f<T>()(U()))>>
+  static void g(U);
+};
+
+int main() {
+  B<int> b;
+  B<void(*)(int)>::g(0); // { dg-bogus "no match" }
+}
  

On 2/5/23 09:57, Patrick Palka wrote:
> On Sat, 4 Feb 2023, Jason Merrill wrote:
> 
>> On 2/4/23 20:41, Jason Merrill wrote:
>>> On 2/4/23 20:08, Patrick Palka wrote:
>>>> On Sat, 4 Feb 2023, Jason Merrill wrote:
>>>>
>>>>> On 2/4/23 15:31, Patrick Palka wrote:
>>>>>> After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
>>>>>> CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of
>>>>>> FUNCTION_DECL.
>>>>>> This innocent change revealed that cp_tree_equal doesn't first check
>>>>>> dependentness of a CALL_EXPR before treating the callee as a dependent
>>>>>> name, which manifests as us incorrectly accepting the first two
>>>>>> testcases below and rejecting the third:
>>>>>>
>>>>>>     * In the first testcase, cp_tree_equal incorrectly returns true for
>>>>>>       the two non-dependent CALL_EXPRs f(0) and f(0) (whose
>>>>>> CALL_EXPR_FN
>>>>>>       are different FUNCTION_DECLs) and so we treat #2 as a
>>>>>> redeclaration
>>>>>>       of #1.
>>>>>>
>>>>>>     * Same issue in the second testcase, for f<int*>() and f<char>().
>>>>>>
>>>>>>     * In the third testcase, cp_tree_equal incorrectly returns true for
>>>>>>       f<int>() and f<void(*)(int)>() which causes us to conflate the
>>>>>> two
>>>>>>       dependent specializations A<decltype(f<int>()(U()))> and
>>>>>>       A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.
>>>>>>
>>>>>> This patch fixes this by making called_fns_equal treat two callees as
>>>>>> dependent names only if the CALL_EXPRs in question are dependent.
>>>>>>
>>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
>>>>>> for
>>>>>> trunk/12?  Patch generated with -w to ignore noisy whitespace changes.
>>>>>>
>>>>>>      PR c++/107461
>>>>>>
>>>>>> gcc/cp/ChangeLog:
>>>>>>
>>>>>>      * pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Treat
>>>>>>      the callee as a dependent name only if the CALL_EXPR is
>>>>>>      dependent.
>>>>>>      * tree.cc (called_fns_equal): Take two CALL_EXPRs instead of
>>>>>>      CALL_EXPR_FNs thereof.  As above.
>>>>>>      (cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>>      * g++.dg/cpp0x/overload5.C: New test.
>>>>>>      * g++.dg/cpp0x/overload5a.C: New test.
>>>>>>      * g++.dg/cpp0x/overload6.C: New test.
>>>>>> ---
>>>>>>     gcc/cp/pt.cc                            |  1 +
>>>>>>     gcc/cp/tree.cc                          | 33
>>>>>> ++++++++++++++-----------
>>>>>>     gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 +++++++++
>>>>>>     gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++
>>>>>>     gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++
>>>>>>     5 files changed, 58 insertions(+), 14 deletions(-)
>>>>>>     create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
>>>>>>     create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
>>>>>>     create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C
>>>>>>
>>>>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>>>>> index 255332dc0c1..c9360240cd2 100644
>>>>>> --- a/gcc/cp/pt.cc
>>>>>> +++ b/gcc/cp/pt.cc
>>>>>> @@ -1841,6 +1841,7 @@ iterative_hash_template_arg (tree arg, hashval_t
>>>>>> val)
>>>>>>         case CALL_EXPR:
>>>>>>           {
>>>>>>         tree fn = CALL_EXPR_FN (arg);
>>>>>> +    if (TREE_TYPE (arg) == NULL_TREE)
>>>>>
>>>>> How about changing dependent_name to take the CALL_EXPR rather than the
>>>>> CALL_EXPR_FN?  That would mean some changes to write_expression to move
>>>>> the
>>>>> dependent_name handling into the CALL_EXPR handling, but that doesn't
>>>>> seem
>>>>> like a bad thing.  Other callers seem like a trivial change.
>>>>
>>>> Indeed changing dependent_name seems best, but I'm worried about such a
>>>> refactoring to write_expression causing unintended mangling changes at
>>>> this stage.  Because it seems the CALL_EXPR case of write_expression
>>>> isn't the user of the dependent_name branch of write_expression, at
>>>> least according to the following patch which causes us to ICE on
>>>> mangle{37,57,58,76}.C:
>>>
>>> Yeah, I tried the same thing.  Maybe for GCC 13 better to add a new function
>>> rather than change the current one.
> 
> Sounds good, like so?  Only regtested so far.  Full bootstrap and
> regtest running on x86_64-pc-linux-gnu.
> 
> -- >8 --
> 
> Subject: [PATCH] c++: equivalence of non-dependent calls [PR107461]
> 
> After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
> CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of FUNCTION_DECL.
> This innocent change revealed that cp_tree_equal doesn't first check
> dependentness of a CALL_EXPR before treating a FUNCTION_DECL callee as a
> dependent name, which manifests as us incorrectly accepting the first
> two testcases below and rejecting the third:
> 
>   * In the first testcase, cp_tree_equal incorrectly returns true for
>     the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
>     are different FUNCTION_DECLs) and so we treat #2 as a redeclaration
>     of #1.
> 
>   * Same issue in the second testcase, for f<int*>() and f<char>().
> 
>   * In the third testcase, cp_tree_equal incorrectly returns true for
>     f<int>() and f<void(*)(int)>() which causes us to conflate the two
>     dependent specializations A<decltype(f<int>()(U()))> and
>     A<decltype(f<void(*)(int)>()(U()))>, leading to a bogus error.
> 
> This patch fixes this by making called_fns_equal treat two callees as
> dependent names only if the overall CALL_EXPRs are dependent, via a new
> convenience function call_expr_dependent_name that is like dependent_name
> but also checks dependence of the overall CALL_EXPR.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/12?  Patch generated with -w to ignore noisy whitespace changes.

OK, thanks.

> 	PR c++/107461
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (call_expr_dependent_name): Declare.
> 	* pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Use
> 	call_expr_dependent_name instead of dependent_name.
> 	* tree.cc (call_expr_dependent_name): Define.
> 	(called_fns_equal): Adjust to take two CALL_EXPRs instead of
> 	CALL_EXPR_FNs thereof.  Use call_expr_dependent_name instead
> 	of dependent_name.
> 	(cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/overload5.C: New test.
> 	* g++.dg/cpp0x/overload5a.C: New test.
> 	* g++.dg/cpp0x/overload6.C: New test.
> ---
>   gcc/cp/cp-tree.h                        |  1 +
>   gcc/cp/pt.cc                            |  2 +-
>   gcc/cp/tree.cc                          | 24 +++++++++++++++++++-----
>   gcc/testsuite/g++.dg/cpp0x/overload5.C  | 12 ++++++++++++
>   gcc/testsuite/g++.dg/cpp0x/overload5a.C | 10 ++++++++++
>   gcc/testsuite/g++.dg/cpp0x/overload6.C  | 16 ++++++++++++++++
>   6 files changed, 59 insertions(+), 6 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload5a.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload6.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 00b2bffc85c..ef601182d4b 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7902,6 +7902,7 @@ extern tree lookup_maybe_add			(tree fns, tree lookup,
>   extern int is_overloaded_fn			(tree) ATTRIBUTE_PURE;
>   extern bool really_overloaded_fn		(tree) ATTRIBUTE_PURE;
>   extern tree dependent_name			(tree);
> +extern tree call_expr_dependent_name		(tree);
>   extern tree maybe_get_fns			(tree) ATTRIBUTE_PURE;
>   extern tree get_fns				(tree) ATTRIBUTE_PURE;
>   extern tree get_first_fn			(tree) ATTRIBUTE_PURE;
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 255332dc0c1..9f3fc1fa089 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -1841,7 +1841,7 @@ iterative_hash_template_arg (tree arg, hashval_t val)
>       case CALL_EXPR:
>         {
>   	tree fn = CALL_EXPR_FN (arg);
> -	if (tree name = dependent_name (fn))
> +	if (tree name = call_expr_dependent_name (arg))
>   	  {
>   	    if (TREE_CODE (fn) == TEMPLATE_ID_EXPR)
>   	      val = iterative_hash_template_arg (TREE_OPERAND (fn, 1), val);
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index c1da868732b..880bd4f9bcf 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -2608,6 +2608,18 @@ dependent_name (tree x)
>     return NULL_TREE;
>   }
>   
> +/* Like dependent_name, but takes the overall CALL_EXPR and checks its
> +   dependence.  */
> +
> +tree
> +call_expr_dependent_name (tree x)
> +{
> +  if (TREE_TYPE (x) != NULL_TREE)
> +    /* X isn't dependent, so its callee isn't a dependent name.  */
> +    return NULL_TREE;
> +  return dependent_name (CALL_EXPR_FN (x));
> +}
> +
>   /* Returns true iff X is an expression for an overloaded function
>      whose type cannot be known without performing overload
>      resolution.  */
> @@ -3870,16 +3882,18 @@ decl_internal_context_p (const_tree decl)
>     return !TREE_PUBLIC (decl);
>   }
>   
> -/* Subroutine of cp_tree_equal: t1 and t2 are the CALL_EXPR_FNs of two
> -   CALL_EXPRS.  Return whether they are equivalent.  */
> +/* Subroutine of cp_tree_equal: t1 and t2 are two CALL_EXPRs.
> +   Return whether their CALL_EXPR_FNs are equivalent.  */
>   
>   static bool
>   called_fns_equal (tree t1, tree t2)
>   {
>     /* Core 1321: dependent names are equivalent even if the overload sets
>        are different.  But do compare explicit template arguments.  */
> -  tree name1 = dependent_name (t1);
> -  tree name2 = dependent_name (t2);
> +  tree name1 = call_expr_dependent_name (t1);
> +  tree name2 = call_expr_dependent_name (t2);
> +  t1 = CALL_EXPR_FN (t1);
> +  t2 = CALL_EXPR_FN (t2);
>     if (name1 || name2)
>       {
>         tree targs1 = NULL_TREE, targs2 = NULL_TREE;
> @@ -4037,7 +4051,7 @@ cp_tree_equal (tree t1, tree t2)
>   	if (KOENIG_LOOKUP_P (t1) != KOENIG_LOOKUP_P (t2))
>   	  return false;
>   
> -	if (!called_fns_equal (CALL_EXPR_FN (t1), CALL_EXPR_FN (t2)))
> +	if (!called_fns_equal (t1, t2))
>   	  return false;
>   
>   	call_expr_arg_iterator iter1, iter2;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5.C b/gcc/testsuite/g++.dg/cpp0x/overload5.C
> new file mode 100644
> index 00000000000..e05b1594f51
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/overload5.C
> @@ -0,0 +1,12 @@
> +// PR c++/107461
> +// { dg-do compile { target c++11 } }
> +
> +int f(...);
> +template<class T> decltype(T() + f(0)) g(); // #1
> +
> +char f(int);
> +template<class T> decltype(T() + f(0)) g(); // #2, distinct from #1
> +
> +int main() {
> +  g<int>(); // { dg-error "ambiguous" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload5a.C b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
> new file mode 100644
> index 00000000000..037114f199c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/overload5a.C
> @@ -0,0 +1,10 @@
> +// PR c++/107461
> +// { dg-do compile { target c++11 } }
> +
> +template<class T> T f();
> +template<class T> decltype(T() + f<int*>()) g(); // #1
> +template<class T> decltype(T() + f<char>()) g(); // #2, distinct from #1
> +
> +int main() {
> +  g<int>(); // { dg-error "ambiguous" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/overload6.C b/gcc/testsuite/g++.dg/cpp0x/overload6.C
> new file mode 100644
> index 00000000000..1fbee0501de
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/overload6.C
> @@ -0,0 +1,16 @@
> +// PR c++/107461
> +// { dg-do compile { target c++11 } }
> +
> +template<class T> T f();
> +
> +template<class> struct A { };
> +
> +template<class T> struct B {
> +  template<class U, class = A<decltype(f<T>()(U()))>>
> +  static void g(U);
> +};
> +
> +int main() {
> +  B<int> b;
> +  B<void(*)(int)>::g(0); // { dg-bogus "no match" }
> +}
  

On Sat, Feb 04, 2023 at 06:02:46PM -0800, Jason Merrill via Gcc-patches wrote:
> On 2/4/23 20:41, Jason Merrill wrote:
> > On 2/4/23 20:08, Patrick Palka wrote:
> > > On Sat, 4 Feb 2023, Jason Merrill wrote:
> > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > > > index 255332dc0c1..c9360240cd2 100644
> > > > > --- a/gcc/cp/pt.cc
> > > > > +++ b/gcc/cp/pt.cc
> > > > > @@ -1841,6 +1841,7 @@ iterative_hash_template_arg (tree arg,
> > > > > hashval_t val)
> > > > >        case CALL_EXPR:
> > > > >          {
> > > > >        tree fn = CALL_EXPR_FN (arg);
> > > > > +    if (TREE_TYPE (arg) == NULL_TREE)
> > > > 
> > > > How about changing dependent_name to take the CALL_EXPR rather than the
> > > > CALL_EXPR_FN?  That would mean some changes to write_expression
> > > > to move the
> > > > dependent_name handling into the CALL_EXPR handling, but that
> > > > doesn't seem
> > > > like a bad thing.  Other callers seem like a trivial change.
> > > 
> > > Indeed changing dependent_name seems best, but I'm worried about such a
> > > refactoring to write_expression causing unintended mangling changes at
> > > this stage.  Because it seems the CALL_EXPR case of write_expression
> > > isn't the user of the dependent_name branch of write_expression, at
> > > least according to the following patch which causes us to ICE on
> > > mangle{37,57,58,76}.C:
> > 
> > Yeah, I tried the same thing.  Maybe for GCC 13 better to add a new
> > function rather than change the current one.
> 
> mangle76 seems like a bug where we're producing (and testing for) the wrong
> mangling -- mangling (*this). that doesn't exist in the source. clang gets
> it right.

Yes, this is https://gcc.gnu.org/PR98756.
 
> mangle5{7,8} has the right mangling, we're just using dependent_name to
> mangle function names that aren't dependent names (because they're template
> arguments in both cases, and qualified in the latter).

Marek