Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recognize friend operator != to fix ambiguity with operator== #70217

Closed
wants to merge 4 commits into from

Conversation

usx95
Copy link
Contributor

@usx95 usx95 commented Oct 25, 2023

The namespace lookup should give a matching friend operator!= as friend functions belong to the namespace scope (as opposed to the lexical class scope).

Thus to resolve ambiguity of operator== with itself, defining a corresponding friend operator!= should suffice.

Fixes: #70210

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 25, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 25, 2023

@llvm/pr-subscribers-clang

Author: Utkarsh Saxena (usx95)

Changes

The namespace lookup should give a matching friend operator!= as friend functions belong to the namespace scope (as opposed to the lexical class scope).

Thus to resolve ambiguity of operator== with itself, defining a corresponding friend operator!= should suffice.

Fixes: #70210


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

2 Files Affected:

  • (modified) clang/lib/Sema/SemaOverload.cpp (+6-3)
  • (modified) clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp (+23)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index db386fef0661c05..3b6b474886340eb 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -962,13 +962,16 @@ static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc,
     return true;
   }
   // Otherwise the search scope is the namespace scope of which F is a member.
-  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+  DeclContext *EqDC = EqFD->getEnclosingNamespaceContext();
+  for (NamedDecl *Op : EqDC->lookup(NotEqOp)) {
     auto *NotEqFD = Op->getAsFunction();
+    DeclContext *NotEqDC = Op->getFriendObjectKind()
+                               ? NotEqFD->getEnclosingNamespaceContext()
+                               : Op->getLexicalDeclContext();
     if (auto *UD = dyn_cast<UsingShadowDecl>(Op))
       NotEqFD = UD->getUnderlyingDecl()->getAsFunction();
     if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) && S.isVisible(NotEqFD) &&
-        declaresSameEntity(cast<Decl>(EqFD->getEnclosingNamespaceContext()),
-                           cast<Decl>(Op->getLexicalDeclContext())))
+        declaresSameEntity(cast<Decl>(EqDC), cast<Decl>(NotEqDC)))
       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 d83a176ec07eec9..603c91bd535f434 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,29 @@ bool x = X() == X(); // expected-warning {{ambiguous}}
 }
 } // namespace P2468R2
 
+namespace friend_opNE_GH{
+namespace test1 {
+struct S {
+    operator int();
+    friend bool operator==(const S &, int); // expected-note {{reversed}}
+};
+struct A : S {};
+struct B : S {};
+bool x = A{} == B{}; // expected-warning {{ambiguous}}
+} // namespace test1
+
+namespace test2 {
+struct S {
+    operator int();
+    friend bool operator==(const S &, int);
+    friend bool operator!=(const S &, int);
+};
+struct A : public P {};
+struct B : public P {};
+bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}}
+} // namespace test2
+} // namespace friend_opNE_GH
+
 namespace ADL_GH68901{
 namespace test1 {
 namespace A {

@github-actions
Copy link

github-actions bot commented Oct 25, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@usx95
Copy link
Contributor Author

usx95 commented Oct 27, 2023

Thanks @zygoloid for filing the core issue https://cplusplus.github.io/CWG/issues/2804.html

Would it make sense to go forward with this change and fix rewrite targets for friend operators (as a clang extension)?

@usx95
Copy link
Contributor Author

usx95 commented Jan 13, 2024

Will wait for the CWG to give final guidance.

@usx95 usx95 closed this Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang doesn't recognize friend operator!= to resolve ambiguity in operator==
2 participants