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

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 19 08:42:52 PDT 2023


================
@@ -960,18 +960,13 @@ 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) {
-    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())))
+  for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) {
+    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())))
----------------
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


More information about the cfe-commits mailing list