[c++20] Delete defaulted comparison functions if they would invoke an

inaccessible comparison function.

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8220,6 +8220,10 @@ def note_defaulted_comparison_reference_member : Note<
 def note_defaulted_comparison_ambiguous : Note<
   "defaulted %0 is implicitly deleted because implied %select{|'==' |'<' }1"
   "comparison %select{|for member %3 |for base class %3 }2is ambiguous">;
+def note_defaulted_comparison_inaccessible : Note<
+  "defaulted %0 is implicitly deleted because it would invoke a "
+  "%select{private|protected}3 %4%select{ member of %6|"
+  " member of %6 to compare member %2| to compare base class %2}1">;
 def note_defaulted_comparison_calls_deleted : Note<
   "defaulted %0 is implicitly deleted because it would invoke a deleted "
   "comparison function%select{| for member %2| for base class %2}1">;

clang/include/clang/Sema/Sema.h

@@ -6664,9 +6664,16 @@ public:
   void CheckLookupAccess(const LookupResult &R);
   bool IsSimplyAccessible(NamedDecl *Decl, CXXRecordDecl *NamingClass,
                           QualType BaseType);
-  bool isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
-                                            AccessSpecifier access,
-                                            QualType objectType);
+  bool isMemberAccessibleForDeletion(CXXRecordDecl *NamingClass,
+                                     DeclAccessPair Found, QualType ObjectType,
+                                     SourceLocation Loc,
+                                     const PartialDiagnostic &Diag);
+  bool isMemberAccessibleForDeletion(CXXRecordDecl *NamingClass,
+                                     DeclAccessPair Found,
+                                     QualType ObjectType) {
+    return isMemberAccessibleForDeletion(NamingClass, Found, ObjectType,
+                                         SourceLocation(), PDiag());
+  }
 
   void HandleDependentAccessCheck(const DependentDiagnostic &DD,
                          const MultiLevelTemplateArgumentList &TemplateArgs);

clang/lib/Sema/SemaAccess.cpp

@@ -1560,21 +1560,24 @@ Sema::AccessResult Sema::CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E,
   return CheckAccess(*this, E->getMemberLoc(), Entity);
 }
 
-/// Is the given special member function accessible for the purposes of
-/// deciding whether to define a special member function as deleted?
-bool Sema::isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
-                                                AccessSpecifier access,
-                                                QualType objectType) {
+/// Is the given member accessible for the purposes of deciding whether to
+/// define a special member function as deleted?
+bool Sema::isMemberAccessibleForDeletion(CXXRecordDecl *NamingClass,
+                                         DeclAccessPair Found,
+                                         QualType ObjectType,
+                                         SourceLocation Loc,
+                                         const PartialDiagnostic &Diag) {
   // Fast path.
-  if (access == AS_public || !getLangOpts().AccessControl) return true;
+  if (Found.getAccess() == AS_public || !getLangOpts().AccessControl)
+    return true;
 
-  AccessTarget entity(Context, AccessTarget::Member, decl->getParent(),
-                      DeclAccessPair::make(decl, access), objectType);
+  AccessTarget Entity(Context, AccessTarget::Member, NamingClass, Found,
+                      ObjectType);
 
   // Suppress diagnostics.
-  entity.setDiag(PDiag());
+  Entity.setDiag(Diag);
 
-  switch (CheckAccess(*this, SourceLocation(), entity)) {
+  switch (CheckAccess(*this, Loc, Entity)) {
   case AR_accessible: return true;
   case AR_inaccessible: return false;
   case AR_dependent: llvm_unreachable("dependent for =delete computation");

clang/lib/Sema/SemaDeclCXX.cpp

@@ -7337,7 +7337,7 @@ private:
 
     OverloadCandidateSet::iterator Best;
     switch (CandidateSet.BestViableFunction(S, FD->getLocation(), Best)) {
-    case OR_Success:
+    case OR_Success: {
       // C++2a [class.compare.secondary]p2 [P2002R0]:
       //   The operator function [...] is defined as deleted if [...] the
       //   candidate selected by overload resolution is not a rewritten
@@ -7353,6 +7353,28 @@ private:
         return Result::deleted();
       }
 
+      // Throughout C++2a [class.compare]: if overload resolution does not
+      // result in a usable function, the candidate function is defined as
+      // deleted. This requires that we selected an accessible function.
+      //
+      // Note that this only considers the access of the function when named
+      // within the type of the subobject, and not the access path for any
+      // derived-to-base conversion.
+      CXXRecordDecl *ArgClass = Args[0]->getType()->getAsCXXRecordDecl();
+      if (ArgClass && Best->FoundDecl.getDecl() &&
+          Best->FoundDecl.getDecl()->isCXXClassMember()) {
+        QualType ObjectType = Subobj.Kind == Subobject::Member
+                                  ? Args[0]->getType()
+                                  : S.Context.getRecordType(RD);
+        if (!S.isMemberAccessibleForDeletion(
+                ArgClass, Best->FoundDecl, ObjectType, Subobj.Loc,
+                Diagnose == ExplainDeleted
+                    ? S.PDiag(diag::note_defaulted_comparison_inaccessible)
+                          << FD << Subobj.Kind << Subobj.Decl
+                    : S.PDiag()))
+          return Result::deleted();
+      }
+
       // C++2a [class.compare.default]p3 [P2002R0]:
       //   A defaulted comparison function is constexpr-compatible if [...]
       //   no overlod resolution performed [...] results in a non-constexpr
@@ -7399,6 +7421,7 @@ private:
       // Note that we might be rewriting to a different operator. That call is
       // not considered until we come to actually build the comparison function.
       break;
+    }
 
     case OR_Ambiguous:
       if (Diagnose == ExplainDeleted) {
@@ -8303,7 +8326,8 @@ bool SpecialMemberDeletionInfo::isAccessible(Subobject Subobj,
     objectTy = S.Context.getTypeDeclType(target->getParent());
   }
 
-  return S.isSpecialMemberAccessibleForDeletion(target, access, objectTy);
+  return S.isMemberAccessibleForDeletion(
+      target->getParent(), DeclAccessPair::make(target, access), objectTy);
 }
 
 /// Check whether we should delete a special member due to the implicit

clang/test/CXX/class/class.compare/class.eq/p2.cpp

@@ -47,3 +47,68 @@ void test() {
   void(X<A[3]>() == X<A[3]>()); // expected-error {{cannot be compared because its 'operator==' is implicitly deleted}}
   void(X<B[3]>() == X<B[3]>());
 }
+
+namespace Access {
+  class A {
+    bool operator==(const A &) const; // expected-note 2{{implicitly declared private here}}
+  };
+  struct B : A { // expected-note 2{{because it would invoke a private 'operator==' to compare base class 'A'}}
+    bool operator==(const B &) const = default; // expected-warning {{deleted}}
+    friend bool operator==(const B &, const B &) = default; // expected-warning {{deleted}}
+  };
+
+  class C {
+  protected:
+    bool operator==(const C &) const; // expected-note 2{{declared protected here}}
+  };
+  struct D : C {
+    bool operator==(const D &) const = default;
+    friend bool operator==(const D &, const D&) = default;
+  };
+  struct E {
+    C c; // expected-note 2{{because it would invoke a protected 'operator==' member of 'Access::C' to compare member 'c'}}
+    bool operator==(const E &) const = default; // expected-warning {{deleted}}
+    friend bool operator==(const E &, const E &) = default; // expected-warning {{deleted}}
+  };
+
+  struct F : C {
+    using C::operator==;
+  };
+  struct G : F {
+    bool operator==(const G&) const = default;
+    friend bool operator==(const G&, const G&) = default;
+  };
+
+  struct H : C {
+  private:
+    using C::operator==; // expected-note 2{{declared private here}}
+  };
+  struct I : H { // expected-note 2{{private 'operator==' to compare base class 'H'}}
+    bool operator==(const I&) const = default; // expected-warning {{deleted}}
+    friend bool operator==(const I&, const I&) = default; // expected-warning {{deleted}}
+  };
+
+  class J {
+    bool operator==(const J&) const;
+    friend class K;
+  };
+  class K {
+    J j;
+    bool operator==(const K&) const = default;
+    friend bool operator==(const K&, const K&) = default;
+  };
+
+  struct X {
+    bool operator==(const X&) const; // expected-note {{ambiguity is between a regular call to this operator and a call with the argument order reversed}}
+  };
+  struct Y : private X { // expected-note {{private}}
+    using X::operator==;
+  };
+  struct Z : Y {
+    // Note: this function is not deleted. The selected operator== is
+    // accessible. But the derived-to-base conversion involves an inaccessible
+    // base class, which we don't check for until we define the function.
+    bool operator==(const Z&) const = default; // expected-error {{cannot cast 'const Access::Y' to its private base class 'const Access::X'}} expected-warning {{ambiguous}}
+  };
+  bool z = Z() == Z(); // expected-note {{first required here}}
+}

clang/test/CXX/class/class.compare/class.rel/p2.cpp

@@ -72,3 +72,13 @@ namespace NotEqual {
     bool operator!=(const Evil&) const = default; // expected-warning {{implicitly deleted}} expected-note {{would be the best match}}
   };
 }
+
+namespace Access {
+  class A {
+    int operator<=>(A) const; // expected-note {{private}}
+  };
+  struct B : A {
+    friend bool operator<(const B&, const B&) = default; // expected-warning {{implicitly deleted}}
+    // expected-note@-1 {{defaulted 'operator<' is implicitly deleted because it would invoke a private 'operator<=>' member of 'Access::A'}}
+  };
+}

clang/test/CXX/class/class.compare/class.spaceship/p1.cpp

@@ -141,6 +141,29 @@ namespace Deletedness {
   }
 }
 
+namespace Access {
+  class A {
+    std::strong_ordering operator<=>(const A &) const; // expected-note {{here}}
+  public:
+    bool operator==(const A &) const;
+    bool operator<(const A &) const;
+  };
+  struct B {
+    A a; // expected-note {{would invoke a private 'operator<=>'}}
+    friend std::strong_ordering operator<=>(const B &, const B &) = default; // expected-warning {{deleted}}
+  };
+
+  class C {
+    std::strong_ordering operator<=>(const C &); // not viable (not const)
+    bool operator==(const C &) const; // expected-note {{here}}
+    bool operator<(const C &) const;
+  };
+  struct D {
+    C c; // expected-note {{would invoke a private 'operator=='}}
+    friend std::strong_ordering operator<=>(const D &, const D &) = default; // expected-warning {{deleted}}
+  };
+}
+
 namespace Synthesis {
   enum Result { False, True, Mu };
 

clang/www/cxx_status.html

@@ -930,14 +930,15 @@ as the draft C++2a standard evolves.
     </tr>
       <tr> <!-- from Jacksonville -->
         <td><a href="https://wg21.link/p0905r1">P0905R1</a></td>
-        <td class="svn" align="center">Clang 10</td>
+        <td class="svn" align="center">SVN</td>
       </tr>
       <tr> <!-- from Rapperswil -->
         <td><a href="https://wg21.link/p1120r0">P1120R0</a></td>
-        <td rowspan="4" class="partial" align="center">Partial</td>
+        <td class="partial" align="center">Partial</td>
       </tr>
       <tr> <!-- from Kona 2019 -->
         <td><a href="https://wg21.link/p1185r2">P1185R2</a></td>
+        <td rowspan="3" class="svn" align="center">SVN</td>
       </tr>
       <tr> <!-- from Cologne -->
         <td><a href="https://wg21.link/p1186r3">P1186R3</a></td>