Skip to content

Commit d182d01

Browse files
elshelsh
committed
Handle package exportability.
This PR treats package access level as exportable, preventing internally imported types from accidentally being declared in package decl signatures. Added package-specific cases to ExportabilityReason and DisallowedOriginKind to track the validity of imported types at use sites with package access scope. Added tests to cover variety of use cases. Resolves rdar://117586046&125050064&124484388&124306642
1 parent 5019d23 commit d182d01

17 files changed

+607
-177
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2286,32 +2286,32 @@ ERROR(pattern_type_not_usable_from_inline,none,
22862286
"type referenced from a '@usableFromInline' "
22872287
"%select{%select{variable|constant}0|property}1 "
22882288
"must be '@usableFromInline' or public",
2289-
(bool, bool))
2289+
(bool, bool, /*ignored*/bool))
22902290
WARNING(pattern_type_not_usable_from_inline_warn,none,
22912291
"type referenced from a '@usableFromInline' "
22922292
"%select{%select{variable|constant}0|property}1 "
22932293
"should be '@usableFromInline' or public",
2294-
(bool, bool))
2294+
(bool, bool, /*ignored*/bool))
22952295
ERROR(pattern_type_not_usable_from_inline_frozen,none,
2296-
"type referenced from a stored property in a '@frozen' struct must "
2297-
"be '@usableFromInline' or public",
2298-
(/*ignored*/bool, /*ignored*/bool))
2296+
"type referenced from a stored property in a '@frozen%select{| package}2' struct must "
2297+
"be '@usableFromInline'%select{ or public|, public, or package}2",
2298+
(/*ignored*/bool, /*ignored*/bool, bool))
22992299
ERROR(pattern_type_not_usable_from_inline_inferred,none,
23002300
"type referenced from a '@usableFromInline' "
23012301
"%select{%select{variable|constant}0|property}1 "
23022302
"with inferred type %2 "
23032303
"must be '@usableFromInline' or public",
2304-
(bool, bool, Type))
2304+
(bool, bool, Type, /*ignored*/bool))
23052305
WARNING(pattern_type_not_usable_from_inline_inferred_warn,none,
23062306
"type referenced from a '@usableFromInline' "
23072307
"%select{%select{variable|constant}0|property}1 "
23082308
"with inferred type %2 "
23092309
"should be '@usableFromInline' or public",
2310-
(bool, bool, Type))
2310+
(bool, bool, Type, /*ignored*/bool))
23112311
ERROR(pattern_type_not_usable_from_inline_inferred_frozen,none,
23122312
"type referenced from a stored property with inferred type %2 in a "
2313-
"'@frozen' struct must be '@usableFromInline' or public",
2314-
(/*ignored*/bool, /*ignored*/bool, Type))
2313+
"'@frozen%select{| package}3' struct must be '@usableFromInline'%select{ or public|, public, or package}3",
2314+
(/*ignored*/bool, /*ignored*/bool, Type, bool))
23152315

23162316
ERROR(pattern_binds_no_variables,none,
23172317
"%select{property|global variable}0 declaration does not bind any "
@@ -3626,14 +3626,18 @@ ERROR(decl_from_hidden_module,none,
36263626
"cannot use %kind0 %select{here|as property wrapper here|"
36273627
"as result builder here|"
36283628
"in an extension with public or '@usableFromInline' members|"
3629+
"in an extension with conditional conformances|"
3630+
"in an extension with public, package, or '@usableFromInline' members|"
36293631
"in an extension with conditional conformances}1; "
36303632
"%select{%2 has been imported as implementation-only|"
36313633
"it is an SPI imported from %2|"
36323634
"it is SPI|"
36333635
"%2 was imported for SPI only|"
36343636
"%2 was not imported by this file|"
36353637
"C++ types from imported module %2 do not support library evolution|"
3636-
"%2 was not imported publicly}3",
3638+
"%2 was not imported publicly|"
3639+
"%2 was imported as package}3"
3640+
"%select{||||| or as package| or as package}1",
36373641
(const Decl *, unsigned, Identifier, unsigned))
36383642
ERROR(typealias_desugars_to_type_from_hidden_module,none,
36393643
"%0 aliases '%1.%2' and cannot be used %select{here|"
@@ -3647,7 +3651,8 @@ ERROR(typealias_desugars_to_type_from_hidden_module,none,
36473651
"%4 was imported for SPI only|"
36483652
"%4 was not imported by this file|"
36493653
"C++ types from imported module %4 do not support library evolution|"
3650-
"%4 was not imported publicly}5",
3654+
"%4 was not imported publicly|"
3655+
"%4 was imported as package}5",
36513656
(const TypeAliasDecl *, StringRef, StringRef, unsigned, Identifier, unsigned))
36523657
ERROR(conformance_from_implementation_only_module,none,
36533658
"cannot use conformance of %0 to %1 %select{here|as property wrapper here|"
@@ -3660,7 +3665,8 @@ ERROR(conformance_from_implementation_only_module,none,
36603665
"%3 was imported for SPI only|"
36613666
"%3 was not imported by this file|"
36623667
"C++ types from imported module %3 do not support library evolution|"
3663-
"%3 was not imported publicly}4",
3668+
"%3 was not imported publicly|"
3669+
"%3 was imported as package}4",
36643670
(Type, Identifier, unsigned, Identifier, unsigned))
36653671
NOTE(assoc_conformance_from_implementation_only_module,none,
36663672
"in associated type %0 (inferred as %1)", (Type, Type))
@@ -6884,7 +6890,8 @@ ERROR(inlinable_decl_ref_from_hidden_module,
68846890
"%2 was imported for SPI only|"
68856891
"%2 was not imported by this file|"
68866892
"C++ APIs from imported module %2 do not support library evolution|"
6887-
"%2 was not imported publicly}3",
6893+
"%2 was not imported publicly|"
6894+
"%2 was imported as package}3",
68886895
(const ValueDecl *, unsigned, Identifier, unsigned))
68896896

68906897
WARNING(inlinable_decl_ref_from_hidden_module_warn,
@@ -6902,7 +6909,8 @@ ERROR(inlinable_typealias_desugars_to_type_from_hidden_module,
69026909
"%4 was imported for SPI only|"
69036910
"%4 was not imported by this file|"
69046911
"C++ types from imported module %4 do not support library evolution|"
6905-
"%4 was not imported publicly}5",
6912+
"%4 was not imported publicly|"
6913+
"%4 was imported as package}5",
69066914
(const TypeAliasDecl *, StringRef, StringRef, unsigned, Identifier, unsigned))
69076915

69086916
NOTE(missing_import_inserted,
@@ -6916,8 +6924,8 @@ ERROR(availability_macro_in_inlinable, none,
69166924
#undef FRAGILE_FUNC_KIND
69176925

69186926
NOTE(resilience_decl_declared_here,
6919-
none, "%kind0 is not '@usableFromInline' or public",
6920-
(const ValueDecl *))
6927+
none, "%kind0 is not '@usableFromInline'%select{ or public|, public, or package}1",
6928+
(const ValueDecl *, bool))
69216929

69226930
ERROR(class_designated_init_inlinable_resilient,none,
69236931
"initializer for class %0 is "

‎include/swift/AST/SourceFile.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,10 @@ class SourceFile final : public FileUnit {
431431
void setImportUsedPreconcurrency(
432432
AttributedImport<ImportedModule> import);
433433

434+
/// True if the highest access level of the declarations referencing
435+
/// this import in signature or inlinable code is internal or less.
436+
bool isMaxAccessLevelUsingImportInternal(AttributedImport<ImportedModule> import) const;
437+
434438
/// Return the highest access level of the declarations referencing
435439
/// this import in signature or inlinable code.
436440
AccessLevel

‎lib/AST/Decl.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2324,7 +2324,7 @@ bool VarDecl::isLayoutExposedToClients() const {
23242324
auto nominalAccess =
23252325
parent->getFormalAccessScope(/*useDC=*/nullptr,
23262326
/*treatUsableFromInlineAsPublic=*/true);
2327-
if (!nominalAccess.isPublic()) return false;
2327+
if (!nominalAccess.isPublicOrPackage()) return false;
23282328

23292329
if (!parent->getAttrs().hasAttribute<FrozenAttr>() &&
23302330
!parent->getAttrs().hasAttribute<FixedLayoutAttr>())
@@ -4571,8 +4571,10 @@ bool ValueDecl::isMoreVisibleThan(ValueDecl *other) const {
45714571

45724572
if (scope.isPublic())
45734573
return !otherScope.isPublic();
4574+
else if (scope.isPackage())
4575+
return !otherScope.isPublicOrPackage();
45744576
else if (scope.isInternal())
4575-
return !otherScope.isPublic() && !otherScope.isInternal();
4577+
return !otherScope.isPublicOrPackage() && !otherScope.isInternal();
45764578
else
45774579
return false;
45784580
}

‎lib/AST/DeclContext.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ swift::FragileFunctionKindRequest::evaluate(Evaluator &evaluator,
467467
auto effectiveAccess =
468468
VD->getFormalAccessScope(/*useDC=*/nullptr,
469469
/*treatUsableFromInlineAsPublic=*/true);
470-
if (effectiveAccess.isPublic()) {
470+
if (effectiveAccess.isPublicOrPackage()) {
471471
return {FragileFunctionKind::DefaultArgument};
472472
}
473473

@@ -499,7 +499,7 @@ swift::FragileFunctionKindRequest::evaluate(Evaluator &evaluator,
499499

500500
// If the function is not externally visible, we will not be serializing
501501
// its body.
502-
if (!funcAccess.isPublic()) {
502+
if (!funcAccess.isPublicOrPackage()) {
503503
return {FragileFunctionKind::None};
504504
}
505505

‎lib/AST/Module.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,6 +2623,12 @@ void SourceFile::setImportUsedPreconcurrency(
26232623
PreconcurrencyImportsUsed.insert(import);
26242624
}
26252625

2626+
bool SourceFile::isMaxAccessLevelUsingImportInternal(
2627+
AttributedImport<ImportedModule> import) const {
2628+
auto maxLevel = getMaxAccessLevelUsingImport(import.module.importedModule);
2629+
return maxLevel < AccessLevel::Package;
2630+
}
2631+
26262632
AccessLevel
26272633
SourceFile::getMaxAccessLevelUsingImport(
26282634
const ModuleDecl *mod) const {

‎lib/Sema/ResilienceDiagnostics.cpp

Lines changed: 86 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -66,17 +66,22 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
6666
ImportAccessLevel problematicImport = D->getImportAccessFrom(DC);
6767
if (problematicImport.has_value()) {
6868
auto SF = DC->getParentSourceFile();
69-
if (SF)
70-
SF->registerAccessLevelUsingImport(problematicImport.value(),
71-
AccessLevel::Public);
72-
73-
if (Context.LangOpts.EnableModuleApiImportRemarks) {
74-
ModuleDecl *importedVia = problematicImport->module.importedModule,
75-
*sourceModule = D->getModuleContext();
76-
Context.Diags.diagnose(loc, diag::module_api_import,
77-
D, importedVia, sourceModule,
78-
importedVia == sourceModule,
79-
/*isImplicit*/false);
69+
if (SF) {
70+
// The max used access level previously registered might be Package,
71+
// in which case, don't reset it to Public here; this ensures proper
72+
// diags between public and package.
73+
if (SF->isMaxAccessLevelUsingImportInternal(problematicImport.value()))
74+
SF->registerAccessLevelUsingImport(problematicImport.value(),
75+
AccessLevel::Public);
76+
77+
if (Context.LangOpts.EnableModuleApiImportRemarks) {
78+
ModuleDecl *importedVia = problematicImport->module.importedModule,
79+
*sourceModule = D->getModuleContext();
80+
Context.Diags.diagnose(loc, diag::module_api_import,
81+
D, importedVia, sourceModule,
82+
importedVia == sourceModule,
83+
/*isImplicit*/false);
84+
}
8085
}
8186
}
8287

@@ -119,15 +124,27 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
119124
if (isa<TypeAliasDecl>(DC) && !Context.isSwiftVersionAtLeast(6))
120125
downgradeToWarning = DowngradeToWarning::Yes;
121126

122-
auto diagID = diag::resilience_decl_unavailable;
123-
if (downgradeToWarning == DowngradeToWarning::Yes)
124-
diagID = diag::resilience_decl_unavailable_warn;
125-
126127
AccessLevel diagAccessLevel = declAccessScope.accessLevelForDiagnostics();
127-
Context.Diags.diagnose(loc, diagID, D, diagAccessLevel,
128-
fragileKind.getSelector());
129128

130-
Context.Diags.diagnose(D, diag::resilience_decl_declared_here, D);
129+
auto allowedForPkgCtx = false;
130+
auto originKind = getDisallowedOriginKind(D, where, downgradeToWarning);
131+
// For a default argument or property initializer, package type is
132+
// allowed at the use site with package access scope.
133+
if (originKind == DisallowedOriginKind::None ||
134+
originKind == DisallowedOriginKind::PackageImport) {
135+
allowedForPkgCtx = where.isPackage() && diagAccessLevel >= AccessLevel::Package;
136+
}
137+
138+
if (!allowedForPkgCtx) {
139+
auto diagID = diag::resilience_decl_unavailable;
140+
if (downgradeToWarning == DowngradeToWarning::Yes)
141+
diagID = diag::resilience_decl_unavailable_warn;
142+
143+
Context.Diags.diagnose(loc, diagID, D, diagAccessLevel,
144+
fragileKind.getSelector());
145+
146+
Context.Diags.diagnose(D, diag::resilience_decl_declared_here, D, allowedForPkgCtx);
147+
}
131148

132149
if (problematicImport.has_value() &&
133150
problematicImport->accessLevel < D->getFormalAccess()) {
@@ -156,10 +173,14 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
156173
where.getDeclContext());
157174
if (problematicImport.has_value()) {
158175
auto SF = where.getDeclContext()->getParentSourceFile();
159-
if (SF)
160-
SF->registerAccessLevelUsingImport(problematicImport.value(),
161-
AccessLevel::Public);
162-
176+
if (SF) {
177+
// The max used access level previously registered might be Package,
178+
// in which case, don't reset it to Public here; this ensures proper
179+
// diags between public and package.
180+
if (SF->isMaxAccessLevelUsingImportInternal(problematicImport.value()))
181+
SF->registerAccessLevelUsingImport(problematicImport.value(),
182+
AccessLevel::Public);
183+
}
163184
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
164185
ModuleDecl *importedVia = problematicImport->module.importedModule,
165186
*sourceModule = D->getModuleContext();
@@ -186,7 +207,8 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
186207
auto definingModule = D->getModuleContext();
187208
auto fragileKind = where.getFragileFunctionKind();
188209
bool warnPreSwift6 = originKind != DisallowedOriginKind::SPIOnly &&
189-
originKind != DisallowedOriginKind::NonPublicImport;
210+
originKind != DisallowedOriginKind::PackageImport &&
211+
originKind != DisallowedOriginKind::InternalOrLessImport;
190212
if (fragileKind.kind == FragileFunctionKind::None) {
191213
auto reason = where.getExportabilityReason();
192214
ctx.Diags
@@ -211,7 +233,8 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
211233
addMissingImport(loc, D, where);
212234

213235
// If limited by an import, note which one.
214-
if (originKind == DisallowedOriginKind::NonPublicImport) {
236+
if (originKind == DisallowedOriginKind::InternalOrLessImport ||
237+
originKind == DisallowedOriginKind::PackageImport) {
215238
const DeclContext *DC = where.getDeclContext();
216239
ImportAccessLevel limitImport = D->getImportAccessFrom(DC);
217240
assert(limitImport.has_value() &&
@@ -242,22 +265,31 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
242265
ImportAccessLevel import = D->getImportAccessFrom(DC);
243266
if (import.has_value() && reason.has_value()) {
244267
auto SF = DC->getParentSourceFile();
245-
if (SF)
246-
SF->registerAccessLevelUsingImport(import.value(),
247-
AccessLevel::Public);
268+
if (SF) {
269+
// The max used access level previously registered might be Package,
270+
// in which case, don't reset it to Public here; this ensures proper
271+
// diags between public and package.
272+
if (SF->isMaxAccessLevelUsingImportInternal(import.value()))
273+
SF->registerAccessLevelUsingImport(import.value(),
274+
AccessLevel::Public);
275+
}
248276
}
249277

250278
// Access levels from imports are reported with the others access levels.
251279
// Except for extensions, we report them here.
252-
if (originKind == DisallowedOriginKind::NonPublicImport &&
253-
reason != ExportabilityReason::ExtensionWithPublicMembers &&
254-
reason != ExportabilityReason::ExtensionWithConditionalConformances)
255-
return false;
280+
if (originKind == DisallowedOriginKind::InternalOrLessImport ||
281+
originKind == DisallowedOriginKind::PackageImport) {
282+
if (reason != ExportabilityReason::ExtensionWithPublicMembers &&
283+
reason != ExportabilityReason::ExtensionWithPackageMembers &&
284+
reason != ExportabilityReason::ExtensionWithConditionalConformances &&
285+
reason != ExportabilityReason::ExtensionWithPackageConditionalConformances)
286+
return false;
287+
}
256288

257289
if (ctx.LangOpts.EnableModuleApiImportRemarks &&
258290
import.has_value() && where.isExported() &&
259291
reason != ExportabilityReason::General &&
260-
originKind != DisallowedOriginKind::NonPublicImport) {
292+
originKind != DisallowedOriginKind::InternalOrLessImport) {
261293
// These may be reported twice, for the Type and for the TypeRepr.
262294
ModuleDecl *importedVia = import->module.importedModule,
263295
*sourceModule = D->getModuleContext();
@@ -270,6 +302,14 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
270302
if (originKind == DisallowedOriginKind::None)
271303
return false;
272304

305+
// No diags needed for extensions with package members or
306+
// conformance to types with package access scope.
307+
if (originKind == DisallowedOriginKind::PackageImport) {
308+
if (reason == ExportabilityReason::ExtensionWithPackageMembers ||
309+
reason == ExportabilityReason::ExtensionWithPackageConditionalConformances)
310+
return false;
311+
}
312+
273313
auto diagName = D->getName();
274314
if (auto accessor = dyn_cast<AccessorDecl>(D)) {
275315
// Only diagnose accessors if their disallowed origin kind differs from
@@ -313,7 +353,8 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
313353
}
314354

315355
// If limited by an import, note which one.
316-
if (originKind == DisallowedOriginKind::NonPublicImport) {
356+
if (originKind == DisallowedOriginKind::InternalOrLessImport ||
357+
originKind == DisallowedOriginKind::PackageImport) {
317358
assert(import.has_value() &&
318359
import->accessLevel < AccessLevel::Public &&
319360
"The import should still be non-public");
@@ -362,10 +403,14 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
362403
ImportAccessLevel problematicImport = ext->getImportAccessFrom(where.getDeclContext());
363404
if (problematicImport.has_value()) {
364405
auto SF = where.getDeclContext()->getParentSourceFile();
365-
if (SF)
366-
SF->registerAccessLevelUsingImport(problematicImport.value(),
367-
AccessLevel::Public);
368-
406+
if (SF) {
407+
// The max used access level previously registered might be Package,
408+
// in which case, don't reset it to Public here; this ensures proper
409+
// diags between public and package.
410+
if (SF->isMaxAccessLevelUsingImportInternal(problematicImport.value()))
411+
SF->registerAccessLevelUsingImport(problematicImport.value(),
412+
AccessLevel::Public);
413+
}
369414
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
370415
ModuleDecl *importedVia = problematicImport->module.importedModule,
371416
*sourceModule = ext->getModuleContext();
@@ -392,7 +437,8 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
392437
static_cast<unsigned>(originKind))
393438
.warnUntilSwiftVersionIf((warnIfConformanceUnavailablePreSwift6 &&
394439
originKind != DisallowedOriginKind::SPIOnly &&
395-
originKind != DisallowedOriginKind::NonPublicImport) ||
440+
originKind != DisallowedOriginKind::PackageImport &&
441+
originKind != DisallowedOriginKind::InternalOrLessImport) ||
396442
originKind == DisallowedOriginKind::MissingImport,
397443
6);
398444

@@ -401,7 +447,8 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
401447
addMissingImport(loc, ext, where);
402448

403449
// If limited by an import, note which one.
404-
if (originKind == DisallowedOriginKind::NonPublicImport) {
450+
if (originKind == DisallowedOriginKind::InternalOrLessImport ||
451+
originKind == DisallowedOriginKind::PackageImport) {
405452
const DeclContext *DC = where.getDeclContext();
406453
ImportAccessLevel limitImport = ext->getImportAccessFrom(DC);
407454
assert(limitImport.has_value() &&

0 commit comments

Comments
 (0)