Skip to content

Commit 4ed6ffe

Browse files
tshortlitshortli
authored
Merge pull request #72974 from tshortli/extension-import-visibility-6.0
[6.0] Implement proper visibility rules for imported extensions
2 parents 0b87ee6 + 8124ee6 commit 4ed6ffe

29 files changed

+361
-33
lines changed

‎include/swift/AST/DiagnosticsSema.def‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ ERROR(init_candidate_inaccessible,none,
165165
"'%select{private|fileprivate|internal|package|@_spi|@_spi}1' protection level",
166166
(Type, AccessLevel))
167167

168+
ERROR(candidate_from_missing_import,none,
169+
"%0 %1 is not available due to missing import of defining module %2",
170+
(DescriptiveDeclKind, DeclName, DeclName))
171+
NOTE(candidate_add_import,none,
172+
"add import of module %0", (DeclName))
173+
168174
ERROR(cannot_pass_rvalue_mutating_subelement,none,
169175
"cannot use mutating member on immutable value: %0",
170176
(StringRef))

‎include/swift/AST/NameLookup.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,15 @@ SelfBounds getSelfBoundsFromWhereClause(
628628
/// given protocol or protocol extension.
629629
SelfBounds getSelfBoundsFromGenericSignature(const ExtensionDecl *extDecl);
630630

631+
/// Determine whether the given declaration is visible to name lookup when
632+
/// found from the given module scope context.
633+
///
634+
/// Note that this routine does not check ASTContext::isAccessControlDisabled();
635+
/// that's left for the caller.
636+
bool declIsVisibleToNameLookup(
637+
const ValueDecl *decl, const DeclContext *moduleScopeContext,
638+
NLOptions options);
639+
631640
namespace namelookup {
632641

633642
/// The bridge between the legacy UnqualifiedLookupFactory and the new ASTScope

‎include/swift/AST/SourceFile.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,11 @@ class SourceFile final : public FileUnit {
418418
ImportedUnderlyingModule = module;
419419
}
420420

421+
/// Finds the import declaration that effectively imports a given module in
422+
/// this source file.
423+
std::optional<AttributedImport<ImportedModule>>
424+
findImport(const ModuleDecl *mod) const;
425+
421426
/// Whether the given import has used @preconcurrency.
422427
bool hasImportUsedPreconcurrency(
423428
AttributedImport<ImportedModule> import) const;

‎include/swift/AST/TypeCheckRequests.h‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4845,6 +4845,27 @@ class ObjCRequirementMapRequest
48454845
bool isCached() const { return true; }
48464846
};
48474847

4848+
/// Finds the import declaration that effectively imports a given module in a
4849+
/// source file.
4850+
class ImportDeclRequest
4851+
: public SimpleRequest<ImportDeclRequest,
4852+
std::optional<AttributedImport<ImportedModule>>(
4853+
const SourceFile *sf, const ModuleDecl *mod),
4854+
RequestFlags::Cached> {
4855+
public:
4856+
using SimpleRequest::SimpleRequest;
4857+
4858+
private:
4859+
friend SimpleRequest;
4860+
4861+
std::optional<AttributedImport<ImportedModule>>
4862+
evaluate(Evaluator &evaluator, const SourceFile *sf,
4863+
const ModuleDecl *mod) const;
4864+
4865+
public:
4866+
bool isCached() const { return true; }
4867+
};
4868+
48484869
#define SWIFT_TYPEID_ZONE TypeChecker
48494870
#define SWIFT_TYPEID_HEADER "swift/AST/TypeCheckerTypeIDZone.def"
48504871
#include "swift/Basic/DefineTypeIDZone.h"

‎include/swift/AST/TypeCheckerTypeIDZone.def‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,4 +562,7 @@ SWIFT_REQUEST(TypeChecker, LocalTypeDeclsRequest,
562562
SWIFT_REQUEST(TypeChecker, ObjCRequirementMapRequest,
563563
ObjCRequirementMap(const ProtocolDecl *proto),
564564
Cached, NoLocationInfo)
565-
565+
SWIFT_REQUEST(TypeChecker, ImportDeclRequest,
566+
std::optional<AttributedImport<ImportedModule>>(
567+
const SourceFile *sf, const ModuleDecl *mod),
568+
Cached, NoLocationInfo)

‎include/swift/Basic/Features.def‎

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

367+
// Whether members of extensions respect the enclosing file's imports.
368+
EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE(ExtensionImportVisibility, true)
369+
367370
// Alias for IsolatedAny
368371
EXPERIMENTAL_FEATURE(IsolatedAny2, true)
369372

‎lib/AST/Decl.cpp‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3853,24 +3853,7 @@ ValueDecl::findImport(const DeclContext *fromDC) {
38533853
if (!fromSourceFile)
38543854
return std::nullopt;
38553855

3856-
// Look to see if the owning module was directly imported.
3857-
for (const auto &import : fromSourceFile->getImports()) {
3858-
if (import.module.importedModule == module)
3859-
return import;
3860-
}
3861-
3862-
// Now look for transitive imports.
3863-
auto &importCache = getASTContext().getImportCache();
3864-
for (const auto &import : fromSourceFile->getImports()) {
3865-
auto &importSet = importCache.getImportSet(import.module.importedModule);
3866-
for (const auto &transitive : importSet.getTransitiveImports()) {
3867-
if (transitive.importedModule == module) {
3868-
return import;
3869-
}
3870-
}
3871-
}
3872-
3873-
return std::nullopt;
3856+
return fromSourceFile->findImport(module);
38743857
}
38753858

38763859
bool ValueDecl::isProtocolRequirement() const {

‎lib/AST/FeatureSet.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,7 @@ static bool usesFeatureIsolatedAny(Decl *decl) {
682682
});
683683
}
684684

685+
UNINTERESTING_FEATURE(ExtensionImportVisibility)
685686
UNINTERESTING_FEATURE(IsolatedAny2)
686687

687688
UNINTERESTING_FEATURE(ObjCImplementation)

‎lib/AST/Module.cpp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2580,6 +2580,38 @@ SourceFile::setImports(ArrayRef<AttributedImport<ImportedModule>> imports) {
25802580
Imports = getASTContext().AllocateCopy(imports);
25812581
}
25822582

2583+
std::optional<AttributedImport<ImportedModule>>
2584+
SourceFile::findImport(const ModuleDecl *module) const {
2585+
return evaluateOrDefault(getASTContext().evaluator,
2586+
ImportDeclRequest{this, module}, std::nullopt);
2587+
}
2588+
2589+
std::optional<AttributedImport<ImportedModule>>
2590+
ImportDeclRequest::evaluate(Evaluator &evaluator, const SourceFile *sf,
2591+
const ModuleDecl *module) const {
2592+
auto &ctx = sf->getASTContext();
2593+
auto imports = sf->getImports();
2594+
2595+
// Look to see if the owning module was directly imported.
2596+
for (const auto &import : imports) {
2597+
if (import.module.importedModule == module)
2598+
return import;
2599+
}
2600+
2601+
// Now look for transitive imports.
2602+
auto &importCache = ctx.getImportCache();
2603+
for (const auto &import : imports) {
2604+
auto &importSet = importCache.getImportSet(import.module.importedModule);
2605+
for (const auto &transitive : importSet.getTransitiveImports()) {
2606+
if (transitive.importedModule == module) {
2607+
return import;
2608+
}
2609+
}
2610+
}
2611+
2612+
return std::nullopt;
2613+
}
2614+
25832615
bool SourceFile::hasImportUsedPreconcurrency(
25842616
AttributedImport<ImportedModule> import) const {
25852617
return PreconcurrencyImportsUsed.count(import) != 0;

‎lib/AST/ModuleNameLookup.cpp‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,22 @@ class LookupVisibleDecls : public ModuleNameLookup<LookupVisibleDecls> {
126126

127127
} // end anonymous namespace
128128

129+
bool swift::declIsVisibleToNameLookup(
130+
const ValueDecl *decl, const DeclContext *moduleScopeContext,
131+
NLOptions options) {
132+
// NL_IgnoreAccessControl only applies to the current module. If
133+
// it applies here, the declaration is visible.
134+
if ((options & NL_IgnoreAccessControl) &&
135+
moduleScopeContext &&
136+
moduleScopeContext->getParentModule() ==
137+
decl->getDeclContext()->getParentModule())
138+
return true;
139+
140+
bool includeUsableFromInline = options & NL_IncludeUsableFromInline;
141+
return decl->isAccessibleFrom(moduleScopeContext, false,
142+
includeUsableFromInline);
143+
}
144+
129145
template <typename LookupStrategy>
130146
void ModuleNameLookup<LookupStrategy>::lookupInModule(
131147
SmallVectorImpl<ValueDecl *> &decls,
@@ -151,7 +167,6 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(
151167

152168
const size_t initialCount = decls.size();
153169
size_t currentCount = decls.size();
154-
bool includeUsableFromInline = options & NL_IncludeUsableFromInline;
155170

156171
auto updateNewDecls = [&](const DeclContext *moduleScopeContext) {
157172
if (decls.size() == currentCount)
@@ -165,13 +180,7 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(
165180
if (resolutionKind == ResolutionKind::MacrosOnly && !isa<MacroDecl>(VD))
166181
return true;
167182
if (respectAccessControl &&
168-
// NL_IgnoreAccessControl applies only to the current module.
169-
!((options & NL_IgnoreAccessControl) &&
170-
moduleScopeContext &&
171-
moduleScopeContext->getParentModule() ==
172-
VD->getDeclContext()->getParentModule()) &&
173-
!VD->isAccessibleFrom(moduleScopeContext, false,
174-
includeUsableFromInline))
183+
!declIsVisibleToNameLookup(VD, moduleScopeContext, options))
175184
return true;
176185
return false;
177186
});

0 commit comments

Comments
 (0)