Skip to content

Commit 8e6c996

Browse files
committed
[AST] Do not copy SearchPathOptions in updateNonUserModule
`updateNonUserModule` was accidentally copying `SearchPathOptions`. Take a reference to it instead. Also, since `addFile` is actually called many times (once for every submodule, of which there are many), change `isNonUserModule` to a request so that it's only calculated when needed. Resolves rdar://107155587.
1 parent 3f6bfca commit 8e6c996

File tree

7 files changed

+79
-54
lines changed

7 files changed

+79
-54
lines changed

include/swift/AST/Decl.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
630630
HasAnyUnavailableValues : 1
631631
);
632632

633-
SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1,
633+
SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1,
634634
/// If the module is compiled as static library.
635635
StaticLibrary : 1,
636636

@@ -680,11 +680,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
680680

681681
/// If the map from @objc provided name to top level swift::Decl in this
682682
/// module is populated
683-
ObjCNameLookupCachePopulated : 1,
684-
685-
/// Whether the module is contained in the SDK or stdlib, or its a system
686-
/// module and no SDK was specified.
687-
IsNonUserModule : 1
683+
ObjCNameLookupCachePopulated : 1
688684
);
689685

690686
SWIFT_INLINE_BITFIELD(PrecedenceGroupDecl, Decl, 1+2,

include/swift/AST/Module.h

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -661,15 +661,9 @@ class ModuleDecl
661661
void setIsSystemModule(bool flag = true);
662662

663663
/// \returns true if this module is part of the stdlib or contained within
664-
/// the SDK. If no SDK was specified, also falls back to whether the module
665-
/// was specified as a system module (ie. it's on the system search path).
666-
bool isNonUserModule() const { return Bits.ModuleDecl.IsNonUserModule; }
667-
668-
private:
669-
/// Update whether this module is a non-user module, see \c isNonUserModule.
670-
/// \p newUnit is the added unit that caused this update, or \c nullptr if
671-
/// the update wasn't caused by adding a new unit.
672-
void updateNonUserModule(FileUnit *newUnit);
664+
/// the SDK. If no SDK was specified, falls back to whether the module was
665+
/// specified as a system module (ie. it's on the system search path).
666+
bool isNonUserModule() const;
673667

674668
public:
675669
/// Returns true if the module was rebuilt from a module interface instead

include/swift/AST/TypeCheckRequests.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4172,6 +4172,25 @@ void simple_display(llvm::raw_ostream &out, const TypeRepr *TyR);
41724172
void simple_display(llvm::raw_ostream &out, ImplicitMemberAction action);
41734173
void simple_display(llvm::raw_ostream &out, ResultBuilderBodyPreCheck pck);
41744174

4175+
/// Computes whether a module is part of the stdlib or contained within the
4176+
/// SDK. If no SDK was specified, falls back to whether the module was
4177+
/// specified as a system module (ie. it's on the system search path).
4178+
class IsNonUserModuleRequest
4179+
: public SimpleRequest<IsNonUserModuleRequest,
4180+
bool(ModuleDecl *),
4181+
RequestFlags::Cached> {
4182+
public:
4183+
using SimpleRequest::SimpleRequest;
4184+
4185+
private:
4186+
friend SimpleRequest;
4187+
4188+
bool evaluate(Evaluator &evaluator, ModuleDecl *mod) const;
4189+
4190+
public:
4191+
bool isCached() const { return true; }
4192+
};
4193+
41754194
#define SWIFT_TYPEID_ZONE TypeChecker
41764195
#define SWIFT_TYPEID_HEADER "swift/AST/TypeCheckerTypeIDZone.def"
41774196
#include "swift/Basic/DefineTypeIDZone.h"

include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,3 +466,6 @@ SWIFT_REQUEST(TypeChecker, GetRuntimeDiscoverableAttributes,
466466
SWIFT_REQUEST(TypeChecker, LocalDiscriminatorsRequest,
467467
unsigned(DeclContext *),
468468
Cached, NoLocationInfo)
469+
SWIFT_REQUEST(TypeChecker, IsNonUserModuleRequest,
470+
bool(ModuleDecl *),
471+
Cached, NoLocationInfo)

lib/AST/Module.cpp

Lines changed: 48 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -605,49 +605,19 @@ ModuleDecl::ModuleDecl(Identifier name, ASTContext &ctx,
605605
Bits.ModuleDecl.HasHermeticSealAtLink = 0;
606606
Bits.ModuleDecl.IsConcurrencyChecked = 0;
607607
Bits.ModuleDecl.ObjCNameLookupCachePopulated = 0;
608-
Bits.ModuleDecl.IsNonUserModule = 0;
609608
}
610609

611610
void ModuleDecl::setIsSystemModule(bool flag) {
612611
Bits.ModuleDecl.IsSystemModule = flag;
613-
updateNonUserModule(/*newFile=*/nullptr);
614612
}
615613

616-
static bool prefixMatches(StringRef prefix, StringRef path) {
617-
auto i = llvm::sys::path::begin(prefix), e = llvm::sys::path::end(prefix);
618-
for (auto pi = llvm::sys::path::begin(path), pe = llvm::sys::path::end(path);
619-
i != e && pi != pe; ++i, ++pi) {
620-
if (*i != *pi)
621-
return false;
622-
}
623-
return i == e;
624-
}
625-
626-
void ModuleDecl::updateNonUserModule(FileUnit *newUnit) {
627-
if (isNonUserModule())
628-
return;
629-
630-
SearchPathOptions searchPathOpts = getASTContext().SearchPathOpts;
631-
StringRef sdkPath = searchPathOpts.getSDKPath();
632-
633-
if (isStdlibModule() || (sdkPath.empty() && isSystemModule())) {
634-
Bits.ModuleDecl.IsNonUserModule = true;
635-
return;
636-
}
614+
bool ModuleDecl::isNonUserModule() const {
615+
// For clang submodules, retrieve their top level module (submodules have no
616+
// source path, so we'd always return false for them).
617+
ModuleDecl *mod = const_cast<ModuleDecl *>(this)->getTopLevelModule();
637618

638-
// If we loaded a serialized module, check if it was compiler adjacent or in
639-
// the SDK.
640-
641-
auto *LF = dyn_cast_or_null<LoadedFile>(newUnit);
642-
if (!LF)
643-
return;
644-
645-
StringRef runtimePath = searchPathOpts.RuntimeResourcePath;
646-
StringRef modulePath = LF->getSourceFilename();
647-
if ((!runtimePath.empty() && prefixMatches(runtimePath, modulePath)) ||
648-
(!sdkPath.empty() && prefixMatches(sdkPath, modulePath))) {
649-
Bits.ModuleDecl.IsNonUserModule = true;
650-
}
619+
auto &evaluator = getASTContext().evaluator;
620+
return evaluateOrDefault(evaluator, IsNonUserModuleRequest{mod}, false);
651621
}
652622

653623
ImplicitImportList ModuleDecl::getImplicitImports() const {
@@ -669,8 +639,6 @@ void ModuleDecl::addFile(FileUnit &newFile) {
669639
cast<SourceFile>(newFile).Kind == SourceFileKind::SIL);
670640
Files.push_back(&newFile);
671641
clearLookupCache();
672-
673-
updateNonUserModule(&newFile);
674642
}
675643

676644
void ModuleDecl::addAuxiliaryFile(SourceFile &sourceFile) {
@@ -4133,3 +4101,45 @@ const UnifiedStatsReporter::TraceFormatter*
41334101
FrontendStatsTracer::getTraceFormatter<const SourceFile *>() {
41344102
return &TF;
41354103
}
4104+
4105+
static bool prefixMatches(StringRef prefix, StringRef path) {
4106+
auto prefixIt = llvm::sys::path::begin(prefix),
4107+
prefixEnd = llvm::sys::path::end(prefix);
4108+
for (auto pathIt = llvm::sys::path::begin(path),
4109+
pathEnd = llvm::sys::path::end(path);
4110+
prefixIt != prefixEnd && pathIt != pathEnd; ++prefixIt, ++pathIt) {
4111+
if (*prefixIt != *pathIt)
4112+
return false;
4113+
}
4114+
return prefixIt == prefixEnd;
4115+
}
4116+
4117+
bool IsNonUserModuleRequest::evaluate(Evaluator &evaluator, ModuleDecl *mod) const {
4118+
// stdlib is non-user by definition
4119+
if (mod->isStdlibModule())
4120+
return true;
4121+
4122+
// If there's no SDK path, fallback to checking whether the module was
4123+
// in the system search path or a clang system module
4124+
SearchPathOptions &searchPathOpts = mod->getASTContext().SearchPathOpts;
4125+
StringRef sdkPath = searchPathOpts.getSDKPath();
4126+
if (sdkPath.empty() && mod->isSystemModule())
4127+
return true;
4128+
4129+
// Some temporary module's get created with no module name and they have no
4130+
// files. Avoid running `getFiles` on them (which will assert if there
4131+
// aren't any).
4132+
if (!mod->hasName() || mod->getFiles().empty())
4133+
return false;
4134+
4135+
auto *LF = dyn_cast_or_null<LoadedFile>(mod->getFiles().front());
4136+
if (!LF)
4137+
return false;
4138+
4139+
StringRef modulePath = LF->getSourceFilename();
4140+
if (modulePath.empty())
4141+
return false;
4142+
4143+
StringRef runtimePath = searchPathOpts.RuntimeResourcePath;
4144+
return (!runtimePath.empty() && prefixMatches(runtimePath, modulePath)) || (!sdkPath.empty() && prefixMatches(sdkPath, modulePath));
4145+
}

lib/IDE/CompletionLookup.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ void CompletionLookup::addSubModuleNames(
291291
CodeCompletionResultBuilder Builder(
292292
Sink, CodeCompletionResultKind::Declaration, SemanticContextKind::None);
293293
auto MD = ModuleDecl::create(Ctx.getIdentifier(Pair.first), Ctx);
294+
MD->setFailedToLoad();
294295
Builder.setAssociatedDecl(MD);
295296
Builder.addBaseName(MD->getNameStr());
296297
Builder.addTypeAnnotation("Module");
@@ -365,6 +366,8 @@ void CompletionLookup::addImportModuleNames() {
365366
continue;
366367

367368
auto MD = ModuleDecl::create(ModuleName, Ctx);
369+
MD->setFailedToLoad();
370+
368371
Optional<ContextualNotRecommendedReason> Reason = None;
369372

370373
// Imported modules are not recommended.

test/IDE/complete_diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ import MyModule
7777
import #^IMPORT^#;
7878
// IMPORT-DAG: Decl[Module]/None/NotRecommended: MyModule[#Module#]; name=MyModule; diagnostics=warning:module 'MyModule' is already imported{{$}}
7979
// IMPORT-DAG: Decl[Module]/None/NotRecommended: OtherModule[#Module#]; name=OtherModule; diagnostics=note:module 'OtherModule' is already imported via another module import{{$}}
80-
// IMPORT-DAG: Decl[Module]/None/NotRecommended: Swift[#Module#]; name=Swift; diagnostics=warning:module 'Swift' is already imported{{$}}
80+
// IMPORT-DAG: Decl[Module]/None/NotRecommended/IsSystem: Swift[#Module#]; name=Swift; diagnostics=warning:module 'Swift' is already imported{{$}}
8181

8282
func test(foo: Foo) {
8383
foo.#^MEMBER^#

0 commit comments

Comments
 (0)