Skip to content

Commit 0a2167c

Browse files
tshortlitshortli
authored
Merge pull request swiftlang#73063 from tshortli/rename-extension-import-visibility-feature
Rename ExtensionImportVisibility to MemberImportVisibility and fix bugs
2 parents 37eb004 + 865c26c commit 0a2167c

34 files changed

+369
-127
lines changed

‎include/swift/AST/Decl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2788,7 +2788,7 @@ class ValueDecl : public Decl {
27882788
public:
27892789
/// Find the import that makes the given declaration available.
27902790
std::optional<AttributedImport<ImportedModule>>
2791-
findImport(const DeclContext *fromDC);
2791+
findImport(const DeclContext *fromDC) const;
27922792

27932793
/// Return true if this protocol member is a protocol requirement.
27942794
///

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ ERROR(init_candidate_inaccessible,none,
167167

168168
ERROR(candidate_from_missing_import,none,
169169
"%0 %1 is not available due to missing import of defining module %2",
170-
(DescriptiveDeclKind, DeclName, DeclName))
170+
(DescriptiveDeclKind, DeclName, ModuleDecl *))
171171
NOTE(candidate_add_import,none,
172-
"add import of module %0", (DeclName))
172+
"add import of module %0", (ModuleDecl *))
173173

174174
ERROR(cannot_pass_rvalue_mutating_subelement,none,
175175
"cannot use mutating member on immutable value: %0",

‎include/swift/AST/Module.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,6 +1269,11 @@ void collectParsedExportedImports(const ModuleDecl *M,
12691269
llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> &QualifiedImports,
12701270
llvm::function_ref<bool(AttributedImport<ImportedModule>)> includeImport = nullptr);
12711271

1272+
/// If the import that would make the given declaration visibile is absent,
1273+
/// emit a diagnostic and a fix-it suggesting adding the missing import.
1274+
bool diagnoseMissingImportForMember(const ValueDecl *decl,
1275+
const DeclContext *dc, SourceLoc loc);
1276+
12721277
} // end namespace swift
12731278

12741279
#endif

‎include/swift/AST/SourceFile.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,9 @@ class SourceFile final : public FileUnit {
457457
/// If not, we can fast-path module checks.
458458
bool hasImportsWithFlag(ImportFlags flag) const;
459459

460+
/// Returns the import flags that are active on imports of \p module.
461+
ImportFlags getImportFlags(const ModuleDecl *module) const;
462+
460463
/// Get the most permissive restriction applied to the imports of \p module.
461464
RestrictedImportKind getRestrictedImportKind(const ModuleDecl *module) const;
462465

‎include/swift/Basic/Features.def

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,9 +364,8 @@ EXPERIMENTAL_FEATURE(ClosureIsolation, true)
364364
// Enable isolated(any) attribute on function types.
365365
CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE(IsolatedAny, true)
366366

367-
368-
// Whether members of extensions respect the enclosing file's imports.
369-
EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE(ExtensionImportVisibility, true)
367+
// Whether lookup of members respects the enclosing file's imports.
368+
EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE(MemberImportVisibility, true)
370369

371370
// Alias for IsolatedAny
372371
EXPERIMENTAL_FEATURE(IsolatedAny2, true)

‎lib/AST/Decl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3865,7 +3865,7 @@ ValueDecl::getSatisfiedProtocolRequirements(bool Sorted) const {
38653865
}
38663866

38673867
std::optional<AttributedImport<ImportedModule>>
3868-
ValueDecl::findImport(const DeclContext *fromDC) {
3868+
ValueDecl::findImport(const DeclContext *fromDC) const {
38693869
// If the type is from the current module, there's no import.
38703870
auto module = getModuleContext();
38713871
if (module == fromDC->getParentModule())

‎lib/AST/FeatureSet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ static bool usesFeatureIsolatedAny(Decl *decl) {
660660
});
661661
}
662662

663-
UNINTERESTING_FEATURE(ExtensionImportVisibility)
663+
UNINTERESTING_FEATURE(MemberImportVisibility)
664664
UNINTERESTING_FEATURE(IsolatedAny2)
665665

666666
static bool usesFeatureGlobalActorIsolatedTypesUsability(Decl *decl) {

‎lib/AST/Module.cpp

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2702,6 +2702,15 @@ bool SourceFile::hasImportsWithFlag(ImportFlags flag) const {
27022702
ctx.evaluator, HasImportsMatchingFlagRequest{mutableThis, flag}, false);
27032703
}
27042704

2705+
ImportFlags SourceFile::getImportFlags(const ModuleDecl *module) const {
2706+
unsigned flags = 0x0;
2707+
for (auto import : *Imports) {
2708+
if (import.module.importedModule == module)
2709+
flags |= import.options.toRaw();
2710+
}
2711+
return ImportFlags(flags);
2712+
}
2713+
27052714
bool SourceFile::hasTestableOrPrivateImport(
27062715
AccessLevel accessLevel, const swift::ValueDecl *ofDecl,
27072716
SourceFile::ImportQueryKind queryKind) const {
@@ -4001,3 +4010,62 @@ version::Version ModuleDecl::getLanguageVersionBuiltWith() const {
40014010

40024011
return version::Version();
40034012
}
4013+
4014+
bool swift::diagnoseMissingImportForMember(const ValueDecl *decl,
4015+
const DeclContext *dc,
4016+
SourceLoc loc) {
4017+
if (decl->findImport(dc))
4018+
return false;
4019+
4020+
auto &ctx = dc->getASTContext();
4021+
auto definingModule = decl->getModuleContext();
4022+
ctx.Diags.diagnose(loc, diag::candidate_from_missing_import,
4023+
decl->getDescriptiveKind(), decl->getName(),
4024+
definingModule);
4025+
4026+
SourceLoc bestLoc =
4027+
ctx.Diags.getBestAddImportFixItLoc(decl, dc->getParentSourceFile());
4028+
if (!bestLoc.isValid())
4029+
return false;
4030+
4031+
llvm::SmallString<64> importText;
4032+
4033+
// Check other source files for import flags that should be applied to the
4034+
// fix-it for consistency with the rest of the imports in the module.
4035+
auto parentModule = dc->getParentModule();
4036+
OptionSet<ImportFlags> flags;
4037+
for (auto file : parentModule->getFiles()) {
4038+
if (auto sf = dyn_cast<SourceFile>(file))
4039+
flags |= sf->getImportFlags(definingModule);
4040+
}
4041+
4042+
if (flags.contains(ImportFlags::Exported) ||
4043+
parentModule->isClangOverlayOf(definingModule))
4044+
importText += "@_exported ";
4045+
if (flags.contains(ImportFlags::ImplementationOnly))
4046+
importText += "@_implementationOnly ";
4047+
if (flags.contains(ImportFlags::WeakLinked))
4048+
importText += "@_weakLinked ";
4049+
if (flags.contains(ImportFlags::SPIOnly))
4050+
importText += "@_spiOnly ";
4051+
4052+
// FIXME: Access level should be considered, too.
4053+
4054+
// @_spi imports.
4055+
if (decl->isSPI()) {
4056+
auto spiGroups = decl->getSPIGroups();
4057+
if (!spiGroups.empty()) {
4058+
importText += "@_spi(";
4059+
importText += spiGroups[0].str();
4060+
importText += ") ";
4061+
}
4062+
}
4063+
4064+
importText += "import ";
4065+
importText += definingModule->getName().str();
4066+
importText += "\n";
4067+
ctx.Diags.diagnose(bestLoc, diag::candidate_add_import, definingModule)
4068+
.fixItInsert(bestLoc, importText);
4069+
4070+
return true;
4071+
}

‎lib/AST/NameLookup.cpp

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2259,6 +2259,48 @@ void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
22592259
vec.push_back(method);
22602260
}
22612261

2262+
static bool missingExplicitImportForMemberDecl(const DeclContext *dc,
2263+
ValueDecl *decl) {
2264+
// Only require explicit imports for members when MemberImportVisibility is
2265+
// enabled.
2266+
if (!dc->getASTContext().LangOpts.hasFeature(Feature::MemberImportVisibility))
2267+
return false;
2268+
2269+
// If the decl is in the same module, no import is required.
2270+
auto declModule = decl->getDeclContext()->getParentModule();
2271+
if (declModule == dc->getParentModule())
2272+
return false;
2273+
2274+
// Source files are not expected to contain an import for the clang header
2275+
// module.
2276+
if (auto *loader = dc->getASTContext().getClangModuleLoader()) {
2277+
if (declModule == loader->getImportedHeaderModule())
2278+
return false;
2279+
}
2280+
2281+
// Only require an import in the context of user authored source file.
2282+
auto sf = dc->getParentSourceFile();
2283+
if (!sf)
2284+
return false;
2285+
2286+
switch (sf->Kind) {
2287+
case SourceFileKind::SIL:
2288+
case SourceFileKind::Interface:
2289+
case SourceFileKind::MacroExpansion:
2290+
case SourceFileKind::DefaultArgument:
2291+
return false;
2292+
case SourceFileKind::Library:
2293+
case SourceFileKind::Main:
2294+
break;
2295+
}
2296+
2297+
// If we've found an import, we're done.
2298+
if (decl->findImport(dc))
2299+
return false;
2300+
2301+
return true;
2302+
}
2303+
22622304
/// Determine whether the given declaration is an acceptable lookup
22632305
/// result when searching from the given DeclContext.
22642306
static bool isAcceptableLookupResult(const DeclContext *dc,
@@ -2291,10 +2333,7 @@ static bool isAcceptableLookupResult(const DeclContext *dc,
22912333

22922334
// Check that there is some import in the originating context that
22932335
// makes this decl visible.
2294-
if (decl->getDeclContext()->getParentModule() != dc->getParentModule() &&
2295-
dc->getASTContext().LangOpts.hasFeature(
2296-
Feature::ExtensionImportVisibility) &&
2297-
!decl->findImport(dc))
2336+
if (missingExplicitImportForMemberDecl(dc, decl))
22982337
return false;
22992338
}
23002339

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6187,39 +6187,11 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
61876187

61886188
auto loc = nameLoc.isValid() ? nameLoc.getStartLoc() : ::getLoc(anchor);
61896189
auto accessLevel = Member->getFormalAccessScope().accessLevelForDiagnostics();
6190-
bool suppressDeclHereNote = false;
61916190
if (accessLevel == AccessLevel::Public &&
6192-
!Member->findImport(getDC())) {
6193-
auto definingModule = Member->getDeclContext()->getParentModule();
6194-
emitDiagnosticAt(loc, diag::candidate_from_missing_import,
6195-
Member->getDescriptiveKind(), Member->getName(),
6196-
definingModule->getName());
6197-
6198-
SourceLoc bestLoc =
6199-
getBestAddImportFixItLocation(Member, getDC()->getParentSourceFile());
6200-
if (bestLoc.isValid()) {
6201-
llvm::SmallString<64> importText;
6202-
6203-
// @_spi imports.
6204-
if (Member->isSPI()) {
6205-
auto spiGroups = Member->getSPIGroups();
6206-
if (!spiGroups.empty()) {
6207-
importText += "@_spi(";
6208-
importText += spiGroups[0].str();
6209-
importText += ") ";
6210-
}
6211-
}
6212-
6213-
importText += "import ";
6214-
importText += definingModule->getName().str();
6215-
importText += "\n";
6216-
emitDiagnosticAt(bestLoc, diag::candidate_add_import,
6217-
definingModule->getName())
6218-
.fixItInsert(bestLoc, importText);
6219-
}
6191+
diagnoseMissingImportForMember(Member, getDC(), loc))
6192+
return true;
62206193

6221-
suppressDeclHereNote = true;
6222-
} else if (auto *CD = dyn_cast<ConstructorDecl>(Member)) {
6194+
if (auto *CD = dyn_cast<ConstructorDecl>(Member)) {
62236195
emitDiagnosticAt(loc, diag::init_candidate_inaccessible,
62246196
CD->getResultInterfaceType(), accessLevel)
62256197
.highlight(nameLoc.getSourceRange());
@@ -6229,8 +6201,7 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
62296201
.highlight(nameLoc.getSourceRange());
62306202
}
62316203

6232-
if (!suppressDeclHereNote)
6233-
emitDiagnosticAt(Member, diag::decl_declared_here, Member);
6204+
emitDiagnosticAt(Member, diag::decl_declared_here, Member);
62346205
return true;
62356206
}
62366207

0 commit comments

Comments
 (0)