From 03dd5a2c26f24bb2c7965d15ddae20d7a557aa6b Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 10 Aug 2025 23:49:03 +0100 Subject: [PATCH 1/3] [Frontend] Split off `loadAccessNotesIfNeeded` This ought to be requestified, but for now let's split it into its own function. --- include/swift/Frontend/Frontend.h | 5 ++++ lib/Frontend/Frontend.cpp | 47 ++++++++++++++++++------------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/include/swift/Frontend/Frontend.h b/include/swift/Frontend/Frontend.h index 029e296ab5d38..e4275280fc743 100644 --- a/include/swift/Frontend/Frontend.h +++ b/include/swift/Frontend/Frontend.h @@ -781,6 +781,11 @@ class CompilerInstance { /// Parses and type-checks all input files. void performSema(); + /// Loads any access notes for the main module. + /// + /// FIXME: This should be requestified. + void loadAccessNotesIfNeeded(); + /// Parses and performs import resolution on all input files. /// /// This is similar to a parse-only invocation, but module imports will also diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index 5cadb6afe4fe1..5ee01c3d21e8f 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -1543,33 +1543,40 @@ void CompilerInstance::setMainModule(ModuleDecl *newMod) { Context->MainModule = newMod; } +void CompilerInstance::loadAccessNotesIfNeeded() { + if (Invocation.getFrontendOptions().AccessNotesPath.empty()) + return; + + auto *mainModule = getMainModule(); + + auto accessNotesPath = Invocation.getFrontendOptions().AccessNotesPath; + + auto bufferOrError = + swift::vfs::getFileOrSTDIN(getFileSystem(), accessNotesPath); + if (bufferOrError) { + int sourceID = SourceMgr.addNewSourceBuffer(std::move(bufferOrError.get())); + auto buffer = SourceMgr.getLLVMSourceMgr().getMemoryBuffer(sourceID); + + if (auto accessNotesFile = AccessNotesFile::load(*Context, buffer)) + mainModule->getAccessNotes() = *accessNotesFile; + } else { + Diagnostics.diagnose(SourceLoc(), diag::access_notes_file_io_error, + accessNotesPath, bufferOrError.getError().message()); + } +} + bool CompilerInstance::performParseAndResolveImportsOnly() { FrontendStatsTracer tracer(getStatsReporter(), "parse-and-resolve-imports"); - auto *mainModule = getMainModule(); + // NOTE: Do not add new logic to this function, use the request evaluator to + // lazily evaluate instead. Once the below computations are requestified we + // ought to be able to remove this function. // Load access notes. - if (!Invocation.getFrontendOptions().AccessNotesPath.empty()) { - auto accessNotesPath = Invocation.getFrontendOptions().AccessNotesPath; - - auto bufferOrError = - swift::vfs::getFileOrSTDIN(getFileSystem(), accessNotesPath); - if (bufferOrError) { - int sourceID = - SourceMgr.addNewSourceBuffer(std::move(bufferOrError.get())); - auto buffer = - SourceMgr.getLLVMSourceMgr().getMemoryBuffer(sourceID); - - if (auto accessNotesFile = AccessNotesFile::load(*Context, buffer)) - mainModule->getAccessNotes() = *accessNotesFile; - } - else { - Diagnostics.diagnose(SourceLoc(), diag::access_notes_file_io_error, - accessNotesPath, bufferOrError.getError().message()); - } - } + loadAccessNotesIfNeeded(); // Resolve imports for all the source files in the module. + auto *mainModule = getMainModule(); performImportResolution(mainModule); bindExtensions(*mainModule); From acf6375d46dd349a77e19da6bd64e9c21fded34b Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 10 Aug 2025 23:49:03 +0100 Subject: [PATCH 2/3] Introduce `BindExtensionsForIDEInspectionRequest` This allows us to lazily bind extensions after mutating the AST, ensuring we don't prematurely kick the building of lookup tables. --- include/swift/AST/TypeCheckRequests.h | 19 +++++++++++++++++++ include/swift/AST/TypeCheckerTypeIDZone.def | 4 ++++ lib/IDE/REPLCodeCompletion.cpp | 1 - lib/IDETool/IDEInspectionInstance.cpp | 4 ++-- lib/Parse/Parser.cpp | 13 ++++++++++++- lib/Sema/TypeChecker.cpp | 7 +++++++ test/IDE/pr83369.swift | 9 +++++++++ .../b4e591f09ce81b0.swift | 2 +- .../f1328ad669bd7cf8.swift | 2 +- 9 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 test/IDE/pr83369.swift rename validation-test/IDE/{crashers => crashers_fixed}/b4e591f09ce81b0.swift (56%) rename validation-test/IDE/{crashers => crashers_fixed}/f1328ad669bd7cf8.swift (53%) diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index e768a9a09e4df..ca631e3abd440 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -5389,6 +5389,25 @@ class DefaultIsolationInSourceFileRequest bool isCached() const { return true; } }; +/// A request that allows IDE inspection to lazily kick extension binding after +/// it has finished mutating the AST. This will eventually be subsumed when we +/// properly requestify extension binding. +class BindExtensionsForIDEInspectionRequest + : public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + evaluator::SideEffect evaluate(Evaluator &evaluator, ModuleDecl *M) const; + +public: + bool isCached() const { return true; } +}; + class ModuleHasTypeCheckerPerformanceHacksEnabledRequest : public SimpleRequest(const SourceFile *), Cached, NoLocationInfo) +SWIFT_REQUEST(TypeChecker, BindExtensionsForIDEInspectionRequest, + evaluator::SideEffect(ModuleDecl *), + Cached, NoLocationInfo) + SWIFT_REQUEST(TypeChecker, ModuleHasTypeCheckerPerformanceHacksEnabledRequest, bool(const ModuleDecl *), Cached, NoLocationInfo) diff --git a/lib/IDE/REPLCodeCompletion.cpp b/lib/IDE/REPLCodeCompletion.cpp index 2b809974ee0c3..d8243b3ba72b0 100644 --- a/lib/IDE/REPLCodeCompletion.cpp +++ b/lib/IDE/REPLCodeCompletion.cpp @@ -256,7 +256,6 @@ doCodeCompletion(SourceFile &SF, StringRef EnteredCode, unsigned *BufferID, auto &newSF = newModule->getMainSourceFile(); performImportResolution(newSF); - bindExtensions(*newModule); performIDEInspectionSecondPass(newSF, *CompletionCallbacksFactory); diff --git a/lib/IDETool/IDEInspectionInstance.cpp b/lib/IDETool/IDEInspectionInstance.cpp index 0d4ef37aa7265..22f709354c820 100644 --- a/lib/IDETool/IDEInspectionInstance.cpp +++ b/lib/IDETool/IDEInspectionInstance.cpp @@ -429,7 +429,6 @@ bool IDEInspectionInstance::performCachedOperationIfPossible( // re-use imported modules. auto *newSF = &newM->getMainSourceFile(); performImportResolution(*newSF); - bindExtensions(*newM); traceDC = newM; #ifndef NDEBUG @@ -518,7 +517,8 @@ void IDEInspectionInstance::performNewOperation( CI->getASTContext().CancellationFlag = CancellationFlag; registerIDERequestFunctions(CI->getASTContext().evaluator); - CI->performParseAndResolveImportsOnly(); + CI->loadAccessNotesIfNeeded(); + performImportResolution(CI->getMainModule()); bool DidFindIDEInspectionTarget = CI->getIDEInspectionFile() ->getDelayedParserState() diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 8eeeacb1895dc..a0d6f986bacb8 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -162,6 +162,7 @@ void Parser::performIDEInspectionSecondPassImpl( // Clear any ASTScopes that were expanded. SF.clearScope(); + // FIXME: We shouldn't be mutating the AST after-the-fact like this. switch (info.Kind) { case IDEInspectionDelayedDeclKind::TopLevelCodeDecl: { // Re-enter the top-level code decl context. @@ -209,7 +210,17 @@ void Parser::performIDEInspectionSecondPassImpl( assert(!State->hasIDEInspectionDelayedDeclState() && "Second pass should not set any code completion info"); - DoneParsingCallback->doneParsing(DC->getParentSourceFile()); + auto *SF = DC->getParentSourceFile(); + + // Bind extensions if needed. This needs to be done here since we may have + // mutated the AST above. + { + auto *M = SF->getParentModule(); + BindExtensionsForIDEInspectionRequest req(M); + evaluateOrDefault(Context.evaluator, req, {}); + } + + DoneParsingCallback->doneParsing(SF); State->restoreIDEInspectionDelayedDeclState(info); } diff --git a/lib/Sema/TypeChecker.cpp b/lib/Sema/TypeChecker.cpp index ce71f941035e3..10f19efe84e18 100644 --- a/lib/Sema/TypeChecker.cpp +++ b/lib/Sema/TypeChecker.cpp @@ -793,3 +793,10 @@ std::pair EvaluateIfConditionRequest::evaluate( llvm_unreachable("Must not be used in C++-only build"); #endif } + +evaluator::SideEffect +BindExtensionsForIDEInspectionRequest::evaluate(Evaluator &evaluator, + ModuleDecl *M) const { + bindExtensions(*M); + return {}; +} diff --git a/test/IDE/pr83369.swift b/test/IDE/pr83369.swift new file mode 100644 index 0000000000000..6fcd79c8ed64c --- /dev/null +++ b/test/IDE/pr83369.swift @@ -0,0 +1,9 @@ +// RUN: %batch-code-completion -module-name main + +extension Int {} + +// Make sure we can resolve P here. +protocol P where X: main.#^COMPLETE^# { + associatedtype X +} +// COMPLETE: Decl[Protocol]/CurrModule: P[#P#]; name=P diff --git a/validation-test/IDE/crashers/b4e591f09ce81b0.swift b/validation-test/IDE/crashers_fixed/b4e591f09ce81b0.swift similarity index 56% rename from validation-test/IDE/crashers/b4e591f09ce81b0.swift rename to validation-test/IDE/crashers_fixed/b4e591f09ce81b0.swift index 4c2e86e5fa1ae..e41ae8ec3e8f0 100644 --- a/validation-test/IDE/crashers/b4e591f09ce81b0.swift +++ b/validation-test/IDE/crashers_fixed/b4e591f09ce81b0.swift @@ -1,5 +1,5 @@ // {"kind":"complete","original":"0b319c77","signature":"swift::ExtensionDecl::getExtendedNominal() const","signatureAssert":"Extension must have already been bound"} -// RUN: not --crash %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s +// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s extension a[ { #^^# } diff --git a/validation-test/IDE/crashers/f1328ad669bd7cf8.swift b/validation-test/IDE/crashers_fixed/f1328ad669bd7cf8.swift similarity index 53% rename from validation-test/IDE/crashers/f1328ad669bd7cf8.swift rename to validation-test/IDE/crashers_fixed/f1328ad669bd7cf8.swift index c359342cfe679..65b74766ca615 100644 --- a/validation-test/IDE/crashers/f1328ad669bd7cf8.swift +++ b/validation-test/IDE/crashers_fixed/f1328ad669bd7cf8.swift @@ -1,4 +1,4 @@ // {"kind":"complete","signature":"Extension must have already been bound","signatureAssert":"Extension must have already been bound"} -// RUN: not --crash %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s +// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s extension a [ { func b { #^COMPLETE^# From deecd46e43d17e94ca19c95aed5843c3452db38e Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 10 Aug 2025 23:49:03 +0100 Subject: [PATCH 3/3] [IDE] Remove extension binding logic from `typeCheckContextAt` Now that we correctly bind extensions after mutating the AST, this is no longer necessary. --- lib/IDE/CodeCompletion.cpp | 11 +++++---- lib/IDE/ConformingMethodList.cpp | 2 +- lib/IDE/ExprContextAnalysis.cpp | 39 -------------------------------- lib/IDE/ExprContextAnalysis.h | 5 ---- lib/IDE/TypeContextInfo.cpp | 2 +- 5 files changed, 8 insertions(+), 51 deletions(-) diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index 620fe0bd251e9..f4cdb547bba40 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -1485,12 +1485,12 @@ void CodeCompletionCallbacksImpl::typeCheckWithLookup( ASTNode Call = CallExpr::create( CurDeclContext->getASTContext(), AttrWithCompletion->getTypeExpr(), AttrWithCompletion->getArgs(), /*implicit=*/true); - typeCheckContextAt( + swift::typeCheckASTNodeAtLoc( TypeCheckASTNodeAtLocContext::node(CurDeclContext, Call), CompletionLoc); } } else { - typeCheckContextAt( + swift::typeCheckASTNodeAtLoc( TypeCheckASTNodeAtLocContext::declContext(CurDeclContext), CompletionLoc); } @@ -1551,8 +1551,9 @@ void CodeCompletionCallbacksImpl::postfixCompletion(SourceLoc CompletionLoc, llvm::SaveAndRestore CompletionCollector( Context.CompletionCallback, &Lookup); - typeCheckContextAt(TypeCheckASTNodeAtLocContext::node(CurDeclContext, AE), - CompletionLoc); + swift::typeCheckASTNodeAtLoc( + TypeCheckASTNodeAtLocContext::node(CurDeclContext, AE), + CompletionLoc); Lookup.collectResults(/*IsLabeledTrailingClosure=*/true, CompletionLoc, CurDeclContext, CompletionContext); } @@ -1711,7 +1712,7 @@ void CodeCompletionCallbacksImpl::doneParsing(SourceFile *SrcFile) { if (Kind != CompletionKind::TypeSimpleWithDot) { // Type member completion does not need a type-checked AST. - typeCheckContextAt( + swift::typeCheckASTNodeAtLoc( TypeCheckASTNodeAtLocContext::declContext(CurDeclContext), ParsedExpr ? ParsedExpr->getLoc() : CurDeclContext->getASTContext() diff --git a/lib/IDE/ConformingMethodList.cpp b/lib/IDE/ConformingMethodList.cpp index 7e95a5ad82f13..9664119736430 100644 --- a/lib/IDE/ConformingMethodList.cpp +++ b/lib/IDE/ConformingMethodList.cpp @@ -109,7 +109,7 @@ void ConformingMethodListCallbacks::doneParsing(SourceFile *SrcFile) { { llvm::SaveAndRestore CompletionCollector( Context.CompletionCallback, &TypeCheckCallback); - typeCheckContextAt( + swift::typeCheckASTNodeAtLoc( TypeCheckASTNodeAtLocContext::declContext(CurDeclContext), CCExpr->getLoc()); } diff --git a/lib/IDE/ExprContextAnalysis.cpp b/lib/IDE/ExprContextAnalysis.cpp index 458ff11db8891..a690c621a9faa 100644 --- a/lib/IDE/ExprContextAnalysis.cpp +++ b/lib/IDE/ExprContextAnalysis.cpp @@ -40,45 +40,6 @@ using namespace swift; using namespace ide; -//===----------------------------------------------------------------------===// -// typeCheckContextAt(DeclContext, SourceLoc) -//===----------------------------------------------------------------------===// - -void swift::ide::typeCheckContextAt(TypeCheckASTNodeAtLocContext TypeCheckCtx, - SourceLoc Loc) { - // Make sure the extension has been bound. - auto DC = TypeCheckCtx.getDeclContext(); - // Even if the extension is invalid (e.g. nested in a function or another - // type), we want to know the "intended nominal" of the extension so that - // we can know the type of 'Self'. - SmallVector extensions; - for (auto typeCtx = DC->getInnermostTypeContext(); typeCtx != nullptr; - typeCtx = typeCtx->getParent()->getInnermostTypeContext()) { - if (auto *ext = dyn_cast(typeCtx)) - extensions.push_back(ext); - } - while (!extensions.empty()) { - extensions.back()->computeExtendedNominal(); - extensions.pop_back(); - } - - // If the completion happens in the inheritance clause of the extension, - // 'DC' is the parent of the extension. We need to iterate the top level - // decls to find it. In theory, we don't need the extended nominal in the - // inheritance clause, but ASTScope lookup requires that. We don't care - // unless 'DC' is not 'SourceFile' because non-toplevel extensions are - // 'canNeverBeBound()' anyway. - if (auto *SF = dyn_cast(DC)) { - auto &SM = DC->getASTContext().SourceMgr; - for (auto *decl : SF->getTopLevelDecls()) - if (auto *ext = dyn_cast(decl)) - if (SM.rangeContainsTokenLoc(ext->getSourceRange(), Loc)) - ext->computeExtendedNominal(); - } - - swift::typeCheckASTNodeAtLoc(TypeCheckCtx, Loc); -} - //===----------------------------------------------------------------------===// // findParsedExpr(DeclContext, Expr) //===----------------------------------------------------------------------===// diff --git a/lib/IDE/ExprContextAnalysis.h b/lib/IDE/ExprContextAnalysis.h index 17f31412b6999..8e344283c388d 100644 --- a/lib/IDE/ExprContextAnalysis.h +++ b/lib/IDE/ExprContextAnalysis.h @@ -28,11 +28,6 @@ class ValueDecl; namespace ide { enum class SemanticContextKind : uint8_t; -/// Type check parent contexts of the given decl context, and the body of the -/// given context until \c Loc if the context is a function body. -void typeCheckContextAt(TypeCheckASTNodeAtLocContext TypeCheckCtx, - SourceLoc Loc); - /// From \p DC, find and returns the outer most expression which source range is /// exact the same as \p TargetRange. Returns \c nullptr if not found. Expr *findParsedExpr(const DeclContext *DC, SourceRange TargetRange); diff --git a/lib/IDE/TypeContextInfo.cpp b/lib/IDE/TypeContextInfo.cpp index bd587f2005c20..ce42490087338 100644 --- a/lib/IDE/TypeContextInfo.cpp +++ b/lib/IDE/TypeContextInfo.cpp @@ -119,7 +119,7 @@ void ContextInfoCallbacks::doneParsing(SourceFile *SrcFile) { { llvm::SaveAndRestore CompletionCollector( Context.CompletionCallback, &TypeCheckCallback); - typeCheckContextAt( + swift::typeCheckASTNodeAtLoc( TypeCheckASTNodeAtLocContext::declContext(CurDeclContext), ParsedExpr->getLoc()); }