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

[Utkarsh Saxena via cfe-commits]

https://github.com/usx95 closed https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

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

>From 8aa439a97a56ef80bfc9ccc90a9f093680e455f5 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena 
Date: Sun, 22 Oct 2023 11:11:53 +0200
Subject: [PATCH 1/3] rebase...

---
 clang/docs/ReleaseNotes.rst   |  4 +
 clang/lib/Sema/SemaOverload.cpp   | 19 ++---
 .../over.match.oper/p3-2a.cpp | 78 +++
 3 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3a9a67f7451c092..cef857244387e8b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -85,6 +85,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 reversed 
`operator==` as
+  outlined in "The Equality Operator You Are Looking For" (`P2468 
`_).
+  Fixes (`#68901: `_).
+
 C++ Specific Potentially Breaking Changes
 -
 - The name mangling rules for function templates has been changed to take into
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d57a7ad8f46859a..75e4184c8aa6cb8 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();
+if (auto *UD = dyn_cast(Op))
+  NotEqFD = UD->getUnderlyingDecl()->getAsFunction();
+if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) &&
+declaresSameEntity(cast(EqFD->getEnclosingNamespaceContext()),
+   cast(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 5727d506fe5e61d..9dc5ee8db565341 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
@@ -374,6 +374,84 @@ bool fine(A a, B b) { return a == b; } // Ok. Found a 
matching operator!=.
 }
 }
 
+
+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 ns {
+template 
+struct A {
+};
+
+template 
+struct B : A {
+};
+
+template  bool operator == (B, A); // expected-note {{candidate 
template ignored: could not match 'B' against 'A'}}
+template  bool operator != (B, A);
+}
+
+void test() {
+ns::A a;
+ns::B b;
+a == b; // expected-error {{invalid operands to binary expression}}
+}
+
+
+} //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
+
 namespace 

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

[Ilya Biryukov via cfe-commits]

ilya-biryukov wrote:

Oh, one last thing! @usx95 maybe add a test for the visibility check?
Various tests with modules (search for files with the `.cppm` extension in the 
tests) should give a good starting point.

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via cfe-commits]

https://github.com/ilya-biryukov approved this pull request.


https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via cfe-commits]

@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();

ilya-biryukov wrote:

Ah, getAsFunction picks a primary template. LG and thanks for adding a test!

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via cfe-commits]

@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();
+if (auto *UD = dyn_cast(Op))
+  NotEqFD = UD->getUnderlyingDecl()->getAsFunction();
+if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) &&
+declaresSameEntity(cast(EqFD->getEnclosingNamespaceContext()),
+   cast(Op->getLexicalDeclContext(

ilya-biryukov wrote:

Sorry, I missed the `isVisible` call here. I agree with the rest, since this 
only disables reversed operator lookup, extra visibility checks from ADL should 
not be required.

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via cfe-commits]

https://github.com/ilya-biryukov edited 
https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

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

>From 8aa439a97a56ef80bfc9ccc90a9f093680e455f5 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena 
Date: Sun, 22 Oct 2023 11:11:53 +0200
Subject: [PATCH 1/2] rebase...

---
 clang/docs/ReleaseNotes.rst   |  4 +
 clang/lib/Sema/SemaOverload.cpp   | 19 ++---
 .../over.match.oper/p3-2a.cpp | 78 +++
 3 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3a9a67f7451c092..cef857244387e8b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -85,6 +85,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 reversed 
`operator==` as
+  outlined in "The Equality Operator You Are Looking For" (`P2468 
`_).
+  Fixes (`#68901: `_).
+
 C++ Specific Potentially Breaking Changes
 -
 - The name mangling rules for function templates has been changed to take into
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d57a7ad8f46859a..75e4184c8aa6cb8 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();
+if (auto *UD = dyn_cast(Op))
+  NotEqFD = UD->getUnderlyingDecl()->getAsFunction();
+if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) &&
+declaresSameEntity(cast(EqFD->getEnclosingNamespaceContext()),
+   cast(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 5727d506fe5e61d..9dc5ee8db565341 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
@@ -374,6 +374,84 @@ bool fine(A a, B b) { return a == b; } // Ok. Found a 
matching operator!=.
 }
 }
 
+
+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 ns {
+template 
+struct A {
+};
+
+template 
+struct B : A {
+};
+
+template  bool operator == (B, A); // expected-note {{candidate 
template ignored: could not match 'B' against 'A'}}
+template  bool operator != (B, A);
+}
+
+void test() {
+ns::A a;
+ns::B b;
+a == b; // expected-error {{invalid operands to binary expression}}
+}
+
+
+} //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
+
 namespace 

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

[Utkarsh Saxena via cfe-commits]

@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();
+if (auto *UD = dyn_cast(Op))
+  NotEqFD = UD->getUnderlyingDecl()->getAsFunction();
+if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) &&
+declaresSameEntity(cast(EqFD->getEnclosingNamespaceContext()),
+   cast(Op->getLexicalDeclContext(

usx95 wrote:

I do not think we have to follow ADL rules here. We only have to lookup in the 
NS. For visibility, `isVisible()` should suffice. Rest of the involved 
visibility rules are specific to ADL and should not concern the lookup for !=.

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

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

>From 8aa439a97a56ef80bfc9ccc90a9f093680e455f5 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena 
Date: Sun, 22 Oct 2023 11:11:53 +0200
Subject: [PATCH] rebase...

---
 clang/docs/ReleaseNotes.rst   |  4 +
 clang/lib/Sema/SemaOverload.cpp   | 19 ++---
 .../over.match.oper/p3-2a.cpp | 78 +++
 3 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3a9a67f7451c092..cef857244387e8b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -85,6 +85,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 reversed 
`operator==` as
+  outlined in "The Equality Operator You Are Looking For" (`P2468 
`_).
+  Fixes (`#68901: `_).
+
 C++ Specific Potentially Breaking Changes
 -
 - The name mangling rules for function templates has been changed to take into
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d57a7ad8f46859a..75e4184c8aa6cb8 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();
+if (auto *UD = dyn_cast(Op))
+  NotEqFD = UD->getUnderlyingDecl()->getAsFunction();
+if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) &&
+declaresSameEntity(cast(EqFD->getEnclosingNamespaceContext()),
+   cast(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 5727d506fe5e61d..9dc5ee8db565341 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
@@ -374,6 +374,84 @@ bool fine(A a, B b) { return a == b; } // Ok. Found a 
matching operator!=.
 }
 }
 
+
+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 ns {
+template 
+struct A {
+};
+
+template 
+struct B : A {
+};
+
+template  bool operator == (B, A); // expected-note {{candidate 
template ignored: could not match 'B' against 'A'}}
+template  bool operator != (B, A);
+}
+
+void test() {
+ns::A a;
+ns::B b;
+a == b; // expected-error {{invalid operands to binary expression}}
+}
+
+
+} //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
+
 namespace 

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

[Utkarsh Saxena via cfe-commits]

https://github.com/usx95 reopened 
https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

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 
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 , 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(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 
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 
`_).
+  Fixes (`#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 
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 
`_).
   Fixes (`#68901: `_).
 

>From 8e26dd5732e4cef11a3f3c6e278147b11ae01ac8 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena 
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 

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

[Utkarsh Saxena via cfe-commits]

https://github.com/usx95 edited https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

https://github.com/usx95 edited https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();

usx95 wrote:

Added this test. This fails now as expected.

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via cfe-commits]

@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();

ilya-biryukov wrote:

What about template functions? Something like:
```
namespace ns {
template 
struct A {
};

template  
struct B : A {
};

template  bool operator == (B, A);
template  bool operator != (B, A);
}

void test() {
ns::A a;
ns::B b;
a == b;
}
```
https://gcc.godbolt.org/z/sc36a7f81


https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via cfe-commits]

@@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema , 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) {
-auto *FD = Op->getAsFunction();
-if(auto* UD = dyn_cast(Op))
-  FD = UD->getUnderlyingDecl()->getAsFunction();
-if (FunctionsCorrespond(S.Context, EqFD, FD) &&
-declaresSameEntity(cast(EqFD->getDeclContext()),
-   cast(Op->getDeclContext(
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+auto *NotEqFD = Op->getAsFunction();
+if (auto *UD = dyn_cast(Op))
+  NotEqFD = UD->getUnderlyingDecl()->getAsFunction();
+if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) &&
+declaresSameEntity(cast(EqFD->getEnclosingNamespaceContext()),
+   cast(Op->getLexicalDeclContext(

ilya-biryukov wrote:

This seems similar to what [ADL 
lookup](https://github.com/llvm/llvm-project/blob/21e1b13f3384b875bd2205a736570320cb020f3e/clang/lib/Sema/SemaLookup.cpp#L3844)
 is doing (excluding the search of associated namespaces), but seems to miss 
the visibility check. 

It's quite a bizzare case when `operator ==` and `operator !=` are coming from 
different modules and only one of those is visible, but it would be nice to add 
checks that this works and handle that case correcly.
At that point, I believe it would also be best to share the code between the 
two functions as the check for visibility is quite involved.

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via cfe-commits]

https://github.com/ilya-biryukov requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via cfe-commits]

https://github.com/ilya-biryukov edited 
https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

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 
Date: Thu, 12 Oct 2023 21:35:52 +0200
Subject: [PATCH 1/4] 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 , 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(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 
Date: Thu, 12 Oct 2023 21:48:20 +0200
Subject: [PATCH 2/4] 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 
`_).
+  Fixes (`#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 
Date: Fri, 13 Oct 2023 12:28:29 +0200
Subject: [PATCH 3/4] 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 
`_).
   Fixes (`#68901: `_).
 

>From 8e26dd5732e4cef11a3f3c6e278147b11ae01ac8 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena 
Date: Mon, 16 Oct 2023 12:15:05 +0200
Subject: [PATCH 4/4] 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 

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

[Utkarsh Saxena via cfe-commits]

usx95 wrote:

Thanks @zygoloid.
This case was failing with my change. Corrected and added tests.

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

zygoloid wrote:

> > Does this work for function-scope operator declarations?
> 
> @zygoloid I am not sure I follow. Could you please give an example.

Sure:

```c++
struct X { operator int(); };

bool f(X x) {
  bool operator==(X, int);
  return x == x;
}


bool g(X x) {
  bool operator==(X, int);
  bool operator!=(X, int);
  return x == x;
}

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;
}
```

Based on the standard's rules, it looks like `f` and `g` should be invalid due 
to ambiguity, but `h` and `i` should be valid. (Which I think is a pretty 
surprising outcome, but that's what the rules say.)

If there's not already something like that in our test suite, it'd be good to 
add it.

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

usx95 wrote:

> Does this work for function-scope operator declarations?

@zygoloid I am not sure I follow. Could you please give an example. If this is 
about member operator, the search scope is class scope of the first operand.

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

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 
Date: Thu, 12 Oct 2023 21:35:52 +0200
Subject: [PATCH 1/3] 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 , 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(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 
Date: Thu, 12 Oct 2023 21:48:20 +0200
Subject: [PATCH 2/3] 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 
`_).
+  Fixes (`#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 
Date: Fri, 13 Oct 2023 12:28:29 +0200
Subject: [PATCH 3/3] 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 
`_).
   Fixes (`#68901: `_).
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

https://github.com/cor3ntin commented:

Looks reasonable, but i found a typo

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

@@ -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

cor3ntin wrote:

```suggestion
- Fixed a bug in finding matching `operator!=` while adding reversed 
`operator==` as
```

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

zygoloid wrote:

Does this work for function-scope operator declarations?

https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

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 
Date: Thu, 12 Oct 2023 21:35:52 +0200
Subject: [PATCH 1/2] 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 , 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(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 
Date: Thu, 12 Oct 2023 21:48:20 +0200
Subject: [PATCH 2/2] 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 
`_).
+  Fixes (`#68901: `_).
+
 C++ Specific Potentially Breaking Changes
 -
 - The name mangling rules for function templates has been changed to take into

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Utkarsh Saxena (usx95)


Changes

`S.getScopeForContext` determins the **active** scope associated with the given 
`declContext`. 
This fails to find the matching `operator!=` if candidate `operator==` was 
found via ADL since that scope is not active.

Instead, just directly lookup using the namespace decl of `operator==`

Fixes #68901

---
Full diff: https://github.com/llvm/llvm-project/pull/68922.diff


2 Files Affected:

- (modified) clang/lib/Sema/SemaOverload.cpp (+1-6) 
- (modified) 
clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp (+24) 


``diff
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 , 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(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 {

``




https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

https://github.com/usx95 ready_for_review 
https://github.com/llvm/llvm-project/pull/68922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Utkarsh Saxena via cfe-commits]

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

`S.getScopeForContext` determins the **active** scope associated with the given 
`declContext`. 
This fails to find the matching `operator!=` if candidate `operator==` was 
found via ADL since that scope is not active.

Instead, just directly lookup using the namespace decl of `operator==`

Fixes #68901

>From 0c71b6bdd557ff4b9ad9a4ec4f0919e3460596b0 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena 
Date: Thu, 12 Oct 2023 21:35:52 +0200
Subject: [PATCH] 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 , 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(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 {

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits