Skip to content

[CodeCompletion] Replace main module instead of file #32147

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
Jun 3, 2020
Merged

[CodeCompletion] Replace main module instead of file #32147

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
19 changes: 19 additions & 0 deletions include/swift/AST/TypeCheckRequests.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2479,6 +2479,25 @@ class PrimarySourceFilesRequest
bool isCached() const { return true; }
};

/// Retrieve the file being used for code completion in the main module.
// FIXME: This isn't really a type-checking request, if we ever split off a
// zone for more basic AST requests, this should be moved there.
class CodeCompletionFileRequest
: public SimpleRequest<CodeCompletionFileRequest,
SourceFile *(ModuleDecl *), RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

SourceFile *evaluate(Evaluator &evaluator, ModuleDecl *mod) const;

public:
// Cached.
bool isCached() const { return true; }
};

// Allow AnyValue to compare two Type values, even though Type doesn't
// support ==.
template<>
Expand Down
2 changes: 2 additions & 0 deletions include/swift/AST/TypeCheckerTypeIDZone.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ SWIFT_REQUEST(TypeChecker, CheckRedeclarationRequest,
Uncached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, ClassAncestryFlagsRequest,
AncestryFlags(ClassDecl *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, CodeCompletionFileRequest,
SourceFile *(ModuleDecl *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, CompareDeclSpecializationRequest,
bool (DeclContext *, ValueDecl *, ValueDecl *, bool), Cached,
NoLocationInfo)
Expand Down
15 changes: 6 additions & 9 deletions include/swift/Frontend/Frontend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,9 +451,6 @@ class CompilerInstance {
/// considered primaries.
llvm::SetVector<unsigned> PrimaryBufferIDs;

/// The file that has been registered for code completion.
NullablePtr<SourceFile> CodeCompletionFile;

/// Return whether there is an entry in PrimaryInputs for buffer \p BufID.
bool isPrimaryInput(unsigned BufID) const {
return PrimaryBufferIDs.count(BufID) != 0;
Expand Down Expand Up @@ -509,8 +506,13 @@ class CompilerInstance {

UnifiedStatsReporter *getStatsReporter() const { return Stats.get(); }

/// Retrieve the main module containing the files being compiled.
ModuleDecl *getMainModule() const;

/// Replace the current main module with a new one. This is used for top-level
/// cached code completion.
void setMainModule(ModuleDecl *newMod);

MemoryBufferSerializedModuleLoader *
getMemoryBufferSerializedModuleLoader() const {
return MemoryBufferLoader;
Expand Down Expand Up @@ -557,12 +559,7 @@ class CompilerInstance {

/// If a code completion buffer has been set, returns the corresponding source
/// file.
NullablePtr<SourceFile> getCodeCompletionFile() { return CodeCompletionFile; }

/// Set a new file that we're performing code completion on.
void setCodeCompletionFile(SourceFile *file) {
CodeCompletionFile = file;
}
SourceFile *getCodeCompletionFile() const;

private:
/// Set up the file system by loading and validating all VFS overlay YAML
Expand Down
1 change: 0 additions & 1 deletion include/swift/IDE/CompletionInstance.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ class CompletionInstance {
std::mutex mtx;

std::unique_ptr<CompilerInstance> CachedCI;
ModuleDecl *CurrentModule = nullptr;
llvm::hash_code CachedArgHash;
llvm::sys::TimePoint<> DependencyCheckedTimestamp;
llvm::StringMap<llvm::hash_code> InMemoryDependencyHash;
Expand Down
14 changes: 14 additions & 0 deletions lib/AST/Module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,20 @@ ArrayRef<SourceFile *> ModuleDecl::getPrimarySourceFiles() const {
return evaluateOrDefault(eval, PrimarySourceFilesRequest{mutableThis}, {});
}

SourceFile *CodeCompletionFileRequest::evaluate(Evaluator &evaluator,
ModuleDecl *mod) const {
const auto &SM = mod->getASTContext().SourceMgr;
assert(mod->isMainModule() && "Can only do completion in the main module");
assert(SM.hasCodeCompletionBuffer() && "Not performing code completion?");

for (auto *file : mod->getFiles()) {
auto *SF = dyn_cast<SourceFile>(file);
if (SF && SF->getBufferID() == SM.getCodeCompletionBufferID())
return SF;
}
llvm_unreachable("Couldn't find the completion file?");
}

#define FORWARD(name, args) \
for (const FileUnit *file : getFiles()) \
file->name args;
Expand Down
18 changes: 12 additions & 6 deletions lib/Frontend/Frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,12 @@ Optional<unsigned> CompilerInstance::setUpCodeCompletionBuffer() {
return codeCompletionBufferID;
}

SourceFile *CompilerInstance::getCodeCompletionFile() const {
auto *mod = getMainModule();
auto &eval = mod->getASTContext().evaluator;
return evaluateOrDefault(eval, CodeCompletionFileRequest{mod}, nullptr);
}

static bool shouldTreatSingleInputAsMain(InputFileKind inputKind) {
switch (inputKind) {
case InputFileKind::Swift:
Expand Down Expand Up @@ -707,6 +713,12 @@ ModuleDecl *CompilerInstance::getMainModule() const {
return MainModule;
}

void CompilerInstance::setMainModule(ModuleDecl *newMod) {
assert(newMod->isMainModule());
MainModule = newMod;
Context->LoadedModules[newMod->getName()] = newMod;
}

void CompilerInstance::performParseOnly(bool EvaluateConditionals,
bool CanDelayBodies) {
const InputFileKind Kind = Invocation.getInputKind();
Expand Down Expand Up @@ -899,12 +911,6 @@ SourceFile *CompilerInstance::createSourceFileForMainModule(
if (isPrimary) {
inputFile->enableInterfaceHash();
}

if (bufferID == SourceMgr.getCodeCompletionBufferID()) {
assert(!CodeCompletionFile && "Multiple code completion files?");
CodeCompletionFile = inputFile;
}

return inputFile;
}

Expand Down
33 changes: 14 additions & 19 deletions lib/IDE/CompletionInstance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,10 @@ static DeclContext *getEquivalentDeclContextFromSourceFile(DeclContext *DC,
/// returns \c true. Returns \c true if any callback call returns \c true, \c
/// false otherwise.
static bool
forEachDependencyUntilTrue(CompilerInstance &CI, ModuleDecl *CurrentModule,
unsigned excludeBufferID,
forEachDependencyUntilTrue(CompilerInstance &CI, unsigned excludeBufferID,
llvm::function_ref<bool(StringRef)> callback) {
// Check files in the current module.
for (FileUnit *file : CurrentModule->getFiles()) {
for (FileUnit *file : CI.getMainModule()->getFiles()) {
StringRef filename;
if (auto SF = dyn_cast<SourceFile>(file)) {
if (SF->getBufferID() == excludeBufferID)
Expand Down Expand Up @@ -203,12 +202,11 @@ forEachDependencyUntilTrue(CompilerInstance &CI, ModuleDecl *CurrentModule,

/// Collect hash codes of the dependencies into \c Map.
static void cacheDependencyHashIfNeeded(CompilerInstance &CI,
ModuleDecl *CurrentModule,
unsigned excludeBufferID,
llvm::StringMap<llvm::hash_code> &Map) {
auto &FS = CI.getFileSystem();
forEachDependencyUntilTrue(
CI, CurrentModule, excludeBufferID, [&](StringRef filename) {
CI, excludeBufferID, [&](StringRef filename) {
if (Map.count(filename))
return false;

Expand All @@ -229,12 +227,12 @@ static void cacheDependencyHashIfNeeded(CompilerInstance &CI,

/// Check if any dependent files are modified since \p timestamp.
static bool areAnyDependentFilesInvalidated(
CompilerInstance &CI, ModuleDecl *CurrentModule, llvm::vfs::FileSystem &FS,
CompilerInstance &CI, llvm::vfs::FileSystem &FS,
unsigned excludeBufferID, llvm::sys::TimePoint<> timestamp,
llvm::StringMap<llvm::hash_code> &Map) {

return forEachDependencyUntilTrue(
CI, CurrentModule, excludeBufferID, [&](StringRef filePath) {
CI, excludeBufferID, [&](StringRef filePath) {
auto stat = FS.status(filePath);
if (!stat)
// Missing.
Expand Down Expand Up @@ -291,7 +289,7 @@ bool CompletionInstance::performCachedOperationIfPossible(
return false;

auto &CI = *CachedCI;
auto *oldSF = CI.getCodeCompletionFile().get();
auto *oldSF = CI.getCodeCompletionFile();

auto *oldState = oldSF->getDelayedParserState();
assert(oldState->hasCodeCompletionDelayedDeclState());
Expand All @@ -304,7 +302,7 @@ bool CompletionInstance::performCachedOperationIfPossible(

if (shouldCheckDependencies()) {
if (areAnyDependentFilesInvalidated(
CI, CurrentModule, *FileSystem, SM.getCodeCompletionBufferID(),
CI, *FileSystem, SM.getCodeCompletionBufferID(),
DependencyCheckedTimestamp, InMemoryDependencyHash))
return false;
DependencyCheckedTimestamp = std::chrono::system_clock::now();
Expand Down Expand Up @@ -441,22 +439,21 @@ bool CompletionInstance::performCachedOperationIfPossible(

// Create a new module and a source file using the current AST context.
auto &Ctx = oldM->getASTContext();
auto *newM =
ModuleDecl::create(oldM->getName(), Ctx, oldM->getImplicitImportInfo());
auto *newM = ModuleDecl::createMainModule(Ctx, oldM->getName(),
oldM->getImplicitImportInfo());
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis Jun 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

How often does this happen in practice? We are adding up to the cache each time we retrieve the completion file for a new main module, even though the old ones are thrown away.

Copy link
Contributor Author

@hamishknight hamishknight Jun 12, 2020

Choose a reason for hiding this comment

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

We only perform a limited number of cached completions before throwing away the CompilerInstance (and therefore both the ASTContext and evaluator cache), so I don't think this should be an issue in practice. Note this doesn't just apply to the evaluator's cache, there are also a bunch of other allocations that we make per module and file (including the AST nodes themselves) that won't be deallocated until the ASTContext is torn down.

Eventually we'd like code completion to take full advantage of the request evaluator by having it invalidate certain parsing requests, which would invalidate any dependant requests and ensure we only need to re-compute the minimal amount depending on what was changed. Once we get to this point, we'll hopefully be able to tie memory lifetimes such that request invalidation causes the deallocation of any state previously computed.

Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis Jun 12, 2020

Choose a reason for hiding this comment

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

Very informative, thank you.

auto *newSF =
new (Ctx) SourceFile(*newM, SourceFileKind::Main, newBufferID);
newM->addFile(*newSF);
newSF->enableInterfaceHash();

// Tell the compiler instance we've replaced the code completion file.
CI.setCodeCompletionFile(newSF);
// Tell the compiler instance we've replaced the main module.
CI.setMainModule(newM);

// Re-process the whole file (parsing will be lazily triggered). Still
// re-use imported modules.
performImportResolution(*newSF);
bindExtensions(*newM);

CurrentModule = newM;
traceDC = newM;
#ifndef NDEBUG
const auto *reparsedState = newSF->getDelayedParserState();
Expand All @@ -483,7 +480,7 @@ bool CompletionInstance::performCachedOperationIfPossible(
}

CachedReuseCount += 1;
cacheDependencyHashIfNeeded(CI, CurrentModule, SM.getCodeCompletionBufferID(),
cacheDependencyHashIfNeeded(CI, SM.getCodeCompletionBufferID(),
InMemoryDependencyHash);

return true;
Expand Down Expand Up @@ -536,8 +533,7 @@ bool CompletionInstance::performNewOperation(
CI.performParseAndResolveImportsOnly();

// If we didn't find a code completion token, bail.
auto completionFile = CI.getCodeCompletionFile();
auto *state = completionFile.get()->getDelayedParserState();
auto *state = CI.getCodeCompletionFile()->getDelayedParserState();
if (!state->hasCodeCompletionDelayedDeclState())
return true;

Expand All @@ -554,14 +550,13 @@ bool CompletionInstance::performNewOperation(
void CompletionInstance::cacheCompilerInstance(
std::unique_ptr<CompilerInstance> CI, llvm::hash_code ArgsHash) {
CachedCI = std::move(CI);
CurrentModule = CachedCI->getMainModule();
CachedArgHash = ArgsHash;
auto now = std::chrono::system_clock::now();
DependencyCheckedTimestamp = now;
CachedReuseCount = 0;
InMemoryDependencyHash.clear();
cacheDependencyHashIfNeeded(
*CachedCI, CurrentModule,
*CachedCI,
CachedCI->getASTContext().SourceMgr.getCodeCompletionBufferID(),
InMemoryDependencyHash);
}
Expand Down
4 changes: 2 additions & 2 deletions tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ static bool swiftCodeCompleteImpl(
SwiftConsumer.setContext(&CI.getASTContext(), &CI.getInvocation(),
&CompletionContext);

auto SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF.get(), *callbacksFactory);
auto *SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF, *callbacksFactory);
SwiftConsumer.clearContext();
});
}
Expand Down
4 changes: 2 additions & 2 deletions tools/SourceKit/lib/SwiftLang/SwiftConformingMethodList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ static bool swiftConformingMethodListImpl(
ide::makeConformingMethodListCallbacksFactory(ExpectedTypeNames,
Consumer));

auto SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF.get(), *callbacksFactory);
auto *SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF, *callbacksFactory);
});
}

Expand Down
4 changes: 2 additions & 2 deletions tools/SourceKit/lib/SwiftLang/SwiftTypeContextInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ static bool swiftTypeContextInfoImpl(
std::unique_ptr<CodeCompletionCallbacksFactory> callbacksFactory(
ide::makeTypeContextInfoCallbacksFactory(Consumer));

auto SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF.get(), *callbacksFactory);
auto *SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF, *callbacksFactory);
});
}

Expand Down
4 changes: 2 additions & 2 deletions tools/swift-ide-test/swift-ide-test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,8 +794,8 @@ static bool doCodeCompletionImpl(
CodeCompletionDiagnostics ? &PrintDiags : nullptr,
[&](CompilerInstance &CI, bool reusingASTContext) {
assert(!reusingASTContext && "reusing AST context without enabling it");
auto SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF.get(), *callbacksFactory);
auto *SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF, *callbacksFactory);
});
return isSuccess ? 0 : 1;
}
Expand Down