Skip to content

Commit b204f2f

Browse files
ahoppenahoppen
committed
[SourceKit] Make sure we reuse ASTContext in function bodies for solver-based cursor info
The main problem that prevented us from reusing the ASTContext was that we weren’t remapping the `LocToResolve` in the temporary buffer that only contains the re-parsed function back to the original buffer. Thus `NodeFinder` couldn’t find the node that we want to get cursor info for. Getting AST reuse to work for top-level items is harder because it currently heavily relies on the `HasCodeCompletion` state being set on the parser result. I’ll try that in a follow-up PR. rdar://103251263
1 parent 58aaa4f commit b204f2f

File tree

9 files changed

+196
-67
lines changed

9 files changed

+196
-67
lines changed

‎include/swift/IDE/CursorInfo.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ class CursorInfoConsumer {
3232
/// Create a factory for code completion callbacks.
3333
IDEInspectionCallbacksFactory *
3434
makeCursorInfoCallbacksFactory(CursorInfoConsumer &Consumer,
35-
SourceLoc RequestedLoc);
35+
SourceLoc RequestedLoc,
36+
SourceLoc RequestedLocInOriginalBuffer);
3637

3738
} // namespace ide
3839
} // namespace swift

‎include/swift/IDETool/IDEInspectionInstance.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ makeCodeCompletionMemoryBuffer(const llvm::MemoryBuffer *origBuf,
4848
struct IDEInspectionInstanceResult {
4949
/// The compiler instance that is prepared for the second pass.
5050
std::shared_ptr<CompilerInstance> CI;
51+
/// If cursor info reuses an AST context, map the source location from the
52+
/// buffer that just contains the function to be reparsed into the original
53+
/// full buffer. From that buffer, meaningful line-column information can be
54+
/// retrieved.
55+
/// In all other cases, this is just the identity function.
56+
std::function<SourceLoc(SourceLoc)> MapSourceLocIntoOriginalBuffer;
5157
/// Whether an AST was reused.
5258
bool DidReuseAST;
5359
/// Whether the IDEInspectionInstance target could be found in the source
@@ -83,6 +89,12 @@ struct ConformingMethodListResults {
8389
struct CursorInfoResults {
8490
/// The actual results. If \c nullptr, no results were found.
8591
const ResolvedCursorInfo *Result;
92+
/// If cursor info reuses an AST context, map the source location from the
93+
/// buffer that just contains the function to be reparsed into the original
94+
/// full buffer. From that buffer, meaningful line-column information can be
95+
/// retrieved.
96+
/// /// In all other cases, this is just the identity function.
97+
std::function<SourceLoc(SourceLoc)> MapSourceLocIntoOriginalBuffer;
8698
/// Whether an AST was reused to produce the results.
8799
bool DidReuseAST;
88100
};

‎lib/IDE/CursorInfo.cpp

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,15 @@ class NodeFinderDeclResult : public NodeFinderResult {
8686
/// AST, also gathers information about shorthand shadows.
8787
class NodeFinder : ASTWalker {
8888
SourceFile &SrcFile;
89+
90+
/// The location to resolve. If the ASTContext is being reused, this might
91+
/// reside in a temporary buffer which only contains that function body.
8992
SourceLoc LocToResolve;
9093

94+
/// If \c LocToResolve resides in a temporary buffer during AST reuse, the
95+
/// corresponding location in the original, full buffer.
96+
SourceLoc LocToResolveInOriginalBuffer;
97+
9198
/// As we are walking the tree, this variable is updated to the last seen
9299
/// DeclContext.
93100
SmallVector<DeclContext *> DeclContextStack;
@@ -103,8 +110,10 @@ class NodeFinder : ASTWalker {
103110
llvm::DenseMap<ValueDecl *, ValueDecl *> ShorthandShadowedDecls;
104111

105112
public:
106-
NodeFinder(SourceFile &SrcFile, SourceLoc LocToResolve)
113+
NodeFinder(SourceFile &SrcFile, SourceLoc LocToResolve,
114+
SourceLoc LocToResolveInOriginalBuffer)
107115
: SrcFile(SrcFile), LocToResolve(LocToResolve),
116+
LocToResolveInOriginalBuffer(LocToResolveInOriginalBuffer),
108117
DeclContextStack({&SrcFile}) {}
109118

110119
void resolve() { SrcFile.walk(*this); }
@@ -133,7 +142,8 @@ class NodeFinder : ASTWalker {
133142
DeclContext *getCurrentDeclContext() { return DeclContextStack.back(); }
134143

135144
bool rangeContainsLocToResolve(SourceRange Range) const {
136-
return Range.contains(LocToResolve);
145+
return Range.contains(LocToResolveInOriginalBuffer) ||
146+
Range.contains(LocToResolve);
137147
}
138148

139149
PreWalkAction walkToDeclPre(Decl *D) override {
@@ -210,12 +220,15 @@ class NodeFinder : ASTWalker {
210220
class CursorInfoDoneParsingCallback : public IDEInspectionCallbacks {
211221
CursorInfoConsumer &Consumer;
212222
SourceLoc RequestedLoc;
223+
SourceLoc RequestedLocInOriginalBuffer;
213224

214225
public:
215226
CursorInfoDoneParsingCallback(Parser &P, CursorInfoConsumer &Consumer,
216-
SourceLoc RequestedLoc)
227+
SourceLoc RequestedLoc,
228+
SourceLoc RequestedLocInOriginalBuffer)
217229
: IDEInspectionCallbacks(P), Consumer(Consumer),
218-
RequestedLoc(RequestedLoc) {}
230+
RequestedLoc(RequestedLoc),
231+
RequestedLocInOriginalBuffer(RequestedLocInOriginalBuffer) {}
219232

220233
std::unique_ptr<ResolvedCursorInfo>
221234
getDeclResult(NodeFinderDeclResult *DeclResult, SourceFile *SrcFile,
@@ -236,7 +249,7 @@ class CursorInfoDoneParsingCallback : public IDEInspectionCallbacks {
236249
if (!SrcFile) {
237250
return;
238251
}
239-
NodeFinder Finder(*SrcFile, RequestedLoc);
252+
NodeFinder Finder(*SrcFile, RequestedLoc, RequestedLocInOriginalBuffer);
240253
Finder.resolve();
241254
auto Result = Finder.takeResult();
242255
if (!Result) {
@@ -257,22 +270,26 @@ class CursorInfoDoneParsingCallback : public IDEInspectionCallbacks {
257270

258271
} // anonymous namespace.
259272

260-
IDEInspectionCallbacksFactory *
261-
swift::ide::makeCursorInfoCallbacksFactory(CursorInfoConsumer &Consumer,
262-
SourceLoc RequestedLoc) {
273+
IDEInspectionCallbacksFactory *swift::ide::makeCursorInfoCallbacksFactory(
274+
CursorInfoConsumer &Consumer, SourceLoc RequestedLoc,
275+
SourceLoc RequestedLocInOriginalBuffer) {
263276
class CursorInfoCallbacksFactoryImpl : public IDEInspectionCallbacksFactory {
264277
CursorInfoConsumer &Consumer;
265278
SourceLoc RequestedLoc;
279+
SourceLoc RequestedLocInOriginalBuffer;
266280

267281
public:
268282
CursorInfoCallbacksFactoryImpl(CursorInfoConsumer &Consumer,
269-
SourceLoc RequestedLoc)
270-
: Consumer(Consumer), RequestedLoc(RequestedLoc) {}
283+
SourceLoc RequestedLoc,
284+
SourceLoc RequestedLocInOriginalBuffer)
285+
: Consumer(Consumer), RequestedLoc(RequestedLoc), RequestedLocInOriginalBuffer(RequestedLocInOriginalBuffer) {}
271286

272287
IDEInspectionCallbacks *createIDEInspectionCallbacks(Parser &P) override {
273-
return new CursorInfoDoneParsingCallback(P, Consumer, RequestedLoc);
288+
return new CursorInfoDoneParsingCallback(P, Consumer, RequestedLoc,
289+
RequestedLocInOriginalBuffer);
274290
}
275291
};
276292

277-
return new CursorInfoCallbacksFactoryImpl(Consumer, RequestedLoc);
293+
return new CursorInfoCallbacksFactoryImpl(Consumer, RequestedLoc,
294+
RequestedLocInOriginalBuffer);
278295
}

‎lib/IDETool/IDEInspectionInstance.cpp

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ bool IDEInspectionInstance::performCachedOperationIfPossible(
263263
if (!newState->hasIDEInspectionDelayedDeclState())
264264
return false;
265265

266+
std::function<SourceLoc(SourceLoc)> MapSourceLocIntoOriginalBuffer =
267+
[](SourceLoc Loc) { return Loc; };
268+
266269
auto &newInfo = newState->getIDEInspectionDelayedDeclState();
267270
unsigned newBufferID;
268271
DeclContext *traceDC = nullptr;
@@ -331,6 +334,17 @@ bool IDEInspectionInstance::performCachedOperationIfPossible(
331334
1);
332335
SM.setIDEInspectionTarget(newBufferID, newOffset);
333336

337+
MapSourceLocIntoOriginalBuffer = [&SM, startOffset, tmpBufferID,
338+
newBufferID](SourceLoc Loc) -> SourceLoc {
339+
if (SM.findBufferContainingLoc(Loc) == newBufferID) {
340+
return SM.getLocForOffset(tmpBufferID,
341+
SM.getLocOffsetInBuffer(Loc, newBufferID) +
342+
startOffset);
343+
} else {
344+
return Loc;
345+
}
346+
};
347+
334348
oldInfo.ParentContext = DC;
335349
oldInfo.StartOffset = newInfo.StartOffset;
336350
oldInfo.EndOffset = newInfo.EndOffset;
@@ -418,7 +432,7 @@ bool IDEInspectionInstance::performCachedOperationIfPossible(
418432
Callback(CancellableResult<IDEInspectionInstanceResult>::cancelled());
419433
} else {
420434
Callback(CancellableResult<IDEInspectionInstanceResult>::success(
421-
{CachedCI, /*reusingASTContext=*/true,
435+
{CachedCI, MapSourceLocIntoOriginalBuffer, /*reusingASTContext=*/true,
422436
/*DidFindIDEInspectionTarget=*/true}));
423437
}
424438

@@ -488,8 +502,10 @@ void IDEInspectionInstance::performNewOperation(
488502
// cancelled. Don't cache it.
489503
ShouldCacheCompilerInstance = false;
490504
} else {
505+
auto MapSourceLocIntoOriginalBuffer = [](SourceLoc Loc) { return Loc; };
491506
Callback(CancellableResult<IDEInspectionInstanceResult>::success(
492-
{CI, /*ReuisingASTContext=*/false, DidFindIDEInspectionTarget}));
507+
{CI, MapSourceLocIntoOriginalBuffer, /*ReuisingASTContext=*/false,
508+
DidFindIDEInspectionTarget}));
493509
if (CancellationFlag &&
494510
CancellationFlag->load(std::memory_order_relaxed)) {
495511
ShouldCacheCompilerInstance = false;
@@ -807,15 +823,18 @@ void swift::ide::IDEInspectionInstance::cursorInfo(
807823

808824
struct ConsumerToCallbackAdapter : public swift::ide::CursorInfoConsumer {
809825
bool ReusingASTContext;
826+
std::function<SourceLoc(SourceLoc)> MapSourceLocIntoOriginalBuffer;
810827
std::shared_ptr<std::atomic<bool>> CancellationFlag;
811828
llvm::function_ref<void(ResultType)> Callback;
812829
bool HandleResultsCalled = false;
813830

814831
ConsumerToCallbackAdapter(
815832
bool ReusingASTContext,
833+
std::function<SourceLoc(SourceLoc)> MapSourceLocIntoOriginalBuffer,
816834
std::shared_ptr<std::atomic<bool>> CancellationFlag,
817835
llvm::function_ref<void(ResultType)> Callback)
818836
: ReusingASTContext(ReusingASTContext),
837+
MapSourceLocIntoOriginalBuffer(MapSourceLocIntoOriginalBuffer),
819838
CancellationFlag(CancellationFlag), Callback(Callback) {}
820839

821840
void handleResults(const ResolvedCursorInfo &result) override {
@@ -824,7 +843,8 @@ void swift::ide::IDEInspectionInstance::cursorInfo(
824843
CancellationFlag->load(std::memory_order_relaxed)) {
825844
Callback(ResultType::cancelled());
826845
} else {
827-
Callback(ResultType::success({&result, ReusingASTContext}));
846+
Callback(ResultType::success(
847+
{&result, MapSourceLocIntoOriginalBuffer, ReusingASTContext}));
828848
}
829849
}
830850
};
@@ -834,18 +854,22 @@ void swift::ide::IDEInspectionInstance::cursorInfo(
834854
/*IgnoreSwiftSourceInfo=*/false, CancellationFlag,
835855
[&](CancellableResult<IDEInspectionInstanceResult> CIResult) {
836856
CIResult.mapAsync<CursorInfoResults>(
837-
[&CancellationFlag, Offset](auto &Result, auto DeliverTransformed) {
838-
auto &Mgr = Result.CI->getSourceMgr();
839-
auto RequestedLoc = Mgr.getLocForOffset(
840-
Mgr.getIDEInspectionTargetBufferID(), Offset);
857+
[&CancellationFlag](auto &Result, auto DeliverTransformed) {
858+
auto RequestedLoc =
859+
Result.CI->getSourceMgr().getIDEInspectionTargetLoc();
860+
auto RequestedLocInOriginalBuffer =
861+
Result.MapSourceLocIntoOriginalBuffer(RequestedLoc);
841862
ConsumerToCallbackAdapter Consumer(
842-
Result.DidReuseAST, CancellationFlag, DeliverTransformed);
863+
Result.DidReuseAST, Result.MapSourceLocIntoOriginalBuffer,
864+
CancellationFlag, DeliverTransformed);
843865
std::unique_ptr<IDEInspectionCallbacksFactory> callbacksFactory(
844-
ide::makeCursorInfoCallbacksFactory(Consumer, RequestedLoc));
866+
ide::makeCursorInfoCallbacksFactory(
867+
Consumer, RequestedLoc, RequestedLocInOriginalBuffer));
845868

846869
if (!Result.DidFindIDEInspectionTarget) {
847870
return DeliverTransformed(ResultType::success(
848-
{/*Results=*/nullptr, Result.DidReuseAST}));
871+
{/*Results=*/nullptr, Result.MapSourceLocIntoOriginalBuffer,
872+
Result.DidReuseAST}));
849873
}
850874

851875
performIDEInspectionSecondPass(
@@ -856,7 +880,8 @@ void swift::ide::IDEInspectionInstance::cursorInfo(
856880
// gets called exactly once, call it manually with no results
857881
// here.
858882
DeliverTransformed(ResultType::success(
859-
{/*Results=*/nullptr, Result.DidReuseAST}));
883+
{/*Results=*/nullptr, Result.MapSourceLocIntoOriginalBuffer,
884+
Result.DidReuseAST}));
860885
}
861886
},
862887
Callback);

‎test/SourceKit/CursorInfo/cursor_reuses_astcontext.swift

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// RUN: %sourcekitd-test -req=cursor -pos=%(line + 2):7 %s -- %s == -req=cursor -pos=%(line + 3):7 %s -- %s | %FileCheck %s --check-prefix IN-FUNCTION
2+
func foo() {
3+
let inFunctionA = 1
4+
let inFunctionB = "hi"
5+
}
6+
7+
// IN-FUNCTION: source.lang.swift.decl.var.local
8+
// IN-FUNCTION-NEXT: inFunctionA
9+
// IN-FUNCTION: DID REUSE AST CONTEXT: 0
10+
// IN-FUNCTION: source.lang.swift.decl.var.local
11+
// IN-FUNCTION-NEXT: inFunctionB
12+
// IN-FUNCTION: DID REUSE AST CONTEXT: 1
13+
14+
// RUN: %sourcekitd-test -req=cursor -pos=%(line + 3):9 %s -- %s == -req=cursor -pos=%(line + 4):9 %s -- %s | %FileCheck %s --check-prefix IN-INSTANCE-METHOD
15+
struct MyStruct {
16+
func test() {
17+
let inInstanceMethod1 = 2
18+
let inInstanceMethod2 = "hello"
19+
}
20+
}
21+
22+
// IN-INSTANCE-METHOD: source.lang.swift.decl.var.local
23+
// IN-INSTANCE-METHOD-NEXT: inInstanceMethod1
24+
// IN-INSTANCE-METHOD: DID REUSE AST CONTEXT: 0
25+
// IN-INSTANCE-METHOD: source.lang.swift.decl.var.local
26+
// IN-INSTANCE-METHOD-NEXT: inInstanceMethod2
27+
// IN-INSTANCE-METHOD: DID REUSE AST CONTEXT: 1

‎tools/SourceKit/include/SourceKit/Core/LangSupport.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ class RequestResult {
363363
return RequestResult();
364364
}
365365

366+
bool isValue() const { return type == Value; }
366367
const T &value() const {
367368
assert(type == Value);
368369
return *data;
@@ -581,8 +582,14 @@ struct CursorInfoData {
581582
llvm::ArrayRef<CursorSymbolInfo> Symbols;
582583
/// All available actions on the code under cursor.
583584
llvm::ArrayRef<RefactoringInfo> AvailableActions;
584-
585-
void print(llvm::raw_ostream &OS, std::string Indentation) const {
585+
/// Whether the ASTContext was reused for this cursor info.
586+
bool DidReuseAST = false;
587+
588+
/// If \p ForSolverBasedCursorInfoVerification is \c true, fields that are
589+
/// acceptable to differ between the AST-based and the solver-based result,
590+
/// will be excluded.
591+
void print(llvm::raw_ostream &OS, std::string Indentation,
592+
bool ForSolverBasedCursorInfoVerification = false) const {
586593
OS << Indentation << "CursorInfoData" << '\n';
587594
OS << Indentation << " Symbols:" << '\n';
588595
for (auto Symbol : Symbols) {
@@ -592,6 +599,9 @@ struct CursorInfoData {
592599
for (auto AvailableAction : AvailableActions) {
593600
AvailableAction.print(OS, Indentation + " ");
594601
}
602+
if (!ForSolverBasedCursorInfoVerification) {
603+
OS << Indentation << "DidReuseAST: " << DidReuseAST << '\n';
604+
}
595605
}
596606

597607
SWIFT_DEBUG_DUMP { print(llvm::errs(), ""); }

0 commit comments

Comments
 (0)