Skip to content

Commit be20333

Browse files
committed
Add access level and scope checks to package types
Resolves rdar://104617227
1 parent fa1f536 commit be20333

File tree

9 files changed

+893
-35
lines changed

9 files changed

+893
-35
lines changed

include/swift/AST/AccessScope.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,10 @@ class AccessScope {
8686
}
8787

8888
/// Returns true if this scope is more restrictive than the argument scope.
89-
/// It's often used to compute the min access scope. The order of restrictiveness
90-
/// is: private (most restrictive), fileprivate, internal, package, public (least restrictive).
89+
/// This function does _not_ check if the access level is more restrictive than the argument access level.
90+
/// It checks if the scope of this use site is more restrictive than the scope of the argument (decl site).
91+
/// For example, the scope is FileUnit for a fileprivate decl, Module for an internal decl, and null for a public
92+
/// or package decl.
9193
/// \see DeclContext::isChildContextOf
9294
bool isChildOf(AccessScope AS) const {
9395
if (isInternalOrLess()) {

lib/AST/ASTVerifier.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -939,12 +939,19 @@ class Verifier : public ASTWalker {
939939

940940
if (D->hasAccess()) {
941941
PrettyStackTraceDecl debugStack("verifying access", D);
942-
if (!D->getASTContext().isAccessControlDisabled() &&
943-
D->getFormalAccessScope().isPublic() &&
944-
D->getFormalAccess() <= AccessLevel::Package) {
945-
Out << "non-public decl has no formal access scope\n";
946-
D->dump(Out);
947-
abort();
942+
if (!D->getASTContext().isAccessControlDisabled()) {
943+
if (D->getFormalAccessScope().isPackage() &&
944+
D->getFormalAccess() < AccessLevel::Package) {
945+
Out << "non-package decl has no formal access scope\n";
946+
D->dump(Out);
947+
abort();
948+
}
949+
if (D->getFormalAccessScope().isPublic() &&
950+
D->getFormalAccess() < AccessLevel::Public) {
951+
Out << "non-public decl has no formal access scope\n";
952+
D->dump(Out);
953+
abort();
954+
}
948955
}
949956
if (D->getEffectiveAccess() == AccessLevel::Private) {
950957
Out << "effective access should use 'fileprivate' for 'private'\n";

lib/AST/Decl.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3776,6 +3776,7 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
37763776
case AccessLevel::Internal:
37773777
return AccessScope(resultDC->getParentModule());
37783778
case AccessLevel::Package:
3779+
return AccessScope::getPackage();
37793780
case AccessLevel::Public:
37803781
case AccessLevel::Open:
37813782
return AccessScope::getPublic();
@@ -3811,8 +3812,15 @@ static bool checkAccessUsingAccessScopes(const DeclContext *useDC,
38113812
if (accessScope.getDeclContext() == useDC) return true;
38123813
if (!AccessScope(useDC).isChildOf(accessScope)) return false;
38133814

3815+
// useDC is null only when caller wants to skip non-public type checks.
3816+
if (!useDC) return true;
3817+
3818+
// Check package access; accessing package decl should not be allowed if package names are different
3819+
if (accessScope.isPackage())
3820+
return VD->getDeclContext()->getParentModule()->getPackageName() == useDC->getParentModule()->getPackageName();
3821+
38143822
// Check SPI access
3815-
if (!useDC || !VD->isSPI()) return true;
3823+
if (!VD->isSPI()) return true;
38163824
auto useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
38173825
return !useSF || useSF->isImportedAsSPI(VD) ||
38183826
VD->getDeclContext()->getParentModule() == useDC->getParentModule();

lib/AST/DeclContext.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ void DeclContext::dumpContext() const {
583583
void AccessScope::dump() const {
584584
llvm::errs() << getAccessLevelSpelling(accessLevelForDiagnostics()) << ": ";
585585

586-
if (isPublic()) {
586+
if (isPublic() || isPackage()) {
587587
llvm::errs() << "(null)\n";
588588
return;
589589
}
@@ -1189,7 +1189,7 @@ AccessScope::AccessScope(const DeclContext *DC, AccessLimitKind limitKind)
11891189
isPrivate = true;
11901190
}
11911191
if (!DC || isa<ModuleDecl>(DC))
1192-
assert(!isPrivate && "public or internal scope can't be private");
1192+
assert(!isPrivate && "public, package, or internal scope can't be private");
11931193
}
11941194

11951195
bool AccessScope::isFileScope() const {
@@ -1205,6 +1205,8 @@ bool AccessScope::isInternal() const {
12051205
AccessLevel AccessScope::accessLevelForDiagnostics() const {
12061206
if (isPublic())
12071207
return AccessLevel::Public;
1208+
if (isPackage())
1209+
return AccessLevel::Package;
12081210
if (isa<ModuleDecl>(getDeclContext()))
12091211
return AccessLevel::Internal;
12101212
if (getDeclContext()->isModuleScopeContext()) {

lib/DriverTool/swift_symbolgraph_extract_main.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ int swift_symbolgraph_extract_main(ArrayRef<const char *> Args,
181181
llvm::StringSwitch<AccessLevel>(A->getValue())
182182
.Case("open", AccessLevel::Open)
183183
.Case("public", AccessLevel::Public)
184+
.Case("package", AccessLevel::Package)
184185
.Case("internal", AccessLevel::Internal)
185186
.Case("fileprivate", AccessLevel::FilePrivate)
186187
.Case("private", AccessLevel::Private)

lib/Frontend/CompilerInvocation.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,7 @@ static void ParseSymbolGraphArgs(symbolgraphgen::SymbolGraphOptions &Opts,
13371337
llvm::StringSwitch<AccessLevel>(A->getValue())
13381338
.Case("open", AccessLevel::Open)
13391339
.Case("public", AccessLevel::Public)
1340+
.Case("package", AccessLevel::Package)
13401341
.Case("internal", AccessLevel::Internal)
13411342
.Case("fileprivate", AccessLevel::FilePrivate)
13421343
.Case("private", AccessLevel::Private)

lib/Parse/ParseDecl.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2065,6 +2065,7 @@ bool Parser::parseDocumentationAttributeArgument(Optional<StringRef> &Metadata,
20652065
llvm::StringSwitch<Optional<AccessLevel>>(ArgumentValue)
20662066
.Case("open", AccessLevel::Open)
20672067
.Case("public", AccessLevel::Public)
2068+
.Case("package", AccessLevel::Package)
20682069
.Case("internal", AccessLevel::Internal)
20692070
.Case("private", AccessLevel::Private)
20702071
.Case("fileprivate", AccessLevel::FilePrivate)
@@ -2742,6 +2743,7 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
27422743
.Case("private", AccessLevel::Private)
27432744
.Case("fileprivate", AccessLevel::FilePrivate)
27442745
.Case("internal", AccessLevel::Internal)
2746+
.Case("package", AccessLevel::Package)
27452747
.Case("public", AccessLevel::Public)
27462748
.Case("open", AccessLevel::Open);
27472749

lib/Sema/TypeCheckAccess.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
182182
return;
183183
// Don't spend time checking local declarations; this is always valid by the
184184
// time we get to this point.
185-
if (!contextAccessScope.isPublic() &&
185+
if (contextAccessScope.isInternalOrLess() &&
186186
contextAccessScope.getDeclContext()->isLocalContext())
187187
return;
188188

@@ -2106,6 +2106,8 @@ static void checkExtensionGenericParamAccess(const ExtensionDecl *ED) {
21062106
desiredAccessScope = AccessScope(ED->getModuleContext());
21072107
break;
21082108
case AccessLevel::Package:
2109+
desiredAccessScope = AccessScope::getPackage();
2110+
break;
21092111
case AccessLevel::Public:
21102112
case AccessLevel::Open:
21112113
break;

0 commit comments

Comments
 (0)