Skip to content

[SourceKit] Stop using isSystemModule to represent "non-user" modules #64367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 17, 2023
Merged

[SourceKit] Stop using isSystemModule to represent "non-user" modules #64367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
HasAnyUnavailableValues : 1
);

SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1,
SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1,
/// If the module is compiled as static library.
StaticLibrary : 1,

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

/// If the map from @objc provided name to top level swift::Decl in this
/// module is populated
ObjCNameLookupCachePopulated : 1
ObjCNameLookupCachePopulated : 1,

/// Whether the module is contained in the SDK or stdlib, or its a system
/// module and no SDK was specified.
IsNonUserModule : 1
);

SWIFT_INLINE_BITFIELD(PrecedenceGroupDecl, Decl, 1+2,
Expand Down
21 changes: 14 additions & 7 deletions include/swift/AST/Module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,10 +630,20 @@ class ModuleDecl
bool isSystemModule() const {
return Bits.ModuleDecl.IsSystemModule;
}
void setIsSystemModule(bool flag = true) {
Bits.ModuleDecl.IsSystemModule = flag;
}
void setIsSystemModule(bool flag = true);

/// \returns true if this module is part of the stdlib or contained within
/// the SDK. If no SDK was specified, also falls back to whether the module
/// was specified as a system module (ie. it's on the system search path).
bool isNonUserModule() const { return Bits.ModuleDecl.IsNonUserModule; }

private:
/// Update whether this module is a non-user module, see \c isNonUserModule.
/// \p newUnit is the added unit that caused this update, or \c nullptr if
/// the update wasn't caused by adding a new unit.
void updateNonUserModule(FileUnit *newUnit);

public:
/// Returns true if the module was rebuilt from a module interface instead
/// of being built from the full source.
bool isBuiltFromInterface() const {
Expand Down Expand Up @@ -946,10 +956,6 @@ class ModuleDecl
/// applicable.
StringRef getModuleLoadedFilename() const;

/// \returns true if this module is defined under the SDK path.
/// If no SDK path is defined, this always returns false.
bool isSDKModule() const;

/// \returns true if this module is the "swift" standard library module.
bool isStdlibModule() const;

Expand Down Expand Up @@ -1074,6 +1080,7 @@ class ModuleEntity {
std::string getFullName(bool useRealNameIfAliased = false) const;

bool isSystemModule() const;
bool isNonUserModule() const;
bool isBuiltinModule() const;
const ModuleDecl *getAsSwiftModule() const;
const clang::Module *getAsClangModule() const;
Expand Down
3 changes: 1 addition & 2 deletions lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5730,8 +5730,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
// Don't print qualifiers for types from the standard library.
if (M->isStdlibModule() ||
M->getRealName() == M->getASTContext().Id_ObjectiveC ||
M->isSystemModule() ||
isLLDBExpressionModule(M))
M->isNonUserModule() || isLLDBExpressionModule(M))
return false;

// Don't print qualifiers for imported types.
Expand Down
70 changes: 53 additions & 17 deletions lib/AST/Module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,49 @@ ModuleDecl::ModuleDecl(Identifier name, ASTContext &ctx,
Bits.ModuleDecl.HasHermeticSealAtLink = 0;
Bits.ModuleDecl.IsConcurrencyChecked = 0;
Bits.ModuleDecl.ObjCNameLookupCachePopulated = 0;
Bits.ModuleDecl.IsNonUserModule = 0;
}

void ModuleDecl::setIsSystemModule(bool flag) {
Bits.ModuleDecl.IsSystemModule = flag;
updateNonUserModule(/*newFile=*/nullptr);
}

static bool prefixMatches(StringRef prefix, StringRef path) {
auto i = llvm::sys::path::begin(prefix), e = llvm::sys::path::end(prefix);
for (auto pi = llvm::sys::path::begin(path), pe = llvm::sys::path::end(path);
i != e && pi != pe; ++i, ++pi) {
if (*i != *pi)
return false;
}
return i == e;
}

void ModuleDecl::updateNonUserModule(FileUnit *newUnit) {
if (isNonUserModule())
return;

SearchPathOptions searchPathOpts = getASTContext().SearchPathOpts;
StringRef sdkPath = searchPathOpts.getSDKPath();

if (isStdlibModule() || (sdkPath.empty() && isSystemModule())) {
Bits.ModuleDecl.IsNonUserModule = true;
return;
}

// If we loaded a serialized module, check if it was compiler adjacent or in
// the SDK.

auto *LF = dyn_cast_or_null<LoadedFile>(newUnit);
if (!LF)
return;

StringRef runtimePath = searchPathOpts.RuntimeResourcePath;
StringRef modulePath = LF->getSourceFilename();
if ((!runtimePath.empty() && prefixMatches(runtimePath, modulePath)) ||
(!sdkPath.empty() && prefixMatches(sdkPath, modulePath))) {
Bits.ModuleDecl.IsNonUserModule = true;
}
}

ImplicitImportList ModuleDecl::getImplicitImports() const {
Expand All @@ -632,6 +675,8 @@ void ModuleDecl::addFile(FileUnit &newFile) {
cast<SourceFile>(newFile).Kind == SourceFileKind::SIL);
Files.push_back(&newFile);
clearLookupCache();

updateNonUserModule(&newFile);
}

void ModuleDecl::addAuxiliaryFile(SourceFile &sourceFile) {
Expand Down Expand Up @@ -2284,23 +2329,6 @@ StringRef ModuleDecl::getModuleLoadedFilename() const {
return StringRef();
}

bool ModuleDecl::isSDKModule() const {
auto sdkPath = getASTContext().SearchPathOpts.getSDKPath();
if (sdkPath.empty())
return false;

auto modulePath = getModuleSourceFilename();
auto si = llvm::sys::path::begin(sdkPath),
se = llvm::sys::path::end(sdkPath);
for (auto mi = llvm::sys::path::begin(modulePath),
me = llvm::sys::path::end(modulePath);
si != se && mi != me; ++si, ++mi) {
if (*si != *mi)
return false;
}
return si == se;
}

bool ModuleDecl::isStdlibModule() const {
return !getParent() && getName() == getASTContext().StdlibModuleName;
}
Expand Down Expand Up @@ -4066,6 +4094,14 @@ bool ModuleEntity::isSystemModule() const {
return getClangModule(Mod)->IsSystem;
}

bool ModuleEntity::isNonUserModule() const {
assert(!Mod.isNull());
if (auto *SwiftMod = Mod.dyn_cast<const ModuleDecl *>())
return SwiftMod->isNonUserModule();
// TODO: Should handle clang modules as well
return getClangModule(Mod)->IsSystem;
}

bool ModuleEntity::isBuiltinModule() const {
assert(!Mod.isNull());
if (auto SwiftMod = Mod.dyn_cast<const ModuleDecl*>())
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/CodeCompletionResult.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ ContextFreeCodeCompletionResult::getCodeCompletionDeclKind(const Decl *D) {
}

bool ContextFreeCodeCompletionResult::getDeclIsSystem(const Decl *D) {
return D->getModuleContext()->isSystemModule();
return D->getModuleContext()->isNonUserModule();
}

// MARK: - CodeCompletionResult
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/CodeCompletionStringPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ void CodeCompletionStringPrinter::printTypeRef(Type T, const TypeDecl *TD,
Identifier Name,
PrintNameContext NameContext) {

NextChunkKind = TD->getModuleContext()->isSystemModule()
NextChunkKind = TD->getModuleContext()->isNonUserModule()
? ChunkKind::TypeIdSystem
: ChunkKind::TypeIdUser;

Expand Down
6 changes: 3 additions & 3 deletions lib/Index/Index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker {

void IndexSwiftASTWalker::visitDeclContext(DeclContext *Context) {
IsModuleFile = false;
isSystemModule = Context->getParentModule()->isSystemModule();
isSystemModule = Context->getParentModule()->isNonUserModule();
auto accessor = dyn_cast<AccessorDecl>(Context);
if (accessor)
ManuallyVisitedAccessorStack.push_back(accessor);
Expand Down Expand Up @@ -1101,7 +1101,7 @@ void IndexSwiftASTWalker::visitModule(ModuleDecl &Mod) {
walk(*SrcFile);
} else {
IsModuleFile = true;
isSystemModule = Mod.isSystemModule();
isSystemModule = Mod.isNonUserModule();
if (!handleSourceOrModuleFile(Mod))
return;
walk(Mod);
Expand Down Expand Up @@ -1178,7 +1178,7 @@ bool IndexSwiftASTWalker::visitImports(
ModuleName = Declaring->getNameStr();

if (!IdxConsumer.startDependency(ModuleName, Path, IsClangModule,
Mod->isSystemModule()))
Mod->isNonUserModule()))
return false;
if (!IsClangModule)
if (!visitImports(*Mod, Visited))
Expand Down
21 changes: 9 additions & 12 deletions lib/Index/IndexRecord.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,9 @@ static void addModuleDependencies(ArrayRef<ImportedModule> imports,
bool withoutUnitName = true;
if (FU->getKind() == FileUnitKind::ClangModule) {
auto clangModUnit = cast<ClangModuleUnit>(LFU);
bool shouldIndexModule = indexClangModules &&
(!clangModUnit->isSystemModule() || indexSystemModules);
bool shouldIndexModule =
indexClangModules &&
(!mod->isNonUserModule() || indexSystemModules);
withoutUnitName = !shouldIndexModule;
if (auto clangMod = clangModUnit->getUnderlyingClangModule()) {
moduleName = clangMod->getTopLevelModuleName();
Expand All @@ -662,10 +663,7 @@ static void addModuleDependencies(ArrayRef<ImportedModule> imports,
// We don't officially support binary swift modules, so normally
// the index data for user modules would get generated while
// building them.
bool isDistributedModule = mod->isSDKModule() ||
mod->getASTContext().SearchPathOpts.getSDKPath().empty();
if (mod->isSystemModule() && indexSystemModules &&
(isDistributedModule || mod->isStdlibModule()) &&
if (mod->isNonUserModule() && indexSystemModules &&
(!skipStdlib || !mod->isStdlibModule())) {
emitDataForSwiftSerializedModule(mod, indexStorePath,
indexClangModules,
Expand Down Expand Up @@ -697,7 +695,7 @@ static void addModuleDependencies(ArrayRef<ImportedModule> imports,
}
clang::index::writer::OpaqueModule opaqMod =
moduleNameScratch.createString(moduleName);
unitWriter.addASTFileDependency(*F, mod->isSystemModule(), opaqMod,
unitWriter.addASTFileDependency(*F, mod->isNonUserModule(), opaqMod,
withoutUnitName);

break;
Expand Down Expand Up @@ -833,7 +831,7 @@ emitDataForSwiftSerializedModule(ModuleDecl *module,
}

auto &fileMgr = clangCI.getFileManager();
bool isSystem = module->isSystemModule();
bool isSystem = module->isNonUserModule();
// FIXME: Get real values for the following.
StringRef swiftVersion;
StringRef sysrootPath = clangCI.getHeaderSearchOpts().Sysroot;
Expand All @@ -848,14 +846,13 @@ emitDataForSwiftSerializedModule(ModuleDecl *module,
targetTriple, sysrootPath, clangRemapper, getModuleInfoFromOpaqueModule);

auto FE = fileMgr.getFile(filename);
bool isSystemModule = module->isSystemModule();
for (auto &pair : records) {
std::string &recordFile = pair.first;
std::string &groupName = pair.second;
if (recordFile.empty())
continue;
clang::index::writer::OpaqueModule mod = &groupName;
unitWriter.addRecordFile(recordFile, *FE, isSystemModule, mod);
unitWriter.addRecordFile(recordFile, *FE, isSystem, mod);
}

SmallVector<ImportedModule, 8> imports;
Expand Down Expand Up @@ -887,7 +884,7 @@ recordSourceFileUnit(SourceFile *primarySourceFile, StringRef indexUnitToken,
DiagnosticEngine &diags) {
auto &fileMgr = clangCI.getFileManager();
auto *module = primarySourceFile->getParentModule();
bool isSystem = module->isSystemModule();
bool isSystem = module->isNonUserModule();
auto mainFile = fileMgr.getFile(primarySourceFile->getFilename());
auto clangRemapper = pathRemapper.asClangPathRemapper();
// FIXME: Get real values for the following.
Expand Down Expand Up @@ -920,7 +917,7 @@ recordSourceFileUnit(SourceFile *primarySourceFile, StringRef indexUnitToken,
auto file = fileMgr.getFile(filename);
unitWriter.addRecordFile(
recordFile, file ? *file : nullptr,
module->isSystemModule(), /*Module=*/nullptr);
module->isNonUserModule(), /*Module=*/nullptr);
});

std::string error;
Expand Down
6 changes: 3 additions & 3 deletions lib/Refactoring/Refactoring.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,15 +611,15 @@ class TextReplacementsRenamer : public Renamer {
};

static const ValueDecl *getRelatedSystemDecl(const ValueDecl *VD) {
if (VD->getModuleContext()->isSystemModule())
if (VD->getModuleContext()->isNonUserModule())
return VD;
for (auto *Req : VD->getSatisfiedProtocolRequirements()) {
if (Req->getModuleContext()->isSystemModule())
if (Req->getModuleContext()->isNonUserModule())
return Req;
}
for (auto Over = VD->getOverriddenDecl(); Over;
Over = Over->getOverriddenDecl()) {
if (Over->getModuleContext()->isSystemModule())
if (Over->getModuleContext()->isNonUserModule())
return Over;
}
return nullptr;
Expand Down
12 changes: 6 additions & 6 deletions test/Index/Store/ignore-system-clang-modules.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@

// Make a basic clang framework to import
//
// RUN: %empty-directory(%t/MySystemFramework.framework/Headers)
// RUN: %empty-directory(%t/MySystemFramework.framework/Modules)
// RUN: echo 'void someSystemFunc(int arg);' > %t/MySystemFramework.framework/Headers/MySystemFramework.h
// RUN: echo 'framework module MySystemFramework { umbrella header "MySystemFramework.h" export * }' > %t/MySystemFramework.framework/Modules/module.modulemap
// RUN: %empty-directory(%t/sdk/MySystemFramework.framework/Headers)
// RUN: %empty-directory(%t/sdk/MySystemFramework.framework/Modules)
// RUN: echo 'void someSystemFunc(int arg);' > %t/sdk/MySystemFramework.framework/Headers/MySystemFramework.h
// RUN: echo 'framework module MySystemFramework { umbrella header "MySystemFramework.h" export * }' > %t/sdk/MySystemFramework.framework/Modules/module.modulemap

import MySystemFramework
someSystemFunc(2)

// Index this file with and without ignoring system frameworks
//
// RUN: %target-swiftc_driver -index-store-path %t/idx1 -o %t/file.o -Fsystem %t -typecheck %s
// RUN: %target-swiftc_driver -index-store-path %t/idx2 -o %t/file.o -index-ignore-system-modules -Fsystem %t -typecheck %s
// RUN: %target-swiftc_driver -index-store-path %t/idx1 -o %t/file.o -Fsystem %t/sdk -sdk %t/sdk -typecheck %s
// RUN: %target-swiftc_driver -index-store-path %t/idx2 -o %t/file.o -index-ignore-system-modules -Fsystem %t/sdk -sdk %t/sdk -typecheck %s
// RUN: c-index-test core -print-unit %t/idx1 | %FileCheck --check-prefixes=ALLOWSYSTEM,BOTH %s
// RUN: c-index-test core -print-unit %t/idx2 | %FileCheck --check-prefixes=IGNORESYSTEM,BOTH %s

Expand Down
7 changes: 3 additions & 4 deletions test/Interop/Cxx/symbolic-imports/indexing-emit-libcxx-symbolic-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ import CxxStdlib
import CxxModule

// REMARK_NEW: remark: emitting symbolic interface at {{.*}}/interfaces/std-{{.*}}.pcm.symbolicswiftinterface{{$}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hyp I'm somewhat confused as to the cause of this failure. It seems like CxxShims is now getting a symbolic interface in this test, which changes the order of the output (though I couldn't reproduce it locally which is odd). I imagine we must now be indexing something where we weren't before and that ends up including CxxShims?

But IIRC what we really care about here is that CxxModule + std are emitted and that we don't emit them when the PCM isn't updated. So I've updated the test to reflect that.

For reference, the failing output was:

        16: <unknown>:0: remark: indexing system module at BUILD_DIR/lib/swift/macosx/Cxx.swiftmodule/x86_64-apple-macos.private.swiftinterface 
        17: <unknown>:0: remark: emitting symbolic interface at BUILD_DIR/test-macosx-x86_64/Interop/Cxx/symbolic-imports/Output/indexing-emit-libcxx-symbolic-module-interface.swift.tmp/store/interfaces/CxxShim-G46YUHGO4LYP.pcm.symbolicswiftinterface; skipping because it's up to date 
        18: <unknown>:0: remark: emitting symbolic interface at BUILD_DIR/test-macosx-x86_64/Interop/Cxx/symbolic-imports/Output/indexing-emit-libcxx-symbolic-module-interface.swift.tmp/store/interfaces/CxxShim-G46YUHGO4LYP.pcm.symbolicswiftinterface; skipping because it's up to date 
        19: <unknown>:0: remark: emitting symbolic interface at BUILD_DIR/test-macosx-x86_64/Interop/Cxx/symbolic-imports/Output/indexing-emit-libcxx-symbolic-module-interface.swift.tmp/store/interfaces/std-THR17V9MOTXM.pcm.symbolicswiftinterface 
        20: <unknown>:0: remark: emitting symbolic interface at BUILD_DIR/test-macosx-x86_64/Interop/Cxx/symbolic-imports/Output/indexing-emit-libcxx-symbolic-module-interface.swift.tmp/store/interfaces/std-THR17V9MOTXM.pcm.symbolicswiftinterface; skipping because it's up to date 
        21: <unknown>:0: remark: emitting symbolic interface at BUILD_DIR/test-macosx-x86_64/Interop/Cxx/symbolic-imports/Output/indexing-emit-libcxx-symbolic-module-interface.swift.tmp/store/interfaces/CxxModule-1OH37PNV2MYG1.pcm.symbolicswiftinterface 
next:35
        22: EOF

// REMARK_NEW-NEXT: remark: emitting symbolic interface at {{.*}}/interfaces/CxxModule-{{.*}}.pcm.symbolicswiftinterface{{$}}
// REMARK_NEW-NEXT: EOF
// REMARK_NEW: remark: emitting symbolic interface at {{.*}}/interfaces/CxxModule-{{.*}}.pcm.symbolicswiftinterface{{$}}

// REMARK_NO_UPDATE: remark: emitting symbolic interface at {{.*}}/interfaces/std-{{.*}}.pcm.symbolicswiftinterface; skipping because it's up to date{{$}}
// REMARK_NO_UPDATE: remark: emitting symbolic interface at {{.*}}/interfaces/CxxModule-{{.*}}.pcm.symbolicswiftinterface; skipping because it's up to date{{$}}
// REMARK_NO_UPDATE-NOT: remark: emitting symbolic interface at {{.*}}/interfaces/std-{{.*}}.pcm.symbolicswiftinterface{{$}}
// REMARK_NO_UPDATE-NOT: remark: emitting symbolic interface at {{.*}}/interfaces/CxxModule-{{.*}}.pcm.symbolicswiftinterface{{$}}

// FILES: CxxModule-{{.*}}.pcm.symbolicswiftinterface
// FILES: std-{{.*}}.pcm.symbolicswiftinterface
Expand Down
1 change: 0 additions & 1 deletion test/SourceKit/CursorInfo/cursor_stdlib.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func foo3(a: Float, b: Bool) {}
// CHECK-OVERLAY-NEXT: UInt
// CHECK-OVERLAY-NEXT: $sSuD
// CHECK-OVERLAY-NEXT: Foundation
// CHECK-OVERLAY-NEXT: SYSTEM
// CHECK-OVERLAY-NEXT: <Declaration>let NSUTF8StringEncoding: <Type usr="s:Su">UInt</Type></Declaration>

// RUN: %sourcekitd-test -req=cursor -pos=5:13 %s -- %s -target %target-triple %clang-importer-sdk-nosource -I %t | %FileCheck -check-prefix=CHECK-ITERATOR %s
Expand Down
Loading