[Sema] Suppress lookup diagnostics when checking reversed operators

[ilya-biryukov]

Summary

The standard specified that Clang should 'search' for members in this
case, which does not involve full lookup and the corresponding checks.

See [over.match.oper]p4 and [basic.lookup.general]p3.

The added test has an example code that breaks.
The change that exposed this behavior is cc1b6668c57170cd440d321037ced89d6a61a9cb.

[ilya-biryukov]

clang/lib/Sema/SemaOverload.cpp

967

I believe we should be able to expose the same errorenous behavior here, but I did not come up with a test yet, so did not change it yet.
We should update it and come up with a test before the change lands.

But I still wanted to get early feedback on the direction of this patch, so sending for review right away.

[ilya-biryukov]

@shafik, @rsmith friendly ping for a review.

Also adding clang-language-wg for a wider set of potential reviewers.

[rsmith]

The prior behavior of Clang is correct. A search of a class scope is ill-formed if it finds an ambiguous result, see http://eel.is/c++draft/basic.lookup#class.member.lookup-6.sentence-2

[ilya-biryukov]

Thanks for clarifying! I misread the standard.

clang/lib/Sema/SemaOverload.cpp

if (isa<CXXMethodDecl>(EqFD)) {
     // If F is a class member, search scope is class type of first operand.
     QualType RHS = FirstOperand->getType();
     auto *RHSRec = RHS->getAs<RecordType>();
     if (!RHSRec)
       return true;
     LookupResult Members(S, NotEqOp, OpLoc,
                          Sema::LookupNameKind::LookupMemberName);
     S.LookupQualifiedName(Members, RHSRec->getDecl());
-    Members.suppressAccessDiagnostics();
+    // According to [over.match.oper]p4 we should run "search" and not "lookup"
+    // for reversed operators, so suppress diagnostics.
+    Members.suppressDiagnostics();
+
     for (NamedDecl *Op : Members)
       if (FunctionsCorrespond(S.Context, EqFD, Op->getAsFunction()))
         return false;
     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())))
       return false;

clang/test/SemaCXX/cxx20-reversed-operators-search.cpp

+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// expected-no-diagnostics
+
+
+namespace members {
+
+struct X {
+  virtual ~X();
+  virtual bool operator ==(X);
+  bool operator !=(X);
+};
+
+struct Y {
+  virtual ~Y();
+  virtual bool operator ==(Y);
+  bool operator !=(Y);
+};
+
+struct Z : X, Y {
+  virtual bool operator==(Z);
+  bool operator==(X) override;
+  bool operator==(Y) override;
+};
+
+void test() {
+  bool b = Z() == Z();
+}
+
+} // namespace members
+