diff mbox series

c++: non-standalone surrogate call template

Message ID 20230712184747.3213450-1-ppalka@redhat.com
State New
Headers show
Series c++: non-standalone surrogate call template | expand

Commit Message

Patrick Palka July 12, 2023, 6:47 p.m. UTC
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  There might be an existing PR for this issue but Bugzilla search
seems to be timing out for me currently.

-- >8 --

I noticed we were accidentally preventing ourselves from considering
a pointer/reference-to-function conversion function template if it's
not the first conversion function that's considered, which for the
testcase below resulted in us accepting the B call but not the A call
despite the only difference between A and B being the order of member
declarations.  This patch fixes this so that the outcome of overload
resolution doesn't arbitrarily depend on declaration order in this
situation.

gcc/cp/ChangeLog:

	* call.cc (add_template_conv_candidate): Don't check for
	non-empty 'candidates' here.
	(build_op_call): Check it here, before we've considered any
	conversion functions.

gcc/testsuite/ChangeLog:

	* g++.dg/overload/conv-op5.C: New test.
---
 gcc/cp/call.cc                           | 24 ++++++++++++++----------
 gcc/testsuite/g++.dg/overload/conv-op5.C | 18 ++++++++++++++++++
 2 files changed, 32 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/overload/conv-op5.C

Comments

Jason Merrill July 17, 2023, 5:44 p.m. UTC | #1
On 7/12/23 14:47, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?  There might be an existing PR for this issue but Bugzilla search
> seems to be timing out for me currently.

OK.

> -- >8 --
> 
> I noticed we were accidentally preventing ourselves from considering
> a pointer/reference-to-function conversion function template if it's
> not the first conversion function that's considered, which for the
> testcase below resulted in us accepting the B call but not the A call
> despite the only difference between A and B being the order of member
> declarations.  This patch fixes this so that the outcome of overload
> resolution doesn't arbitrarily depend on declaration order in this
> situation.
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (add_template_conv_candidate): Don't check for
> 	non-empty 'candidates' here.
> 	(build_op_call): Check it here, before we've considered any
> 	conversion functions.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/overload/conv-op5.C: New test.
> ---
>   gcc/cp/call.cc                           | 24 ++++++++++++++----------
>   gcc/testsuite/g++.dg/overload/conv-op5.C | 18 ++++++++++++++++++
>   2 files changed, 32 insertions(+), 10 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/overload/conv-op5.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 81935b83908..119063979fa 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -3709,12 +3709,6 @@ add_template_conv_candidate (struct z_candidate **candidates, tree tmpl,
>   			     tree return_type, tree access_path,
>   			     tree conversion_path, tsubst_flags_t complain)
>   {
> -  /* Making this work broke PR 71117 and 85118, so until the committee resolves
> -     core issue 2189, let's disable this candidate if there are any call
> -     operators.  */
> -  if (*candidates)
> -    return NULL;
> -
>     return
>       add_template_candidate_real (candidates, tmpl, NULL_TREE, NULL_TREE,
>   				 NULL_TREE, arglist, return_type, access_path,
> @@ -5290,6 +5284,8 @@ build_op_call (tree obj, vec<tree, va_gc> **args, tsubst_flags_t complain)
>   		      LOOKUP_NORMAL, &candidates, complain);
>       }
>   
> +  bool any_call_ops = candidates != nullptr;
> +
>     convs = lookup_conversions (type);
>   
>     for (; convs; convs = TREE_CHAIN (convs))
> @@ -5306,10 +5302,18 @@ build_op_call (tree obj, vec<tree, va_gc> **args, tsubst_flags_t complain)
>   	      continue;
>   
>   	    if (TREE_CODE (fn) == TEMPLATE_DECL)
> -	      add_template_conv_candidate
> -		(&candidates, fn, obj, *args, totype,
> -		 /*access_path=*/NULL_TREE,
> -		 /*conversion_path=*/NULL_TREE, complain);
> +	      {
> +		/* Making this work broke PR 71117 and 85118, so until the
> +		   committee resolves core issue 2189, let's disable this
> +		   candidate if there are any call operators.  */
> +		if (any_call_ops)
> +		  continue;
> +
> +		add_template_conv_candidate
> +		  (&candidates, fn, obj, *args, totype,
> +		   /*access_path=*/NULL_TREE,
> +		   /*conversion_path=*/NULL_TREE, complain);
> +	      }
>   	    else
>   	      add_conv_candidate (&candidates, fn, obj,
>   				  *args, /*conversion_path=*/NULL_TREE,
> diff --git a/gcc/testsuite/g++.dg/overload/conv-op5.C b/gcc/testsuite/g++.dg/overload/conv-op5.C
> new file mode 100644
> index 00000000000..b7724908b62
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/overload/conv-op5.C
> @@ -0,0 +1,18 @@
> +// { dg-do compile { target c++11 } }
> +
> +template<class T> using F = int(*)(T);
> +using G = int(*)(int*);
> +
> +struct A {
> +  template<class T> operator F<T>();  // #1
> +  operator G() = delete; // #2
> +};
> +
> +int i = A{}(0); // selects #1
> +
> +struct B {
> +  operator G() = delete; // #2
> +  template<class T> operator F<T>();  // #1
> +};
> +
> +int j = B{}(0); // selects #1
diff mbox series

Patch

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 81935b83908..119063979fa 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -3709,12 +3709,6 @@  add_template_conv_candidate (struct z_candidate **candidates, tree tmpl,
 			     tree return_type, tree access_path,
 			     tree conversion_path, tsubst_flags_t complain)
 {
-  /* Making this work broke PR 71117 and 85118, so until the committee resolves
-     core issue 2189, let's disable this candidate if there are any call
-     operators.  */
-  if (*candidates)
-    return NULL;
-
   return
     add_template_candidate_real (candidates, tmpl, NULL_TREE, NULL_TREE,
 				 NULL_TREE, arglist, return_type, access_path,
@@ -5290,6 +5284,8 @@  build_op_call (tree obj, vec<tree, va_gc> **args, tsubst_flags_t complain)
 		      LOOKUP_NORMAL, &candidates, complain);
     }
 
+  bool any_call_ops = candidates != nullptr;
+
   convs = lookup_conversions (type);
 
   for (; convs; convs = TREE_CHAIN (convs))
@@ -5306,10 +5302,18 @@  build_op_call (tree obj, vec<tree, va_gc> **args, tsubst_flags_t complain)
 	      continue;
 
 	    if (TREE_CODE (fn) == TEMPLATE_DECL)
-	      add_template_conv_candidate
-		(&candidates, fn, obj, *args, totype,
-		 /*access_path=*/NULL_TREE,
-		 /*conversion_path=*/NULL_TREE, complain);
+	      {
+		/* Making this work broke PR 71117 and 85118, so until the
+		   committee resolves core issue 2189, let's disable this
+		   candidate if there are any call operators.  */
+		if (any_call_ops)
+		  continue;
+
+		add_template_conv_candidate
+		  (&candidates, fn, obj, *args, totype,
+		   /*access_path=*/NULL_TREE,
+		   /*conversion_path=*/NULL_TREE, complain);
+	      }
 	    else
 	      add_conv_candidate (&candidates, fn, obj,
 				  *args, /*conversion_path=*/NULL_TREE,
diff --git a/gcc/testsuite/g++.dg/overload/conv-op5.C b/gcc/testsuite/g++.dg/overload/conv-op5.C
new file mode 100644
index 00000000000..b7724908b62
--- /dev/null
+++ b/gcc/testsuite/g++.dg/overload/conv-op5.C
@@ -0,0 +1,18 @@ 
+// { dg-do compile { target c++11 } }
+
+template<class T> using F = int(*)(T);
+using G = int(*)(int*);
+
+struct A {
+  template<class T> operator F<T>();  // #1
+  operator G() = delete; // #2
+};
+
+int i = A{}(0); // selects #1
+
+struct B {
+  operator G() = delete; // #2
+  template<class T> operator F<T>();  // #1
+};
+
+int j = B{}(0); // selects #1