Author: Richard Smith Date: 2019-12-10T19:28:30-08:00 New Revision: 8e0c9e21bf5f3e7a427b07e3eaf3bc80d2c74cb6
URL: https://github.com/llvm/llvm-project/commit/8e0c9e21bf5f3e7a427b07e3eaf3bc80d2c74cb6 DIFF: https://github.com/llvm/llvm-project/commit/8e0c9e21bf5f3e7a427b07e3eaf3bc80d2c74cb6.diff LOG: [c++20] Delete defaulted comparison functions if they would invoke an inaccessible comparison function. Added: Modified: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaAccess.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/test/CXX/class/class.compare/class.eq/p2.cpp clang/test/CXX/class/class.compare/class.rel/p2.cpp clang/test/CXX/class/class.compare/class.spaceship/p1.cpp clang/www/cxx_status.html Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a5f35996cfdc..4eec4b066ae5 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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">; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 5a1ee507218c..53db210a0177 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6664,9 +6664,16 @@ class Sema final { 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); diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp index 9dbb93322b7d..bd15b81cbed0 100644 --- a/clang/lib/Sema/SemaAccess.cpp +++ b/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"); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 72ac81776aae..365286097699 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -7337,7 +7337,7 @@ class DefaultedComparisonAnalyzer 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 @@ class DefaultedComparisonAnalyzer 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 @@ class DefaultedComparisonAnalyzer // Note that we might be rewriting to a diff erent 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 diff --git a/clang/test/CXX/class/class.compare/class.eq/p2.cpp b/clang/test/CXX/class/class.compare/class.eq/p2.cpp index 55b2cc3c08f6..e5f4a020d5c8 100644 --- a/clang/test/CXX/class/class.compare/class.eq/p2.cpp +++ b/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}} +} diff --git a/clang/test/CXX/class/class.compare/class.rel/p2.cpp b/clang/test/CXX/class/class.compare/class.rel/p2.cpp index 2abe7fb8d79c..21a68f9f4c67 100644 --- a/clang/test/CXX/class/class.compare/class.rel/p2.cpp +++ b/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'}} + }; +} diff --git a/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp b/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp index a131842d3944..7768e84323d0 100644 --- a/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp +++ b/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 }; diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index c32ee7f50bd2..f8dbc840763c 100755 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -930,14 +930,15 @@ <h2 id="cxx20">C++2a implementation status</h2> </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> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
