[clang] 8e0c9e2 - [c++20] Delete defaulted comparison functions if they would invoke an
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 10 19:28:41 PST 2019
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 at -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>
More information about the cfe-commits
mailing list