[PATCH] c++: ambiguous member lookup for rewritten cands [PR113529]

[PATCH] c++: ambiguous member lookup for rewritten cands [PR113529]

[Patrick Palka]

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk/13?

-- >8 --

Here we handle the operator expression u < v inconsistently: in a SFINAE
context (such as a requires-expression) we accept the it, and in a
non-SFINAE context we reject it with

  error: request for member 'operator<=>' is ambiguous

as per [class.member.lookup]/6.  This inconsistency is ultimately
because we neglect to propagate error_mark_node after recursing in
add_operator_candidates, fixed like so.

	PR c++/113529

gcc/cp/ChangeLog:

	* call.cc (add_operator_candidates): Propagate error_mark_node
	result after recursing to find rewritten candidates.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/spaceship-sfinae3.C: New test.
---
 gcc/cp/call.cc                                | 11 +++++++---
 .../g++.dg/cpp2a/spaceship-sfinae3.C          | 22 +++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-sfinae3.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 1f5ff417c81..183bb8c1348 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -7007,9 +7007,14 @@ add_operator_candidates (z_candidate **candidates,
 	{
 	  flags |= LOOKUP_REWRITTEN;
 	  if (rewrite_code != code)
-	    /* Add rewritten candidates in same order.  */
-	    add_operator_candidates (candidates, rewrite_code, ERROR_MARK,
-				     arglist, lookups, flags, complain);
+	    {
+	      /* Add rewritten candidates in same order.  */
+	      tree r = add_operator_candidates (candidates, rewrite_code,
+						ERROR_MARK, arglist, lookups,
+						flags, complain);
+	      if (r == error_mark_node)
+		return error_mark_node;
+	    }
 
 	  z_candidate *save_cand = *candidates;
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-sfinae3.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-sfinae3.C
new file mode 100644
index 00000000000..ca159260ec7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-sfinae3.C
@@ -0,0 +1,22 @@
+// PR c++/113529
+// { dg-do compile { target c++20 } }
+
+#include <compare>
+
+struct A {
+  auto operator<=>(const A&) const = default;
+  bool operator<(const A&) const;
+};
+struct B {
+  auto operator<=>(const B&) const = default;
+};
+struct C : A, B { };
+
+
+template<class T>
+void f(T u, T v) {
+  static_assert(!requires { u < v; });
+  u < v; // { dg-error "request for member 'operator<=>' is ambiguous" }
+}
+
+template void f(C, C);
-- 
2.43.0.386.ge02ecfcc53

Re: [PATCH] c++: ambiguous member lookup for rewritten cands [PR113529]

[Jason Merrill]

On 1/22/24 13:11, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> OK for trunk/13?
> 
> -- >8 --
> 
> Here we handle the operator expression u < v inconsistently: in a SFINAE
> context (such as a requires-expression) we accept the it, and in a
> non-SFINAE context we reject it with
> 
>    error: request for member 'operator<=>' is ambiguous
> 
> as per [class.member.lookup]/6.  This inconsistency is ultimately
> because we neglect to propagate error_mark_node after recursing in
> add_operator_candidates, fixed like so.

Shouldn't we do the same with the other recursive call?

Please also add return value documentation to the comment before the 
function.

OK with those changes.

Jason

Re: [PATCH] c++: ambiguous member lookup for rewritten cands [PR113529]

[Patrick Palka]

On Tue, 23 Jan 2024, Jason Merrill wrote:

> On 1/22/24 13:11, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > OK for trunk/13?
> > 
> > -- >8 --
> > 
> > Here we handle the operator expression u < v inconsistently: in a SFINAE
> > context (such as a requires-expression) we accept the it, and in a
> > non-SFINAE context we reject it with
> > 
> >    error: request for member 'operator<=>' is ambiguous
> > 
> > as per [class.member.lookup]/6.  This inconsistency is ultimately
> > because we neglect to propagate error_mark_node after recursing in
> > add_operator_candidates, fixed like so.
> 
> Shouldn't we do the same with the other recursive call?

Oops, yes.  I thought that the function is symmetric with respect to
member lookup so it should suffice to check the first recursive call,
but member lookup is only performed for the first argument type, and
arguments are swapped for the second recursive call.

> 
> Please also add return value documentation to the comment before the function.

Will do, thanks!

> 
> OK with those changes.
> 
> Jason
> 
>