From 5c9b3b757623e6bea5b4929b2046a565ee2d11bc Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Tue, 25 Sep 2018 22:12:44 +0000 Subject: [PATCH] P0969R0: allow structured binding of accessible members, not only public members. llvm-svn: 343036 --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 10 ++-- clang/include/clang/Sema/Sema.h | 4 ++ clang/lib/Sema/SemaAccess.cpp | 16 ++++++ clang/lib/Sema/SemaDeclAttr.cpp | 8 +++ clang/lib/Sema/SemaDeclCXX.cpp | 71 +++++++++--------------- clang/test/CXX/dcl.decl/dcl.decomp/p4.cpp | 51 +++++++++++++++-- clang/www/cxx_status.html | 2 +- 7 files changed, 108 insertions(+), 54 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a4c0f97..936cc79 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -452,10 +452,12 @@ def err_decomp_decl_multiple_bases_with_members : Error< "have non-static data members">; def err_decomp_decl_ambiguous_base : Error< "cannot decompose members of ambiguous base class %1 of %0:%2">; -def err_decomp_decl_non_public_base : Error< - "cannot decompose members of non-public base class %1 of %0">; -def err_decomp_decl_non_public_member : Error< - "cannot decompose non-public member %0 of %1">; +def err_decomp_decl_inaccessible_base : Error< + "cannot decompose members of inaccessible base class %1 of %0">, + AccessControl; +def err_decomp_decl_inaccessible_field : Error< + "cannot decompose %select{private|protected}0 member %1 of %3">, + AccessControl; def err_decomp_decl_anon_union_member : Error< "cannot decompose class type %0 because it has an anonymous " "%select{struct|union}1 member">; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index f91331b..dfcc61f 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6036,6 +6036,10 @@ public: AccessResult CheckMemberAccess(SourceLocation UseLoc, CXXRecordDecl *NamingClass, DeclAccessPair Found); + AccessResult + CheckStructuredBindingMemberAccess(SourceLocation UseLoc, + CXXRecordDecl *DecomposedClass, + DeclAccessPair Field); AccessResult CheckMemberOperatorAccess(SourceLocation Loc, Expr *ObjectExpr, Expr *ArgExpr, diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp index e06792c..cf33231 100644 --- a/clang/lib/Sema/SemaAccess.cpp +++ b/clang/lib/Sema/SemaAccess.cpp @@ -1728,6 +1728,22 @@ Sema::AccessResult Sema::CheckMemberAccess(SourceLocation UseLoc, return CheckAccess(*this, UseLoc, Entity); } +/// Checks implicit access to a member in a structured binding. +Sema::AccessResult +Sema::CheckStructuredBindingMemberAccess(SourceLocation UseLoc, + CXXRecordDecl *DecomposedClass, + DeclAccessPair Field) { + if (!getLangOpts().AccessControl || + Field.getAccess() == AS_public) + return AR_accessible; + + AccessTarget Entity(Context, AccessTarget::Member, DecomposedClass, Field, + Context.getRecordType(DecomposedClass)); + Entity.setDiag(diag::err_decomp_decl_inaccessible_field); + + return CheckAccess(*this, UseLoc, Entity); +} + /// Checks access to an overloaded member operator, including /// conversion operators. Sema::AccessResult Sema::CheckMemberOperatorAccess(SourceLocation OpLoc, diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index ff432a6..50c4643 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7514,6 +7514,7 @@ void Sema::PopParsingDeclaration(ParsingDeclState state, Decl *decl) { // for each of the different declarations. const DelayedDiagnosticPool *pool = &poppedPool; do { + bool AnyAccessFailures = false; for (DelayedDiagnosticPool::pool_iterator i = pool->pool_begin(), e = pool->pool_end(); i != e; ++i) { // This const_cast is a bit lame. Really, Triggered should be mutable. @@ -7530,7 +7531,14 @@ void Sema::PopParsingDeclaration(ParsingDeclState state, Decl *decl) { break; case DelayedDiagnostic::Access: + // Only produce one access control diagnostic for a structured binding + // declaration: we don't need to tell the user that all the fields are + // inaccessible one at a time. + if (AnyAccessFailures && isa(decl)) + continue; HandleDelayedAccessCheck(diag, decl); + if (diag.Triggered) + AnyAccessFailures = true; break; case DelayedDiagnostic::ForbiddenType: diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index eb3a2fc..b81e5be 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -1227,16 +1227,16 @@ static bool checkTupleLikeDecomposition(Sema &S, /// Find the base class to decompose in a built-in decomposition of a class type. /// This base class search is, unfortunately, not quite like any other that we /// perform anywhere else in C++. -static const CXXRecordDecl *findDecomposableBaseClass(Sema &S, - SourceLocation Loc, - const CXXRecordDecl *RD, - CXXCastPath &BasePath) { +static DeclAccessPair findDecomposableBaseClass(Sema &S, SourceLocation Loc, + const CXXRecordDecl *RD, + CXXCastPath &BasePath) { auto BaseHasFields = [](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) { return Specifier->getType()->getAsCXXRecordDecl()->hasDirectFields(); }; const CXXRecordDecl *ClassWithFields = nullptr; + AccessSpecifier AS = AS_public; if (RD->hasDirectFields()) // [dcl.decomp]p4: // Otherwise, all of E's non-static data members shall be public direct @@ -1249,7 +1249,7 @@ static const CXXRecordDecl *findDecomposableBaseClass(Sema &S, if (!RD->lookupInBases(BaseHasFields, Paths)) { // If no classes have fields, just decompose RD itself. (This will work // if and only if zero bindings were provided.) - return RD; + return DeclAccessPair::make(const_cast(RD), AS_public); } CXXBasePath *BestPath = nullptr; @@ -1262,7 +1262,7 @@ static const CXXRecordDecl *findDecomposableBaseClass(Sema &S, S.Diag(Loc, diag::err_decomp_decl_multiple_bases_with_members) << false << RD << BestPath->back().Base->getType() << P.back().Base->getType(); - return nullptr; + return DeclAccessPair(); } else if (P.Access < BestPath->Access) { BestPath = &P; } @@ -1273,23 +1273,13 @@ static const CXXRecordDecl *findDecomposableBaseClass(Sema &S, if (Paths.isAmbiguous(S.Context.getCanonicalType(BaseType))) { S.Diag(Loc, diag::err_decomp_decl_ambiguous_base) << RD << BaseType << S.getAmbiguousPathsDisplayString(Paths); - return nullptr; + return DeclAccessPair(); } - // ... public base class of E. - if (BestPath->Access != AS_public) { - S.Diag(Loc, diag::err_decomp_decl_non_public_base) - << RD << BaseType; - for (auto &BS : *BestPath) { - if (BS.Base->getAccessSpecifier() != AS_public) { - S.Diag(BS.Base->getBeginLoc(), diag::note_access_constrained_by_path) - << (BS.Base->getAccessSpecifier() == AS_protected) - << (BS.Base->getAccessSpecifierAsWritten() == AS_none); - break; - } - } - return nullptr; - } + // ... [accessible, implied by other rules] base class of E. + S.CheckBaseClassAccess(Loc, BaseType, S.Context.getRecordType(RD), + *BestPath, diag::err_decomp_decl_inaccessible_base); + AS = BestPath->Access; ClassWithFields = BaseType->getAsCXXRecordDecl(); S.BuildBasePathArray(Paths, BasePath); @@ -1302,17 +1292,19 @@ static const CXXRecordDecl *findDecomposableBaseClass(Sema &S, S.Diag(Loc, diag::err_decomp_decl_multiple_bases_with_members) << (ClassWithFields == RD) << RD << ClassWithFields << Paths.front().back().Base->getType(); - return nullptr; + return DeclAccessPair(); } - return ClassWithFields; + return DeclAccessPair::make(const_cast(ClassWithFields), AS); } static bool checkMemberDecomposition(Sema &S, ArrayRef Bindings, ValueDecl *Src, QualType DecompType, - const CXXRecordDecl *RD) { + const CXXRecordDecl *OrigRD) { CXXCastPath BasePath; - RD = findDecomposableBaseClass(S, Src->getLocation(), RD, BasePath); + DeclAccessPair BasePair = + findDecomposableBaseClass(S, Src->getLocation(), OrigRD, BasePath); + const CXXRecordDecl *RD = cast_or_null(BasePair.getDecl()); if (!RD) return true; QualType BaseType = S.Context.getQualifiedType(S.Context.getRecordType(RD), @@ -1329,7 +1321,8 @@ static bool checkMemberDecomposition(Sema &S, ArrayRef Bindings, return true; }; - // all of E's non-static data members shall be public [...] members, + // all of E's non-static data members shall be [...] well-formed + // when named as e.name in the context of the structured binding, // E shall not have an anonymous union member, ... unsigned I = 0; for (auto *FD : RD->fields()) { @@ -1347,26 +1340,16 @@ static bool checkMemberDecomposition(Sema &S, ArrayRef Bindings, if (I >= Bindings.size()) return DiagnoseBadNumberOfBindings(); auto *B = Bindings[I++]; - SourceLocation Loc = B->getLocation(); - if (FD->getAccess() != AS_public) { - S.Diag(Loc, diag::err_decomp_decl_non_public_member) << FD << DecompType; - - // Determine whether the access specifier was explicit. - bool Implicit = true; - for (const auto *D : RD->decls()) { - if (declaresSameEntity(D, FD)) - break; - if (isa(D)) { - Implicit = false; - break; - } - } - S.Diag(FD->getLocation(), diag::note_access_natural) - << (FD->getAccess() == AS_protected) << Implicit; - return true; - } + // The field must be accessible in the context of the structured binding. + // We already checked that the base class is accessible. + // FIXME: Add 'const' to AccessedEntity's classes so we can remove the + // const_cast here. + S.CheckStructuredBindingMemberAccess( + Loc, const_cast(OrigRD), + DeclAccessPair::make(FD, CXXRecordDecl::MergeAccess( + BasePair.getAccess(), FD->getAccess()))); // Initialize the binding to Src.FD. ExprResult E = S.BuildDeclRefExpr(Src, DecompType, VK_LValue, Loc); diff --git a/clang/test/CXX/dcl.decl/dcl.decomp/p4.cpp b/clang/test/CXX/dcl.decl/dcl.decomp/p4.cpp index c461eb6..f14c0d0 100644 --- a/clang/test/CXX/dcl.decl/dcl.decomp/p4.cpp +++ b/clang/test/CXX/dcl.decl/dcl.decomp/p4.cpp @@ -20,15 +20,15 @@ namespace NonPublicMembers { int a; // expected-note 2{{declared private here}} }; - struct NonPublic3 : private A {}; // expected-note {{constrained by private inheritance}} + struct NonPublic3 : private A {}; // expected-note {{declared private here}} struct NonPublic4 : NonPublic2 {}; void test() { - auto [a1] = NonPublic1(); // expected-error {{cannot decompose non-public member 'a' of 'NonPublicMembers::NonPublic1'}} - auto [a2] = NonPublic2(); // expected-error {{cannot decompose non-public member 'a' of 'NonPublicMembers::NonPublic2'}} - auto [a3] = NonPublic3(); // expected-error {{cannot decompose members of non-public base class 'A' of 'NonPublic3'}} - auto [a4] = NonPublic4(); // expected-error {{cannot decompose non-public member 'a' of 'NonPublicMembers::NonPublic4'}} + auto [a1] = NonPublic1(); // expected-error {{cannot decompose protected member 'a' of 'NonPublicMembers::NonPublic1'}} + auto [a2] = NonPublic2(); // expected-error {{cannot decompose private member 'a' of 'NonPublicMembers::NonPublic2'}} + auto [a3] = NonPublic3(); // expected-error {{cannot decompose members of inaccessible base class 'A' of 'NonPublicMembers::NonPublic3'}} + auto [a4] = NonPublic4(); // expected-error {{cannot decompose private member 'a' of 'NonPublicMembers::NonPublic2'}} } } @@ -198,3 +198,44 @@ namespace std_example { same same1; same same2; } + +namespace p0969r0 { + struct A { + int x; + int y; + }; + struct B : private A { // expected-note {{declared private here}} + void test_member() { + auto &[x, y] = *this; + } + friend void test_friend(B); + }; + void test_friend(B b) { + auto &[x, y] = b; + } + void test_external(B b) { + auto &[x, y] = b; // expected-error {{cannot decompose members of inaccessible base class 'p0969r0::A' of 'p0969r0::B'}} + } + + struct C { + int x; + protected: + int y; // expected-note {{declared protected here}} expected-note {{can only access this member on an object of type 'p0969r0::D'}} + void test_member() { + auto &[x, y] = *this; + } + friend void test_friend(struct D); + }; + struct D : C { + static void test_member(D d, C c) { + auto &[x1, y1] = d; + auto &[x2, y2] = c; // expected-error {{cannot decompose protected member 'y' of 'p0969r0::C'}} + } + }; + void test_friend(D d) { + auto &[x, y] = d; + } + void test_external(D d) { + auto &[x, y] = d; // expected-error {{cannot decompose protected member 'y' of 'p0969r0::C'}} + } +} diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index 6930779..0036f42 100755 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -760,7 +760,7 @@ version 3.7. P0969R0 (DR) - No + SVN Separate variable and condition for if and switch -- 2.7.4