Skip to content

Commit a077469

Browse files
tshortlitshortli
committed
Sema: Consolidate logic for -Rmodule-api-import.
`recordRequiredImportAccessLevelForDecl()` now handles the decision to emit a remark which consolidates some duplicated logic.
1 parent 53a137b commit a077469

File tree

3 files changed

+83
-111
lines changed

3 files changed

+83
-111
lines changed

‎lib/Sema/ResilienceDiagnostics.cpp‎

Lines changed: 42 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,15 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
7676
}
7777

7878
// 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.
82-
ImportAccessLevel problematicImport = D->getImportAccessFrom(DC);
83-
if (problematicImport.has_value()) {
84-
if (Context.LangOpts.EnableModuleApiImportRemarks) {
85-
ModuleDecl *importedVia = problematicImport->module.importedModule,
86-
*sourceModule = D->getModuleContext();
87-
Context.Diags.diagnose(loc, diag::module_api_import,
88-
D, importedVia, sourceModule,
89-
importedVia == sourceModule,
90-
/*isImplicit*/false);
91-
}
92-
}
79+
recordRequiredImportAccessLevelForDecl(
80+
D, DC, AccessLevel::Public,
81+
[&](AttributedImport<ImportedModule> attributedImport) {
82+
ModuleDecl *importedVia = attributedImport.module.importedModule,
83+
*sourceModule = D->getModuleContext();
84+
Context.Diags.diagnose(loc, diag::module_api_import, D, importedVia,
85+
sourceModule, importedVia == sourceModule,
86+
/*isImplicit*/ false);
87+
});
9388

9489
// General check on access-level of the decl.
9590
auto declAccessScope =
@@ -140,6 +135,7 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
140135

141136
Context.Diags.diagnose(D, diag::resilience_decl_declared_here, D);
142137

138+
ImportAccessLevel problematicImport = D->getImportAccessFrom(DC);
143139
if (problematicImport.has_value() &&
144140
problematicImport->accessLevel < D->getFormalAccess()) {
145141
Context.Diags.diagnose(problematicImport->importLoc,
@@ -166,19 +162,14 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
166162

167163
// Remember that the module defining the underlying type must be imported
168164
// publicly.
169-
recordRequiredImportAccessLevelForDecl(D, DC, AccessLevel::Public);
170-
171-
// Emit a remark explaining the required access level.
172-
ImportAccessLevel problematicImport = D->getImportAccessFrom(DC);
173-
if (problematicImport.has_value()) {
174-
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
175-
ModuleDecl *importedVia = problematicImport->module.importedModule,
176-
*sourceModule = D->getModuleContext();
177-
ctx.Diags.diagnose(loc, diag::module_api_import_aliases,
178-
D, importedVia, sourceModule,
179-
importedVia == sourceModule);
180-
}
181-
}
165+
recordRequiredImportAccessLevelForDecl(
166+
D, DC, AccessLevel::Public,
167+
[&](AttributedImport<ImportedModule> attributedImport) {
168+
ModuleDecl *importedVia = attributedImport.module.importedModule,
169+
*sourceModule = D->getModuleContext();
170+
ctx.Diags.diagnose(loc, diag::module_api_import_aliases, D, importedVia,
171+
sourceModule, importedVia == sourceModule);
172+
});
182173

183174
auto ignoredDowngradeToWarning = DowngradeToWarning::No;
184175
auto originKind =
@@ -249,7 +240,19 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
249240
auto originKind = getDisallowedOriginKind(D, where, downgradeToWarning);
250241

251242
// Remember that the module defining the decl must be imported publicly.
252-
recordRequiredImportAccessLevelForDecl(D, DC, AccessLevel::Public);
243+
recordRequiredImportAccessLevelForDecl(
244+
D, DC, AccessLevel::Public,
245+
[&](AttributedImport<ImportedModule> attributedImport) {
246+
if (where.isExported() && reason != ExportabilityReason::General &&
247+
originKind != DisallowedOriginKind::NonPublicImport) {
248+
// These may be reported twice, for the Type and for the TypeRepr.
249+
ModuleDecl *importedVia = attributedImport.module.importedModule,
250+
*sourceModule = D->getModuleContext();
251+
ctx.Diags.diagnose(loc, diag::module_api_import, D, importedVia,
252+
sourceModule, importedVia == sourceModule,
253+
/*isImplicit*/ false);
254+
}
255+
});
253256

254257
// Access levels from imports are reported with the others access levels.
255258
// Except for extensions and protocol conformances, we report them here.
@@ -269,21 +272,6 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
269272
return false;
270273
}
271274

272-
// Emit a remark explaining the required access level.
273-
ImportAccessLevel import = D->getImportAccessFrom(DC);
274-
if (ctx.LangOpts.EnableModuleApiImportRemarks &&
275-
import.has_value() && where.isExported() &&
276-
reason != ExportabilityReason::General &&
277-
originKind != DisallowedOriginKind::NonPublicImport) {
278-
// These may be reported twice, for the Type and for the TypeRepr.
279-
ModuleDecl *importedVia = import->module.importedModule,
280-
*sourceModule = D->getModuleContext();
281-
ctx.Diags.diagnose(loc, diag::module_api_import,
282-
D, importedVia, sourceModule,
283-
importedVia == sourceModule,
284-
/*isImplicit*/false);
285-
}
286-
287275
if (originKind == DisallowedOriginKind::None)
288276
return false;
289277

@@ -330,6 +318,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
330318
}
331319

332320
// If limited by an import, note which one.
321+
ImportAccessLevel import = D->getImportAccessFrom(DC);
333322
if (originKind == DisallowedOriginKind::NonPublicImport) {
334323
assert(import.has_value() &&
335324
import->accessLevel < AccessLevel::Public &&
@@ -379,20 +368,16 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
379368

380369
// Remember that the module defining the conformance must be imported
381370
// publicly.
382-
recordRequiredImportAccessLevelForDecl(ext, DC, AccessLevel::Public);
383-
384-
// Emit a remark explaining the required access level.
385-
ImportAccessLevel problematicImport = ext->getImportAccessFrom(DC);
386-
if (problematicImport.has_value()) {
387-
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
388-
ModuleDecl *importedVia = problematicImport->module.importedModule,
389-
*sourceModule = ext->getModuleContext();
390-
ctx.Diags.diagnose(loc, diag::module_api_import_conformance,
391-
rootConf->getType(), rootConf->getProtocol(),
392-
importedVia, sourceModule,
393-
importedVia == sourceModule);
394-
}
395-
}
371+
recordRequiredImportAccessLevelForDecl(
372+
ext, DC, AccessLevel::Public,
373+
[&](AttributedImport<ImportedModule> attributedImport) {
374+
ModuleDecl *importedVia = attributedImport.module.importedModule,
375+
*sourceModule = ext->getModuleContext();
376+
ctx.Diags.diagnose(loc, diag::module_api_import_conformance,
377+
rootConf->getType(), rootConf->getProtocol(),
378+
importedVia, sourceModule,
379+
importedVia == sourceModule);
380+
});
396381

397382
auto originKind = getDisallowedOriginKind(ext, where);
398383
if (originKind == DisallowedOriginKind::None)

‎lib/Sema/TypeCheckAccess.cpp‎

Lines changed: 35 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -211,22 +211,17 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
211211
auto report = [&](const DeclRefTypeRepr *typeRepr, const ValueDecl *VD) {
212212
// Remember that the module defining the decl must be imported publicly.
213213
recordRequiredImportAccessLevelForDecl(
214-
VD, useDC, contextAccessScope.accessLevelForDiagnostics());
215-
216-
// Emit a remark explaining the required access level.
217-
ImportAccessLevel import = VD->getImportAccessFrom(useDC);
218-
if (import.has_value()) {
219-
if (Context.LangOpts.EnableModuleApiImportRemarks) {
220-
SourceLoc diagLoc = typeRepr? typeRepr->getLoc()
221-
: extractNearestSourceLoc(useDC);
222-
ModuleDecl *importedVia = import->module.importedModule,
223-
*sourceModule = VD->getModuleContext();
224-
Context.Diags.diagnose(diagLoc, diag::module_api_import,
225-
VD, importedVia, sourceModule,
226-
importedVia == sourceModule,
227-
/*isImplicit*/!typeRepr);
228-
}
229-
}
214+
VD, useDC, contextAccessScope.accessLevelForDiagnostics(),
215+
[&](AttributedImport<ImportedModule> attributedImport) {
216+
SourceLoc diagLoc =
217+
typeRepr ? typeRepr->getLoc() : extractNearestSourceLoc(useDC);
218+
ModuleDecl *importedVia = attributedImport.module.importedModule,
219+
*sourceModule = VD->getModuleContext();
220+
Context.Diags.diagnose(diagLoc, diag::module_api_import, VD,
221+
importedVia, sourceModule,
222+
importedVia == sourceModule,
223+
/*isImplicit*/ !typeRepr);
224+
});
230225
};
231226

232227
if (typeRepr) {
@@ -2396,22 +2391,15 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
23962391

23972392
// Remember that the module defining the extended type must be imported
23982393
// publicly.
2399-
recordRequiredImportAccessLevelForDecl(extendedType, DC,
2400-
AccessLevel::Public);
2401-
2402-
// Emit a remark explaining the required access level.
2403-
ImportAccessLevel import = extendedType->getImportAccessFrom(DC);
2404-
if (import.has_value()) {
2405-
auto &ctx = DC->getASTContext();
2406-
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
2407-
ModuleDecl *importedVia = import->module.importedModule,
2408-
*sourceModule = ED->getModuleContext();
2409-
ED->diagnose(diag::module_api_import,
2410-
ED, importedVia, sourceModule,
2411-
importedVia == sourceModule,
2412-
/*isImplicit*/false);
2413-
}
2414-
}
2394+
recordRequiredImportAccessLevelForDecl(
2395+
extendedType, DC, AccessLevel::Public,
2396+
[&](AttributedImport<ImportedModule> attributedImport) {
2397+
ModuleDecl *importedVia = attributedImport.module.importedModule,
2398+
*sourceModule = ED->getModuleContext();
2399+
ED->diagnose(diag::module_api_import, ED, importedVia, sourceModule,
2400+
importedVia == sourceModule,
2401+
/*isImplicit*/ false);
2402+
});
24152403
}
24162404
}
24172405

@@ -2508,9 +2496,9 @@ DisallowedOriginKind swift::getDisallowedOriginKind(const Decl *decl,
25082496
return getDisallowedOriginKind(decl, where, downgradeToWarning);
25092497
}
25102498

2511-
void swift::recordRequiredImportAccessLevelForDecl(const Decl *decl,
2512-
const DeclContext *dc,
2513-
AccessLevel accessLevel) {
2499+
void swift::recordRequiredImportAccessLevelForDecl(
2500+
const Decl *decl, const DeclContext *dc, AccessLevel accessLevel,
2501+
RequiredImportAccessLevelCallback remark) {
25142502
auto sf = dc->getParentSourceFile();
25152503
if (!sf)
25162504
return;
@@ -2528,6 +2516,9 @@ void swift::recordRequiredImportAccessLevelForDecl(const Decl *decl,
25282516
// module as requiring the minimum access level too.
25292517
if (importedModule != definingModule)
25302518
sf->registerRequiredAccessLevelForModule(importedModule, accessLevel);
2519+
2520+
if (dc->getASTContext().LangOpts.EnableModuleApiImportRemarks)
2521+
remark(*attributedImport);
25312522
}
25322523
}
25332524

@@ -2601,22 +2592,15 @@ void registerPackageAccessForPackageExtendedType(ExtensionDecl *ED) {
26012592

26022593
// Remember that the module defining the decl must be imported with at least
26032594
// package visibility.
2604-
recordRequiredImportAccessLevelForDecl(extendedType, DC,
2605-
AccessLevel::Package);
2606-
2607-
// Emit a remark explaining the required access level.
2608-
ImportAccessLevel import = extendedType->getImportAccessFrom(DC);
2609-
if (import.has_value()) {
2610-
auto &ctx = DC->getASTContext();
2611-
if (ctx.LangOpts.EnableModuleApiImportRemarks) {
2612-
ModuleDecl *importedVia = import->module.importedModule,
2613-
*sourceModule = ED->getModuleContext();
2614-
ED->diagnose(diag::module_api_import,
2615-
ED, importedVia, sourceModule,
2616-
importedVia == sourceModule,
2617-
/*isImplicit*/false);
2618-
}
2619-
}
2595+
recordRequiredImportAccessLevelForDecl(
2596+
extendedType, DC, AccessLevel::Package,
2597+
[&](AttributedImport<ImportedModule> attributedImport) {
2598+
ModuleDecl *importedVia = attributedImport.module.importedModule,
2599+
*sourceModule = ED->getModuleContext();
2600+
ED->diagnose(diag::module_api_import, ED, importedVia, sourceModule,
2601+
importedVia == sourceModule,
2602+
/*isImplicit*/ false);
2603+
});
26202604
}
26212605

26222606
void swift::checkAccessControl(Decl *D) {

‎lib/Sema/TypeChecker.h‎

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

1525+
using RequiredImportAccessLevelCallback =
1526+
std::function<void(AttributedImport<ImportedModule>)>;
1527+
15251528
/// Make a note that uses of \p decl in \p dc require that the decl's defining
15261529
/// module be imported with an access level that is at least as permissive as \p
15271530
/// accessLevel.
1528-
void recordRequiredImportAccessLevelForDecl(const Decl *decl,
1529-
const DeclContext *dc,
1530-
AccessLevel accessLevel);
1531+
void recordRequiredImportAccessLevelForDecl(
1532+
const Decl *decl, const DeclContext *dc, AccessLevel accessLevel,
1533+
RequiredImportAccessLevelCallback remark);
15311534

15321535
/// Report imports that are marked public but are not used in API.
15331536
void diagnoseUnnecessaryPublicImports(SourceFile &SF);

0 commit comments

Comments
 (0)