Skip to content

Commit 53a137b

Browse files
tshortlitshortli
committed
Sema: Refactor superfluous public import tracking.
In anticipation of reusing minimum access level information for diagnostics related to the `MemberImportVisibility` feature, refactor the way the type checker tracks the modules which must be imported publicly. Recording minimum access levels is no longer restricted to modules that are already imported in a source file since `MemberImportVisibility` diagnostics will need this information when emitting fix-its for modules that are not already imported. Unblocks rdar://126637855.
1 parent 9906199 commit 53a137b

File tree

5 files changed

+86
-62
lines changed

5 files changed

+86
-62
lines changed

‎include/swift/AST/SourceFile.h‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -440,9 +440,11 @@ class SourceFile final : public FileUnit {
440440
AccessLevel
441441
getMaxAccessLevelUsingImport(const ModuleDecl *import) const;
442442

443-
/// Register the use of \p import from an API with \p accessLevel.
444-
void registerAccessLevelUsingImport(AttributedImport<ImportedModule> import,
445-
AccessLevel accessLevel);
443+
/// Register the requirement that \p mod be imported with an access level
444+
/// that is at least as permissive as \p accessLevel in order to satisfy
445+
/// access or exportability checking constraints.
446+
void registerRequiredAccessLevelForModule(ModuleDecl *mod,
447+
AccessLevel accessLevel);
446448

447449
enum ImportQueryKind {
448450
/// Return the results for testable or private imports.

‎lib/AST/Module.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2728,10 +2728,8 @@ SourceFile::getMaxAccessLevelUsingImport(
27282728
return known->second;
27292729
}
27302730

2731-
void SourceFile::registerAccessLevelUsingImport(
2732-
AttributedImport<ImportedModule> import,
2733-
AccessLevel accessLevel) {
2734-
auto mod = import.module.importedModule;
2731+
void SourceFile::registerRequiredAccessLevelForModule(ModuleDecl *mod,
2732+
AccessLevel accessLevel) {
27352733
auto known = ImportsUseAccessLevel.find(mod);
27362734
if (known == ImportsUseAccessLevel.end())
27372735
ImportsUseAccessLevel[mod] = accessLevel;
@@ -2754,7 +2752,7 @@ void SourceFile::registerAccessLevelUsingImport(
27542752
auto otherImportModName = otherImportMod->getName();
27552753
if (otherImportMod == declaringMod ||
27562754
llvm::find(bystanders, otherImportModName) != bystanders.end()) {
2757-
registerAccessLevelUsingImport(otherImport, accessLevel);
2755+
registerRequiredAccessLevelForModule(otherImportMod, accessLevel);
27582756
}
27592757
}
27602758
}

‎lib/Sema/ResilienceDiagnostics.cpp‎

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,12 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
7575
}
7676
}
7777

78+
// Remember that the module defining the decl must be imported publicly.
79+
recordRequiredImportAccessLevelForDecl(D, DC, AccessLevel::Public);
80+
81+
// Emit a remark explaining the required access level.
7882
ImportAccessLevel problematicImport = D->getImportAccessFrom(DC);
7983
if (problematicImport.has_value()) {
80-
auto SF = DC->getParentSourceFile();
81-
if (SF)
82-
SF->registerAccessLevelUsingImport(problematicImport.value(),
83-
AccessLevel::Public);
84-
8584
if (Context.LangOpts.EnableModuleApiImportRemarks) {
8685
ModuleDecl *importedVia = problematicImport->module.importedModule,
8786
*sourceModule = D->getModuleContext();
@@ -161,17 +160,17 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
161160
if (!D)
162161
return false;
163162

164-
auto exportingModule = where.getDeclContext()->getParentModule();
163+
const DeclContext *DC = where.getDeclContext();
164+
auto exportingModule = DC->getParentModule();
165165
ASTContext &ctx = exportingModule->getASTContext();
166166

167-
ImportAccessLevel problematicImport = D->getImportAccessFrom(
168-
where.getDeclContext());
169-
if (problematicImport.has_value()) {
170-
auto SF = where.getDeclContext()->getParentSourceFile();
171-
if (SF)
172-
SF->registerAccessLevelUsingImport(problematicImport.value(),
173-
AccessLevel::Public);
167+
// Remember that the module defining the underlying type must be imported
168+
// publicly.
169+
recordRequiredImportAccessLevelForDecl(D, DC, AccessLevel::Public);
174170

171+
// Emit a remark explaining the required access level.
172+
ImportAccessLevel problematicImport = D->getImportAccessFrom(DC);
173+
if (problematicImport.has_value()) {
175174
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
176175
ModuleDecl *importedVia = problematicImport->module.importedModule,
177176
*sourceModule = D->getModuleContext();
@@ -224,7 +223,6 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
224223

225224
// If limited by an import, note which one.
226225
if (originKind == DisallowedOriginKind::NonPublicImport) {
227-
const DeclContext *DC = where.getDeclContext();
228226
ImportAccessLevel limitImport = D->getImportAccessFrom(DC);
229227
assert(limitImport.has_value() &&
230228
limitImport->accessLevel < AccessLevel::Public &&
@@ -250,14 +248,8 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
250248
ASTContext &ctx = DC->getASTContext();
251249
auto originKind = getDisallowedOriginKind(D, where, downgradeToWarning);
252250

253-
// If we got here it was used in API, we can record the use of the import.
254-
ImportAccessLevel import = D->getImportAccessFrom(DC);
255-
if (import.has_value() && reason.has_value()) {
256-
auto SF = DC->getParentSourceFile();
257-
if (SF)
258-
SF->registerAccessLevelUsingImport(import.value(),
259-
AccessLevel::Public);
260-
}
251+
// Remember that the module defining the decl must be imported publicly.
252+
recordRequiredImportAccessLevelForDecl(D, DC, AccessLevel::Public);
261253

262254
// Access levels from imports are reported with the others access levels.
263255
// Except for extensions and protocol conformances, we report them here.
@@ -277,6 +269,8 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
277269
return false;
278270
}
279271

272+
// Emit a remark explaining the required access level.
273+
ImportAccessLevel import = D->getImportAccessFrom(DC);
280274
if (ctx.LangOpts.EnableModuleApiImportRemarks &&
281275
import.has_value() && where.isExported() &&
282276
reason != ExportabilityReason::General &&
@@ -379,16 +373,17 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
379373
if (ext->getParentModule()->isBuiltinModule())
380374
return false;
381375

376+
const DeclContext *DC = where.getDeclContext();
382377
ModuleDecl *M = ext->getParentModule();
383378
ASTContext &ctx = M->getASTContext();
384379

385-
ImportAccessLevel problematicImport = ext->getImportAccessFrom(where.getDeclContext());
386-
if (problematicImport.has_value()) {
387-
auto SF = where.getDeclContext()->getParentSourceFile();
388-
if (SF)
389-
SF->registerAccessLevelUsingImport(problematicImport.value(),
390-
AccessLevel::Public);
380+
// Remember that the module defining the conformance must be imported
381+
// publicly.
382+
recordRequiredImportAccessLevelForDecl(ext, DC, AccessLevel::Public);
391383

384+
// Emit a remark explaining the required access level.
385+
ImportAccessLevel problematicImport = ext->getImportAccessFrom(DC);
386+
if (problematicImport.has_value()) {
392387
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
393388
ModuleDecl *importedVia = problematicImport->module.importedModule,
394389
*sourceModule = ext->getModuleContext();
@@ -425,7 +420,6 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
425420

426421
// If limited by an import, note which one.
427422
if (originKind == DisallowedOriginKind::NonPublicImport) {
428-
const DeclContext *DC = where.getDeclContext();
429423
ImportAccessLevel limitImport = ext->getImportAccessFrom(DC);
430424
assert(limitImport.has_value() &&
431425
limitImport->accessLevel < AccessLevel::Public &&

‎lib/Sema/TypeCheckAccess.cpp‎

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -207,18 +207,15 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
207207
return;
208208

209209
// Report where it was imported from.
210-
if (contextAccessScope.isPublic() ||
211-
contextAccessScope.isPackage()) {
212-
auto SF = useDC->getParentSourceFile();
210+
if (contextAccessScope.isPublicOrPackage()) {
213211
auto report = [&](const DeclRefTypeRepr *typeRepr, const ValueDecl *VD) {
212+
// Remember that the module defining the decl must be imported publicly.
213+
recordRequiredImportAccessLevelForDecl(
214+
VD, useDC, contextAccessScope.accessLevelForDiagnostics());
215+
216+
// Emit a remark explaining the required access level.
214217
ImportAccessLevel import = VD->getImportAccessFrom(useDC);
215218
if (import.has_value()) {
216-
if (SF) {
217-
auto useLevel = contextAccessScope.isPublic() ? AccessLevel::Public
218-
: AccessLevel::Package;
219-
SF->registerAccessLevelUsingImport(import.value(), useLevel);
220-
}
221-
222219
if (Context.LangOpts.EnableModuleApiImportRemarks) {
223220
SourceLoc diagLoc = typeRepr? typeRepr->getLoc()
224221
: extractNearestSourceLoc(useDC);
@@ -2390,20 +2387,21 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
23902387
!ED->getInherited().empty());
23912388
checkConstrainedExtensionRequirements(ED, hasExportedMembers);
23922389

2393-
if (!hasExportedMembers &&
2394-
!ED->getInherited().empty()) {
2395-
// If we haven't already visited the extended nominal visit it here.
2396-
// This logic is too wide but prevents false reports of an unused public
2397-
// import. We should instead check for public generic requirements
2398-
// similarly to ShouldPrintForModuleInterface::shouldPrint.
2390+
// If we haven't already visited the extended nominal visit it here.
2391+
// This logic is too wide but prevents false reports of an unused public
2392+
// import. We should instead check for public generic requirements
2393+
// similarly to ShouldPrintForModuleInterface::shouldPrint.
2394+
if (!hasExportedMembers && !ED->getInherited().empty()) {
23992395
auto DC = Where.getDeclContext();
2400-
ImportAccessLevel import = extendedType->getImportAccessFrom(DC);
2401-
if (import.has_value()) {
2402-
auto SF = DC->getParentSourceFile();
2403-
if (SF)
2404-
SF->registerAccessLevelUsingImport(import.value(),
2396+
2397+
// Remember that the module defining the extended type must be imported
2398+
// publicly.
2399+
recordRequiredImportAccessLevelForDecl(extendedType, DC,
24052400
AccessLevel::Public);
24062401

2402+
// Emit a remark explaining the required access level.
2403+
ImportAccessLevel import = extendedType->getImportAccessFrom(DC);
2404+
if (import.has_value()) {
24072405
auto &ctx = DC->getASTContext();
24082406
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
24092407
ModuleDecl *importedVia = import->module.importedModule,
@@ -2510,6 +2508,29 @@ DisallowedOriginKind swift::getDisallowedOriginKind(const Decl *decl,
25102508
return getDisallowedOriginKind(decl, where, downgradeToWarning);
25112509
}
25122510

2511+
void swift::recordRequiredImportAccessLevelForDecl(const Decl *decl,
2512+
const DeclContext *dc,
2513+
AccessLevel accessLevel) {
2514+
auto sf = dc->getParentSourceFile();
2515+
if (!sf)
2516+
return;
2517+
2518+
auto definingModule = decl->getModuleContext();
2519+
if (definingModule == dc->getParentModule())
2520+
return;
2521+
2522+
sf->registerRequiredAccessLevelForModule(definingModule, accessLevel);
2523+
2524+
if (auto attributedImport = sf->getImportAccessLevel(definingModule)) {
2525+
auto importedModule = attributedImport->module.importedModule;
2526+
2527+
// If the defining module is transitively imported, mark the responsible
2528+
// module as requiring the minimum access level too.
2529+
if (importedModule != definingModule)
2530+
sf->registerRequiredAccessLevelForModule(importedModule, accessLevel);
2531+
}
2532+
}
2533+
25132534
void swift::diagnoseUnnecessaryPublicImports(SourceFile &SF) {
25142535
ASTContext &ctx = SF.getASTContext();
25152536
if (ctx.TypeCheckerOpts.SkipFunctionBodies != FunctionBodySkipping::None)
@@ -2577,13 +2598,15 @@ void registerPackageAccessForPackageExtendedType(ExtensionDecl *ED) {
25772598
return;
25782599

25792600
DeclContext *DC = ED->getDeclContext();
2580-
ImportAccessLevel import = extendedType->getImportAccessFrom(DC);
2581-
if (import.has_value()) {
2582-
auto SF = DC->getParentSourceFile();
2583-
if (SF)
2584-
SF->registerAccessLevelUsingImport(import.value(),
2601+
2602+
// Remember that the module defining the decl must be imported with at least
2603+
// package visibility.
2604+
recordRequiredImportAccessLevelForDecl(extendedType, DC,
25852605
AccessLevel::Package);
25862606

2607+
// Emit a remark explaining the required access level.
2608+
ImportAccessLevel import = extendedType->getImportAccessFrom(DC);
2609+
if (import.has_value()) {
25872610
auto &ctx = DC->getASTContext();
25882611
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
25892612
ModuleDecl *importedVia = import->module.importedModule,

‎lib/Sema/TypeChecker.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,6 +1522,13 @@ class DiscriminatorFinder : public ASTWalker {
15221522
}
15231523
};
15241524

1525+
/// Make a note that uses of \p decl in \p dc require that the decl's defining
1526+
/// module be imported with an access level that is at least as permissive as \p
1527+
/// accessLevel.
1528+
void recordRequiredImportAccessLevelForDecl(const Decl *decl,
1529+
const DeclContext *dc,
1530+
AccessLevel accessLevel);
1531+
15251532
/// Report imports that are marked public but are not used in API.
15261533
void diagnoseUnnecessaryPublicImports(SourceFile &SF);
15271534

0 commit comments

Comments
 (0)