Skip to content

Commit 69c622f

Browse files
xedinxedin
authored
Merge pull request #21783 from xedin/rdar-30933988
[AST] `Decl::is*AccessibleFrom` methods should respect access control…
2 parents 6c57610 + da8f384 commit 69c622f

16 files changed

+60
-33
lines changed

‎include/swift/AST/ASTContext.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,12 @@ class ASTContext final {
936936
return LangOpts.isSwiftVersionAtLeast(major, minor);
937937
}
938938

939+
/// Check whether it's important to respect access control restrictions
940+
/// in current context.
941+
bool isAccessControlDisabled() const {
942+
return !LangOpts.EnableAccessControl;
943+
}
944+
939945
private:
940946
friend Decl;
941947
Optional<RawComment> getRawComment(const Decl *D);

‎lib/AST/Decl.cpp

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2788,6 +2788,9 @@ ValueDecl::getFormalAccessScope(const DeclContext *useDC,
27882788
static bool checkAccessUsingAccessScopes(const DeclContext *useDC,
27892789
const ValueDecl *VD,
27902790
AccessLevel access) {
2791+
if (VD->getASTContext().isAccessControlDisabled())
2792+
return true;
2793+
27912794
AccessScope accessScope =
27922795
getAccessScopeForFormalAccess(VD, access, useDC,
27932796
/*treatUsableFromInlineAsPublic*/false);
@@ -2806,7 +2809,12 @@ static bool checkAccessUsingAccessScopes(const DeclContext *useDC,
28062809
///
28072810
/// See ValueDecl::isAccessibleFrom for a description of \p forConformance.
28082811
static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
2809-
AccessLevel access, bool forConformance) {
2812+
bool forConformance,
2813+
llvm::function_ref<AccessLevel()> getAccessLevel) {
2814+
if (VD->getASTContext().isAccessControlDisabled())
2815+
return true;
2816+
2817+
auto access = getAccessLevel();
28102818
auto *sourceDC = VD->getDeclContext();
28112819

28122820
if (!forConformance) {
@@ -2868,8 +2876,8 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
28682876

28692877
bool ValueDecl::isAccessibleFrom(const DeclContext *useDC,
28702878
bool forConformance) const {
2871-
auto access = getFormalAccess();
2872-
bool result = checkAccess(useDC, this, access, forConformance);
2879+
bool result = checkAccess(useDC, this, forConformance,
2880+
[&]() { return getFormalAccess(); });
28732881

28742882
// For everything outside of protocols and operators, we should get the same
28752883
// result using either implementation of checkAccess, because useDC must
@@ -2878,9 +2886,9 @@ bool ValueDecl::isAccessibleFrom(const DeclContext *useDC,
28782886
// because we're finding internal operators within private types. Fortunately
28792887
// we have a requirement that a member operator take the enclosing type as an
28802888
// argument, so it won't ever match.
2881-
assert(getDeclContext()->getSelfProtocolDecl() ||
2882-
isOperator() ||
2883-
result == checkAccessUsingAccessScopes(useDC, this, access));
2889+
assert(getDeclContext()->getSelfProtocolDecl() || isOperator() ||
2890+
result ==
2891+
checkAccessUsingAccessScopes(useDC, this, getFormalAccess()));
28842892

28852893
return result;
28862894
}
@@ -2898,8 +2906,8 @@ bool AbstractStorageDecl::isSetterAccessibleFrom(const DeclContext *DC,
28982906
if (isa<ParamDecl>(this))
28992907
return true;
29002908

2901-
auto access = getSetterFormalAccess();
2902-
return checkAccess(DC, this, access, forConformance);
2909+
return checkAccess(DC, this, forConformance,
2910+
[&]() { return getSetterFormalAccess(); });
29032911
}
29042912

29052913
void ValueDecl::copyFormalAccessFrom(const ValueDecl *source,

‎lib/AST/ModuleNameLookup.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,8 @@ void namelookup::lookupInModule(ModuleDecl *startModule,
258258
ArrayRef<ModuleDecl::ImportedModule> extraImports) {
259259
assert(moduleScopeContext && moduleScopeContext->isModuleScopeContext());
260260
ModuleLookupCache cache;
261-
bool respectAccessControl = startModule->getASTContext().LangOpts
262-
.EnableAccessControl;
261+
bool respectAccessControl =
262+
!startModule->getASTContext().isAccessControlDisabled();
263263
::lookupInModule<CanTypeSet>(startModule, topAccessPath, decls,
264264
resolutionKind, /*canReturnEarly=*/true,
265265
typeResolver, cache, moduleScopeContext,
@@ -282,7 +282,7 @@ void namelookup::lookupVisibleDeclsInModule(
282282
ArrayRef<ModuleDecl::ImportedModule> extraImports) {
283283
assert(moduleScopeContext && moduleScopeContext->isModuleScopeContext());
284284
ModuleLookupCache cache;
285-
bool respectAccessControl = M->getASTContext().LangOpts.EnableAccessControl;
285+
bool respectAccessControl = !M->getASTContext().isAccessControlDisabled();
286286
::lookupInModule<NamedCanTypeSet>(M, accessPath, decls,
287287
resolutionKind, /*canReturnEarly=*/false,
288288
typeResolver, cache, moduleScopeContext,

‎lib/AST/NameLookup.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,11 @@ void DebuggerClient::anchor() {}
6565

6666
void AccessFilteringDeclConsumer::foundDecl(ValueDecl *D,
6767
DeclVisibilityKind reason) {
68-
if (D->getASTContext().LangOpts.EnableAccessControl) {
69-
if (D->isInvalid())
70-
return;
71-
if (!D->isAccessibleFrom(DC))
72-
return;
73-
}
68+
if (D->isInvalid())
69+
return;
70+
if (!D->isAccessibleFrom(DC))
71+
return;
72+
7473
ChainedConsumer.foundDecl(D, reason);
7574
}
7675

@@ -1843,7 +1842,7 @@ static void configureLookup(const DeclContext *dc,
18431842
ReferencedNameTracker *&tracker,
18441843
bool &isLookupCascading) {
18451844
auto &ctx = dc->getASTContext();
1846-
if (!ctx.LangOpts.EnableAccessControl)
1845+
if (ctx.isAccessControlDisabled())
18471846
options |= NL_IgnoreAccessControl;
18481847

18491848
// Find the dependency tracker we'll need for this lookup.

‎lib/PrintAsObjC/PrintAsObjC.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,7 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
11221122

11231123
ASTContext &ctx = M.getASTContext();
11241124
bool isSettable = VD->isSettable(nullptr);
1125-
if (isSettable && ctx.LangOpts.EnableAccessControl)
1125+
if (isSettable && !ctx.isAccessControlDisabled())
11261126
isSettable = (VD->getSetterFormalAccess() >= minRequiredAccess);
11271127
if (!isSettable)
11281128
os << ", readonly";

‎lib/Sema/CSApply.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6791,9 +6791,8 @@ static Type adjustSelfTypeForMember(Type baseTy, ValueDecl *member,
67916791
// If the base of the access is mutable, then we may be invoking a getter or
67926792
// setter that requires the base to be mutable.
67936793
auto *SD = cast<AbstractStorageDecl>(member);
6794-
bool isSettableFromHere = SD->isSettable(UseDC)
6795-
&& (!UseDC->getASTContext().LangOpts.EnableAccessControl
6796-
|| SD->isSetterAccessibleFrom(UseDC));
6794+
bool isSettableFromHere =
6795+
SD->isSettable(UseDC) && SD->isSetterAccessibleFrom(UseDC);
67976796

67986797
// If neither the property's getter nor its setter are mutating, the base
67996798
// can be an rvalue.

‎lib/Sema/ConstraintSystem.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -715,8 +715,7 @@ static bool doesStorageProduceLValue(TypeChecker &TC,
715715
if (!storage->isSettable(useDC, base))
716716
return false;
717717

718-
if (TC.Context.LangOpts.EnableAccessControl &&
719-
!storage->isSetterAccessibleFrom(useDC))
718+
if (!storage->isSetterAccessibleFrom(useDC))
720719
return false;
721720

722721
// If there is no base, or if the base isn't being used, it is settable.

‎lib/Sema/LookupVisibleDecls.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,7 @@ static bool isDeclVisibleInLookupMode(ValueDecl *Member, LookupState LS,
129129

130130
// Check access when relevant.
131131
if (!Member->getDeclContext()->isLocalContext() &&
132-
!isa<GenericTypeParamDecl>(Member) && !isa<ParamDecl>(Member) &&
133-
FromContext->getASTContext().LangOpts.EnableAccessControl) {
132+
!isa<GenericTypeParamDecl>(Member) && !isa<ParamDecl>(Member)) {
134133
if (!Member->isAccessibleFrom(FromContext))
135134
return false;
136135
}

‎lib/Sema/TypeCheckAccess.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
160160
Type type, TypeRepr *typeRepr, AccessScope contextAccessScope,
161161
const DeclContext *useDC, bool mayBeInferred,
162162
llvm::function_ref<CheckTypeAccessCallback> diagnose) {
163-
if (!TC.getLangOpts().EnableAccessControl)
163+
if (TC.Context.isAccessControlDisabled())
164164
return;
165165
if (!type)
166166
return;

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2775,7 +2775,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
27752775
}
27762776
}
27772777

2778-
if (TC.getLangOpts().EnableAccessControl) {
2778+
if (!TC.Context.isAccessControlDisabled()) {
27792779
// Require the superclass to be open if this is outside its
27802780
// defining module. But don't emit another diagnostic if we
27812781
// already complained about the class being inherently

‎lib/Sema/TypeCheckDeclOverride.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,7 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
761761

762762
static void checkOverrideAccessControl(ValueDecl *baseDecl, ValueDecl *decl,
763763
ASTContext &ctx) {
764-
if (!ctx.LangOpts.EnableAccessControl)
764+
if (ctx.isAccessControlDisabled())
765765
return;
766766

767767
auto &diags = ctx.Diags;
@@ -1479,7 +1479,7 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
14791479
// read-only. Observing properties look at change, read-only properties
14801480
// have nothing to observe!
14811481
bool baseIsSettable = baseASD->isSettable(baseASD->getDeclContext());
1482-
if (baseIsSettable && ctx.LangOpts.EnableAccessControl) {
1482+
if (baseIsSettable) {
14831483
baseIsSettable =
14841484
baseASD->isSetterAccessibleFrom(overrideASD->getDeclContext());
14851485
}

‎lib/Sema/TypeCheckNameLookup.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ namespace {
9090
bool isMemberLookup)
9191
: Result(result), DC(dc), Options(options),
9292
IsMemberLookup(isMemberLookup) {
93-
if (!dc->getASTContext().LangOpts.EnableAccessControl)
93+
if (dc->getASTContext().isAccessControlDisabled())
9494
Options |= NameLookupFlags::IgnoreAccessControl;
9595
}
9696

‎lib/Sema/TypeCheckProtocol.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,7 @@ bool WitnessChecker::checkWitnessAccess(ValueDecl *requirement,
12741274
ValueDecl *witness,
12751275
bool *isSetter) {
12761276
*isSetter = false;
1277-
if (!TC.getLangOpts().EnableAccessControl)
1277+
if (TC.Context.isAccessControlDisabled())
12781278
return false;
12791279

12801280
AccessScope actualScopeToCheck = getRequiredAccessScope();

‎lib/Sema/TypeCheckProtocol.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ class WitnessChecker {
497497
AccessScope getRequiredAccessScope();
498498

499499
bool isUsableFromInlineRequired() {
500-
if (!TC.getLangOpts().EnableAccessControl)
500+
if (TC.Context.isAccessControlDisabled())
501501
return false;
502502

503503
assert(RequiredAccessScopeAndUsableFromInline.hasValue() &&

‎validation-test/Sema/Inputs/rdar30933988_enum.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
enum E<T> {
2+
case foo
3+
case bar(T)
4+
}

‎validation-test/Sema/rdar30933988.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -o %t/rdar30933988_enum.swiftmodule %S/Inputs/rdar30933988_enum.swift
3+
// RUN: %target-typecheck-verify-swift -I %t -disable-access-control
4+
5+
import rdar30933988_enum
6+
7+
let _: E = .foo
8+
// expected-error@-1 {{generic parameter 'T' could not be inferred}}
9+
let _: E<Int> = .foo // Ok
10+
let _: E = .bar(42) // Ok
11+
let _: E<String> = .bar(42)
12+
// expected-error@-1 {{member 'bar' in 'E<String>' produces result of type 'E<T>', but context expects 'E<String>'}}
13+
let _: E<Int> = .bar(42) // Ok

0 commit comments

Comments
 (0)