Skip to content

Commit 26c6a18

Browse files
xymusxymus
committed
[Sema] Improve type-checking of the use and exposability of SPIs
1 parent c61cc6f commit 26c6a18

File tree

5 files changed

+56
-84
lines changed

5 files changed

+56
-84
lines changed

‎include/swift/AST/Attr.def

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,8 +541,9 @@ DECL_ATTR(derivative, Derivative,
541541
97)
542542

543543
DECL_ATTR(_spi, SPIAccessControl,
544-
OnFunc | OnExtension | OnGenericType | OnVar | OnSubscript | OnConstructor |
545-
OnImport | AllowMultipleAttributes | UserInaccessible |
544+
OnAbstractFunction | OnExtension | OnGenericType | OnVar | OnSubscript |
545+
OnImport | OnAccessor | OnEnumElement |
546+
AllowMultipleAttributes | UserInaccessible |
546547
ABIStableToAdd | ABIStableToRemove | APIBreakingToAdd | APIStableToRemove,
547548
98)
548549

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -143,17 +143,17 @@ FIXIT(insert_type_qualification,"%0.",(Type))
143143

144144
ERROR(candidate_inaccessible,none,
145145
"%0 is inaccessible due to "
146-
"'%select{private|fileprivate|internal|SPI|SPI}1' protection level",
146+
"'%select{private|fileprivate|internal|@_spi|@_spi}1' protection level",
147147
(DeclBaseName, AccessLevel))
148148

149149
NOTE(note_candidate_inaccessible,none,
150150
"%0 is inaccessible due to "
151-
"'%select{private|fileprivate|internal|%error|%error}1' protection level",
151+
"'%select{private|fileprivate|internal|@_spi|@_spi}1' protection level",
152152
(DeclName, AccessLevel))
153153

154154
ERROR(init_candidate_inaccessible,none,
155155
"%0 initializer is inaccessible due to "
156-
"'%select{private|fileprivate|internal|SPI|SPI}1' protection level",
156+
"'%select{private|fileprivate|internal|@_spi|@_spi}1' protection level",
157157
(Type, AccessLevel))
158158

159159
ERROR(cannot_pass_rvalue_mutating_subelement,none,
@@ -1649,15 +1649,15 @@ ERROR(function_type_access,none,
16491649
"|cannot be declared "
16501650
"%select{in this context|fileprivate|internal|public|open}1}0 "
16511651
"because its %select{parameter|result}5 uses "
1652-
"%select{a private|a fileprivate|an internal|%error|%error}3 type",
1652+
"%select{a private|a fileprivate|an internal|an '@_spi'|an '@_spi'}3 type",
16531653
(bool, AccessLevel, bool, AccessLevel, unsigned, bool))
16541654
ERROR(function_type_spi,none,
16551655
"%select{function|method|initializer}0 "
16561656
"cannot be declared '@_spi' "
16571657
"because its %select{parameter|result}1 uses "
1658-
"%select{a private|a fileprivate|an internal|%error|%error}2 type "
1659-
"without a compatible '@_spi'",
1660-
(unsigned, bool, AccessLevel))
1658+
"%select{a private|a fileprivate|an internal|a public|an open}2 type"
1659+
"%select{| that is not '@_spi'}3",
1660+
(unsigned, bool, AccessLevel, bool))
16611661
WARNING(function_type_access_warn,none,
16621662
"%select{function|method|initializer}4 "
16631663
"%select{should be declared %select{private|fileprivate|internal|%error|%error}1"
@@ -2295,14 +2295,14 @@ ERROR(generic_param_access,none,
22952295
"|cannot be declared "
22962296
"%select{in this context|fileprivate|internal|public|open}2}1 "
22972297
"because its generic %select{parameter|requirement}5 uses "
2298-
"%select{a private|a fileprivate|an internal|%error|%error}3 type",
2298+
"%select{a private|a fileprivate|an internal|an '@_spi'|an '@_spi'}3 type",
22992299
(DescriptiveDeclKind, bool, AccessLevel, AccessLevel, bool, bool))
23002300
WARNING(generic_param_access_warn,none,
23012301
"%0 %select{should be declared "
23022302
"%select{private|fileprivate|internal|%error|%error}3"
23032303
"|should not be declared %select{in this context|fileprivate|internal|public|open}2}1 "
23042304
"because its generic %select{parameter|requirement}5 uses "
2305-
"%select{a private|a fileprivate|an internal|%error|%error}3 type",
2305+
"%select{a private|a fileprivate|an internal|an '@_spi'|an '@_spi'}3 type",
23062306
(DescriptiveDeclKind, bool, AccessLevel, AccessLevel, bool, bool))
23072307
ERROR(generic_param_usable_from_inline,none,
23082308
"type referenced from a "
@@ -3873,17 +3873,17 @@ ERROR(class_super_access,none,
38733873
"%select{private|fileprivate|internal|%error|%error}1|private or fileprivate}3"
38743874
"|cannot be declared %select{in this context|fileprivate|internal|public|open}1}0 "
38753875
"because its superclass "
3876-
"%select{is %select{private|fileprivate|internal|%error|%error}2"
3877-
"|uses %select{a private|a fileprivate|an internal|%error|%error}2 "
3876+
"%select{is %select{private|fileprivate|internal|'@_spi'|'@_spi'}2"
3877+
"|uses %select{a private|a fileprivate|an internal|an '@_spi'|an '@_spi'}2 "
38783878
"type as a generic parameter}4",
38793879
(bool, AccessLevel, AccessLevel, bool, bool))
38803880
WARNING(class_super_access_warn,none,
38813881
"class %select{should be declared "
38823882
"%select{private|fileprivate|internal|%error|%error}1"
38833883
"|should not be declared %select{in this context|fileprivate|internal|public|open}1}0 "
38843884
"because its superclass "
3885-
"%select{is %select{private|fileprivate|internal|%error|%error}2"
3886-
"|uses %select{a private|a fileprivate|an internal|%error|%error}2 "
3885+
"%select{is %select{private|fileprivate|internal|'@_spi'|'@_spi'}2"
3886+
"|uses %select{a private|a fileprivate|an internal|an '@_spi'|an '@_spi'}2 "
38873887
"type as a generic parameter}4",
38883888
(bool, AccessLevel, AccessLevel, bool, bool))
38893889
ERROR(class_super_not_usable_from_inline,none,
@@ -4594,20 +4594,15 @@ ERROR(local_type_in_inlinable_function,
45944594
(DeclName, unsigned))
45954595

45964596
ERROR(resilience_decl_unavailable,
4597-
none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
4597+
none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|'@_spi'|'@_spi'}2 and "
45984598
"cannot be referenced from " FRAGILE_FUNC_KIND "3",
45994599
(DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
46004600

46014601
WARNING(resilience_decl_unavailable_warn,
4602-
none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
4602+
none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|'@_spi'|'@_spi'}2 and "
46034603
"should not be referenced from " FRAGILE_FUNC_KIND "3",
46044604
(DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
46054605

4606-
ERROR(resilience_decl_unavailable_spi,
4607-
none, DECL_OR_ACCESSOR "4 %1 is imported as SPI; "
4608-
"it cannot be referenced from " FRAGILE_FUNC_KIND "3",
4609-
(DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
4610-
46114606
ERROR(inlinable_decl_ref_from_hidden_module,
46124607
none, "%0 %1 cannot be used in " FRAGILE_FUNC_KIND "2 "
46134608
"because %select{%3 was imported implementation-only|"

‎lib/AST/Decl.cpp

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3189,8 +3189,7 @@ static AccessLevel getMaximallyOpenAccessFor(const ValueDecl *decl) {
31893189
static AccessLevel getAdjustedFormalAccess(const ValueDecl *VD,
31903190
AccessLevel access,
31913191
const DeclContext *useDC,
3192-
bool treatUsableFromInlineAsPublic,
3193-
bool treatSPIAsPublic) {
3192+
bool treatUsableFromInlineAsPublic) {
31943193
// If access control is disabled in the current context, adjust
31953194
// access level of the current declaration to be as open as possible.
31963195
if (useDC && VD->getASTContext().isAccessControlDisabled())
@@ -3209,9 +3208,6 @@ static AccessLevel getAdjustedFormalAccess(const ValueDecl *VD,
32093208
if (!useSF) return access;
32103209
if (useSF->hasTestableOrPrivateImport(access, VD))
32113210
return getMaximallyOpenAccessFor(VD);
3212-
} else if (!treatSPIAsPublic && VD->isSPI()) {
3213-
// Restrict access to SPI decls.
3214-
return AccessLevel::Internal;
32153211
}
32163212

32173213
return access;
@@ -3221,18 +3217,15 @@ static AccessLevel getAdjustedFormalAccess(const ValueDecl *VD,
32213217
/// adjust.
32223218
static AccessLevel
32233219
getAdjustedFormalAccess(const ValueDecl *VD, const DeclContext *useDC,
3224-
bool treatUsableFromInlineAsPublic,
3225-
bool treatSPIAsPublic) {
3220+
bool treatUsableFromInlineAsPublic) {
32263221
return getAdjustedFormalAccess(VD, VD->getFormalAccess(), useDC,
3227-
treatUsableFromInlineAsPublic,
3228-
treatSPIAsPublic);
3222+
treatUsableFromInlineAsPublic);
32293223
}
32303224

32313225
AccessLevel ValueDecl::getEffectiveAccess() const {
32323226
auto effectiveAccess =
32333227
getAdjustedFormalAccess(this, /*useDC=*/nullptr,
3234-
/*treatUsableFromInlineAsPublic=*/true,
3235-
/*treatSPIAsPublic=*/true);
3228+
/*treatUsableFromInlineAsPublic=*/true);
32363229

32373230
// Handle @testable/@_private(sourceFile:)
32383231
switch (effectiveAccess) {
@@ -3299,8 +3292,7 @@ bool ValueDecl::hasOpenAccess(const DeclContext *useDC) const {
32993292

33003293
AccessLevel access =
33013294
getAdjustedFormalAccess(this, useDC,
3302-
/*treatUsableFromInlineAsPublic*/false,
3303-
/*treatSPIAsPublic=*/false);
3295+
/*treatUsableFromInlineAsPublic*/false);
33043296
return access == AccessLevel::Open;
33053297
}
33063298

@@ -3316,8 +3308,7 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
33163308
const DeclContext *useDC,
33173309
bool treatUsableFromInlineAsPublic) {
33183310
AccessLevel access = getAdjustedFormalAccess(VD, formalAccess, useDC,
3319-
treatUsableFromInlineAsPublic,
3320-
/*treatSPIAsPublic=*/false);
3311+
treatUsableFromInlineAsPublic);
33213312
const DeclContext *resultDC = VD->getDeclContext();
33223313

33233314
while (!resultDC->isModuleScopeContext()) {
@@ -3332,8 +3323,7 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
33323323
if (auto enclosingNominal = dyn_cast<GenericTypeDecl>(resultDC)) {
33333324
auto enclosingAccess =
33343325
getAdjustedFormalAccess(enclosingNominal, useDC,
3335-
treatUsableFromInlineAsPublic,
3336-
/*treatSPIAsPublic=*/false);
3326+
treatUsableFromInlineAsPublic);
33373327
access = std::min(access, enclosingAccess);
33383328

33393329
} else if (auto enclosingExt = dyn_cast<ExtensionDecl>(resultDC)) {
@@ -3343,8 +3333,7 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
33433333
if (nominal->getParentModule() == enclosingExt->getParentModule()) {
33443334
auto nominalAccess =
33453335
getAdjustedFormalAccess(nominal, useDC,
3346-
treatUsableFromInlineAsPublic,
3347-
/*treatSPIAsPublic=*/false);
3336+
treatUsableFromInlineAsPublic);
33483337
access = std::min(access, nominalAccess);
33493338
}
33503339
}
@@ -3364,14 +3353,8 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
33643353
case AccessLevel::Internal:
33653354
return AccessScope(resultDC->getParentModule());
33663355
case AccessLevel::Public:
3367-
case AccessLevel::Open: {
3368-
if (useDC) {
3369-
auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
3370-
if (useSF && VD->isSPI() && !useSF->isImportedAsSPI(VD))
3371-
return AccessScope(VD->getModuleContext(), /*private*/false);
3372-
}
3373-
return AccessScope::getPublic();
3374-
}
3356+
case AccessLevel::Open:
3357+
return AccessScope::getPublic(VD->isSPI());
33753358
}
33763359

33773360
llvm_unreachable("unknown access level");
@@ -3400,8 +3383,14 @@ static bool checkAccessUsingAccessScopes(const DeclContext *useDC,
34003383
AccessScope accessScope =
34013384
getAccessScopeForFormalAccess(VD, access, useDC,
34023385
/*treatUsableFromInlineAsPublic*/false);
3403-
return accessScope.getDeclContext() == useDC ||
3404-
AccessScope(useDC).isChildOf(accessScope);
3386+
if (accessScope.getDeclContext() == useDC) return true;
3387+
if (!AccessScope(useDC).isChildOf(accessScope)) return false;
3388+
3389+
// Check SPI access
3390+
if (!useDC || !VD->isSPI()) return true;
3391+
auto useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
3392+
return !useSF || useSF->isImportedAsSPI(VD) ||
3393+
VD->getDeclContext()->getParentModule() == useDC->getParentModule();
34053394
}
34063395

34073396
/// Checks if \p VD may be used from \p useDC, taking \@testable and \@_spi
@@ -3477,10 +3466,9 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
34773466
}
34783467
case AccessLevel::Public:
34793468
case AccessLevel::Open: {
3480-
if (useDC) {
3469+
if (useDC && VD->isSPI()) {
34813470
auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
3482-
if (useSF && VD->isSPI() && !useSF->isImportedAsSPI(VD))
3483-
return false;
3471+
return !useSF || useSF->isImportedAsSPI(VD);
34843472
}
34853473
return true;
34863474
}

‎lib/Sema/ResilienceDiagnostics.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,10 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
127127
if (D->getDeclContext()->isLocalContext())
128128
return false;
129129

130-
// Public declarations are OK.
130+
// Public non-SPI declarations are OK.
131131
if (D->getFormalAccessScope(/*useDC=*/nullptr,
132-
TreatUsableFromInlineAsPublic).isPublic())
132+
TreatUsableFromInlineAsPublic).isPublic() &&
133+
!D->isSPI())
133134
return false;
134135

135136
auto &Context = DC->getASTContext();
@@ -184,11 +185,6 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
184185
if (downgradeToWarning == DowngradeToWarning::Yes)
185186
diagID = diag::resilience_decl_unavailable_warn;
186187

187-
// If SPI, don't mention the access level.
188-
const SourceFile *SF = DC->getParentSourceFile();
189-
if (SF && SF->isImportedAsSPI(D))
190-
diagID = diag::resilience_decl_unavailable_spi;
191-
192188
Context.Diags.diagnose(
193189
loc, diagID,
194190
D->getDescriptiveKind(), diagName,

‎lib/Sema/TypeCheckAccess.cpp

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,6 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
204204
const DeclContext *useDC, bool mayBeInferred, FromSPI fromSPI,
205205
llvm::function_ref<CheckTypeAccessCallback> diagnose) {
206206

207-
if (fromSPI == FromSPI::Yes)
208-
contextAccessScope = AccessScope::getPublic();
209-
210207
auto &Context = useDC->getASTContext();
211208
if (Context.isAccessControlDisabled())
212209
return;
@@ -250,7 +247,13 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
250247
contextAccessScope.isChildOf(*typeReprAccessScope)) {
251248
// Only if both the Type and the TypeRepr follow the access rules can
252249
// we exit; otherwise we have to emit a diagnostic.
253-
return;
250+
251+
if (typeReprAccessScope->isPublic() != contextAccessScope.isPublic() ||
252+
!typeReprAccessScope->isSPI() ||
253+
contextAccessScope.isSPI()) {
254+
// And we exit only if there is no SPI violation.
255+
return;
256+
}
254257
}
255258
problematicAccessScope = *typeReprAccessScope;
256259

@@ -282,15 +285,6 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
282285
const TypeRepr *complainRepr = TypeAccessScopeDiagnoser::findTypeWithScope(
283286
typeRepr, problematicAccessScope, useDC, checkUsableFromInline);
284287

285-
if (fromSPI == FromSPI::Yes) {
286-
// If in SPI mode, don't report SPI use from within the same module.
287-
if (auto CITR = dyn_cast<ComponentIdentTypeRepr>(complainRepr)) {
288-
const ValueDecl *VD = CITR->getBoundDecl();
289-
if (useDC->getParentModule() == VD->getModuleContext() && VD->isSPI())
290-
return;
291-
}
292-
}
293-
294288
diagnose(problematicAccessScope, complainRepr, downgradeToWarning);
295289
}
296290

@@ -368,7 +362,8 @@ void AccessControlCheckerBase::checkGenericParamAccess(
368362
(thisDowngrade == DowngradeToWarning::No &&
369363
downgradeToWarning == DowngradeToWarning::Yes) ||
370364
(!complainRepr &&
371-
typeAccessScope.hasEqualDeclContextWith(minAccessScope))) {
365+
typeAccessScope.hasEqualDeclContextWith(minAccessScope)) ||
366+
typeAccessScope.isSPI()) {
372367
minAccessScope = typeAccessScope;
373368
complainRepr = thisComplainRepr;
374369
accessControlErrorKind = callbackACEK;
@@ -397,7 +392,7 @@ void AccessControlCheckerBase::checkGenericParamAccess(
397392
const_cast<GenericContext *>(ownerCtx)),
398393
accessScope, DC, callback);
399394

400-
if (minAccessScope.isPublic())
395+
if (minAccessScope.isPublic() && !minAccessScope.isSPI())
401396
return;
402397

403398
// FIXME: Promote these to an error in the next -swift-version break.
@@ -980,7 +975,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
980975
});
981976
}
982977

983-
if (!minAccessScope.isPublic()) {
978+
if (!minAccessScope.isPublic() || minAccessScope.isSPI()) {
984979
auto minAccess = minAccessScope.accessLevelForDiagnostics();
985980
auto functionKind = isa<ConstructorDecl>(fn)
986981
? FK_Initializer
@@ -995,14 +990,11 @@ class AccessControlChecker : public AccessControlCheckerBase,
995990
// Report as an SPI problem if the type is at least as public as the decl.
996991
AccessScope contextAccessScope =
997992
fn->getFormalAccessScope(fn->getDeclContext(), checkUsableFromInline);
998-
auto restrictedBySPI = !minAccessScope.isChildOf(contextAccessScope);
999-
assert((!restrictedBySPI ||
1000-
fn->getAttrs().hasAttribute<SPIAccessControlAttr>()) &&
1001-
"There should be SPI attributes on this decl");
1002993

1003-
if (restrictedBySPI) {
994+
if (contextAccessScope.isSPI()) {
1004995
auto diag = fn->diagnose(diag::function_type_spi, functionKind,
1005-
problemIsResult, minAccess);
996+
problemIsResult, minAccess,
997+
minAccess >= AccessLevel::Public);
1006998
highlightOffendingType(diag, complainRepr);
1007999
} else {
10081000
auto diagID = diag::function_type_access;
@@ -1709,7 +1701,7 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
17091701
AccessScope accessScope =
17101702
VD->getFormalAccessScope(nullptr,
17111703
/*treatUsableFromInlineAsPublic*/true);
1712-
if (accessScope.isPublic())
1704+
if (accessScope.isPublic() && !accessScope.isSPI())
17131705
return false;
17141706

17151707
// Is this a stored property in a non-resilient struct or class?

0 commit comments

Comments
 (0)