diff mbox series

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

Message ID 20240122181153.876572-1-ppalka@redhat.com
State Accepted
Headers
Series c++: ambiguous member lookup for rewritten cands [PR113529] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Patrick Palka Jan. 22, 2024, 6:11 p.m. UTC 
  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

Comments

Jason Merrill Jan. 23, 2024, 8:56 p.m. UTC | #1 
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
Patrick Palka Jan. 23, 2024, 9:23 p.m. UTC | #2 
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
> 
>
diff mbox series

Patch

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