[clang] Use the correct namespace for looking up matching operator!= (PR #68922)

Utkarsh Saxena via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 20 02:26:24 PDT 2023


https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/68922

>From 0c71b6bdd557ff4b9ad9a4ec4f0919e3460596b0 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Thu, 12 Oct 2023 21:35:52 +0200
Subject: [PATCH 1/5] Find opertor!= in the correct namespace

---
 clang/lib/Sema/SemaOverload.cpp               |  7 +-----
 .../over.match.oper/p3-2a.cpp                 | 24 +++++++++++++++++++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index ce78994e6553814..e1e0b4a888e49fa 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -960,12 +960,7 @@ static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc,
     return true;
   }
   // Otherwise the search scope is the namespace scope of which F is a member.
-  LookupResult NonMembers(S, NotEqOp, OpLoc,
-                          Sema::LookupNameKind::LookupOperatorName);
-  S.LookupName(NonMembers,
-               S.getScopeForContext(EqFD->getEnclosingNamespaceContext()));
-  NonMembers.suppressAccessDiagnostics();
-  for (NamedDecl *Op : NonMembers) {
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
     auto *FD = Op->getAsFunction();
     if(auto* UD = dyn_cast<UsingShadowDecl>(Op))
       FD = UD->getUnderlyingDecl()->getAsFunction();
diff --git a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
index 5c6804eb7726b5f..7142ed22ddb22ca 100644
--- a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
+++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
@@ -324,6 +324,30 @@ bool x = X() == X(); // expected-warning {{ambiguous}}
 }
 } // namespace P2468R2
 
+namespace ADL_GH68901{
+namespace A {
+struct S {};
+bool operator==(S, int); // expected-note {{no known conversion from 'int' to 'S' for 1st argument}}
+bool operator!=(S, int);
+} // namespace A
+bool a = 0 == A::S(); // expected-error {{invalid operands to binary expression ('int' and 'A::S')}}
+
+namespace B {
+struct Derived {};
+struct Base : Derived {};
+
+bool operator==(Derived& a, Base& b);
+bool operator!=(Derived& a, Base& b);
+}  // namespace B
+
+void foo() {
+  // B::Base{} == B::Base{};
+  B::Base a,b;
+  bool v = a == b;
+}
+} //namespace ADL_GH68901
+
+
 #else // NO_ERRORS
 
 namespace problem_cases {

>From eb7ac047b269d71e4fc7afbbb8b8f65e857ff3a3 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Thu, 12 Oct 2023 21:48:20 +0200
Subject: [PATCH 2/5] Add release notes

---
 clang/docs/ReleaseNotes.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2d918967e7f0b02..c5b5f665ce9834c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -64,6 +64,10 @@ C/C++ Language Potentially Breaking Changes
     ``__has_c_attribute(warn_unused_result)``,          202003, 0
     ``__has_c_attribute(gnu::warn_unused_result)``,     202003, 1
 
+- Fixed a bug in finding matching `operator!=` while adding reveresed `operator==` as
+  outlined in "The Equality Operator You Are Looking For" (`P2468 <http://wg21.link/p2468r2>`_).
+  Fixes (`#68901: <https://github.com/llvm/llvm-project/issues/68901>`_).
+
 C++ Specific Potentially Breaking Changes
 -----------------------------------------
 - The name mangling rules for function templates has been changed to take into

>From 73d48e546775290196c5a5fbe2922d26df29f013 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 13 Oct 2023 12:28:29 +0200
Subject: [PATCH 3/5] fix a typo

---
 clang/docs/ReleaseNotes.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c5b5f665ce9834c..88cbf1d864da400 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -64,7 +64,7 @@ C/C++ Language Potentially Breaking Changes
     ``__has_c_attribute(warn_unused_result)``,          202003, 0
     ``__has_c_attribute(gnu::warn_unused_result)``,     202003, 1
 
-- Fixed a bug in finding matching `operator!=` while adding reveresed `operator==` as
+- Fixed a bug in finding matching `operator!=` while adding reversed `operator==` as
   outlined in "The Equality Operator You Are Looking For" (`P2468 <http://wg21.link/p2468r2>`_).
   Fixes (`#68901: <https://github.com/llvm/llvm-project/issues/68901>`_).
 

>From 8e26dd5732e4cef11a3f3c6e278147b11ae01ac8 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Mon, 16 Oct 2023 12:15:05 +0200
Subject: [PATCH 4/5] Add tests for function scope operators

---
 clang/lib/Sema/SemaOverload.cpp               | 12 +++----
 .../over.match.oper/p3-2a.cpp                 | 32 +++++++++++++++++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index e1e0b4a888e49fa..773310f69d0551b 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -961,12 +961,12 @@ static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc,
   }
   // Otherwise the search scope is the namespace scope of which F is a member.
   for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
-    auto *FD = Op->getAsFunction();
-    if(auto* UD = dyn_cast<UsingShadowDecl>(Op))
-      FD = UD->getUnderlyingDecl()->getAsFunction();
-    if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-        declaresSameEntity(cast<Decl>(EqFD->getDeclContext()),
-                           cast<Decl>(Op->getDeclContext())))
+    auto *NotEqFD = Op->getAsFunction();
+    if (auto *UD = dyn_cast<UsingShadowDecl>(Op))
+      NotEqFD = UD->getUnderlyingDecl()->getAsFunction();
+    if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) &&
+        declaresSameEntity(cast<Decl>(EqFD->getEnclosingNamespaceContext()),
+                           cast<Decl>(Op->getLexicalDeclContext())))
       return false;
   }
   return true;
diff --git a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
index 7142ed22ddb22ca..d70264c21fba861 100644
--- a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
+++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
@@ -347,6 +347,38 @@ void foo() {
 }
 } //namespace ADL_GH68901
 
+namespace function_scope_operator_eqeq {
+// For non-members, we always lookup for matching operator!= in the namespace scope of 
+// operator== (and not in the scope of operator==).
+struct X { operator int(); };
+namespace test1{
+bool h(X x) {
+  bool operator==(X, int); // expected-note {{reversed}}
+  return x == x; // expected-warning {{ambiguous}}
+}
+
+bool g(X x) {
+  bool operator==(X, int); // expected-note {{reversed}}
+  bool operator!=(X, int);
+  return x == x;  // expected-warning {{ambiguous}}
+}
+} // namespace test1
+
+namespace test2 {
+bool operator!=(X, int);
+
+bool h(X x) {
+  bool operator==(X, int);
+  return x == x;
+}
+
+bool i(X x) {
+  bool operator==(X, int);
+  bool operator!=(X, int);
+  return x == x;
+}
+} // namespace test2
+} // namespace function_scope_operator_eqeq
 
 #else // NO_ERRORS
 

>From 8804fe6224e4b9ae1ab97127e579ea47d65b6a2a Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 20 Oct 2023 11:26:08 +0200
Subject: [PATCH 5/5] add more test

---
 .../over.match.oper/p3-2a.cpp                 | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
index d70264c21fba861..c3d5d67f37355d6 100644
--- a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
+++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
@@ -345,6 +345,27 @@ void foo() {
   B::Base a,b;
   bool v = a == b;
 }
+
+namespace ns {
+template <class T>
+struct A {
+};
+
+template <class T> 
+struct B : A<T> {
+};
+
+template <class T> bool operator == (B<T>, A<T>); // expected-note {{candidate template ignored: could not match 'B' against 'A'}}
+template <class T> bool operator != (B<T>, A<T>);
+}
+
+void test() {
+    ns::A<int> a;
+    ns::B<int> b;
+    a == b; // expected-error {{invalid operands to binary expression}}
+}
+
+
 } //namespace ADL_GH68901
 
 namespace function_scope_operator_eqeq {



More information about the cfe-commits mailing list