Skip to content

Commit f48c64f

Browse files
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 15c9fc7 commit f48c64f

File tree

11 files changed

+167
-31
lines changed

11 files changed

+167
-31
lines changed

include/swift/Basic/SourceManager.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,17 @@ class SourceManager {
166166

167167
SourceLoc getIDEInspectionTargetLoc() const;
168168

169+
/// Returns whether `Range` contains `Loc`. This also respects the
170+
/// `ReplacedRanges`, i.e. if `Loc` is in a range that replaces a range which
171+
/// overlaps `Range`, this also returns `true`.
172+
bool containsRespectingReplacedRanges(SourceRange Range, SourceLoc Loc) const;
173+
174+
/// Returns whether `Enclosing` contains `Inner`. This also respects the
175+
/// `ReplacedRanges`, i.e. if `Inner` is contained in a range that replaces a
176+
/// range which overlaps `Range`, this also returns `true`.
177+
bool rangeContainsRespectingReplacedRanges(SourceRange Enclosing,
178+
SourceRange Inner) const;
179+
169180
const llvm::DenseMap<SourceRange, SourceRange> &getReplacedRanges() const {
170181
return ReplacedRanges;
171182
}

lib/Basic/SourceLoc.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,31 @@ SourceLoc SourceManager::getIDEInspectionTargetLoc() const {
4545
.getAdvancedLoc(IDEInspectionTargetOffset);
4646
}
4747

48+
bool SourceManager::containsRespectingReplacedRanges(SourceRange Range,
49+
SourceLoc Loc) const {
50+
if (Loc.isInvalid() || Range.isInvalid()) {
51+
return false;
52+
}
53+
54+
if (Range.contains(Loc)) {
55+
return true;
56+
}
57+
for (const auto &pair : getReplacedRanges()) {
58+
auto OriginalRange = pair.first;
59+
auto NewRange = pair.second;
60+
if (NewRange.contains(Loc) && Range.overlaps(OriginalRange)) {
61+
return true;
62+
}
63+
}
64+
return false;
65+
}
66+
67+
bool SourceManager::rangeContainsRespectingReplacedRanges(
68+
SourceRange Enclosing, SourceRange Inner) const {
69+
return containsRespectingReplacedRanges(Enclosing, Inner.Start) &&
70+
containsRespectingReplacedRanges(Enclosing, Inner.End);
71+
}
72+
4873
StringRef SourceManager::getDisplayNameForLoc(SourceLoc Loc, bool ForceGeneratedSourceToDisk) const {
4974
// Respect #line first
5075
if (auto VFile = getVirtualFile(Loc))

lib/IDE/CursorInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class NodeFinder : ASTWalker {
140140
DeclContext *getCurrentDeclContext() { return DeclContextStack.back(); }
141141

142142
bool rangeContainsLocToResolve(SourceRange Range) const {
143-
return Range.contains(LocToResolve);
143+
return getSourceMgr().containsRespectingReplacedRanges(Range, LocToResolve);
144144
}
145145

146146
PreWalkAction walkToDeclPre(Decl *D) override {

lib/IDETool/IDEInspectionInstance.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -842,10 +842,9 @@ void swift::ide::IDEInspectionInstance::cursorInfo(
842842
CancellationFlag,
843843
[&](CancellableResult<IDEInspectionInstanceResult> CIResult) {
844844
CIResult.mapAsync<CursorInfoResults>(
845-
[&CancellationFlag, Offset](auto &Result, auto DeliverTransformed) {
845+
[&CancellationFlag](auto &Result, auto DeliverTransformed) {
846846
auto &Mgr = Result.CI->getSourceMgr();
847-
auto RequestedLoc = Mgr.getLocForOffset(
848-
Mgr.getIDEInspectionTargetBufferID(), Offset);
847+
auto RequestedLoc = Mgr.getIDEInspectionTargetLoc();
849848
ConsumerToCallbackAdapter Consumer(
850849
Result.DidReuseAST, CancellationFlag, DeliverTransformed);
851850
std::unique_ptr<IDEInspectionCallbacksFactory> callbacksFactory(

lib/Refactoring/Refactoring.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,11 @@ class ContextFinder : public SourceEntityWalker {
5252
std::function<bool(ASTNode)> IsContext;
5353
SmallVector<ASTNode, 4> AllContexts;
5454
bool contains(ASTNode Enclosing) {
55-
auto Result = SM.rangeContains(Enclosing.getSourceRange(), Target);
56-
if (Result && IsContext(Enclosing))
55+
auto Result = SM.rangeContainsRespectingReplacedRanges(
56+
Enclosing.getSourceRange(), Target);
57+
if (Result && IsContext(Enclosing)) {
5758
AllContexts.push_back(Enclosing);
59+
}
5860
return Result;
5961
}
6062
public:
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
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
func materialize() {
2+
let a = 2
3+
}
4+
5+
func outer() {
6+
func inner() {
7+
}
8+
}
9+
10+
// RUN: %sourcekitd-test -req=cursor -pos=2:6 %s -- %s == \
11+
// RUN: -req=cursor -req-opts=retrieve_refactor_actions=1 -pos=6:8 %s -- %s | %FileCheck %s
12+
13+
// CHECK: source.lang.swift.decl.function.free (6:8-6:15)
14+
// CHECK: inner()
15+
// CHECK: ACTIONS BEGIN
16+
// CHECK-DAG: source.refactoring.kind.rename.local
17+
// CHECK-DAG: Local Rename
18+
// CHECK-DAG: source.refactoring.kind.convert.func-to-async
19+
// CHECK-DAG: Convert Function to Async
20+
// CHECK: ACTIONS END

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;
@@ -588,8 +589,14 @@ struct CursorInfoData {
588589
llvm::ArrayRef<CursorSymbolInfo> Symbols;
589590
/// All available actions on the code under cursor.
590591
llvm::ArrayRef<RefactoringInfo> AvailableActions;
591-
592-
void print(llvm::raw_ostream &OS, std::string Indentation) const {
592+
/// Whether the ASTContext was reused for this cursor info.
593+
bool DidReuseAST = false;
594+
595+
/// If \p ForSolverBasedCursorInfoVerification is \c true, fields that are
596+
/// acceptable to differ between the AST-based and the solver-based result,
597+
/// will be excluded.
598+
void print(llvm::raw_ostream &OS, std::string Indentation,
599+
bool ForSolverBasedCursorInfoVerification = false) const {
593600
OS << Indentation << "CursorInfoData" << '\n';
594601
OS << Indentation << " Symbols:" << '\n';
595602
for (auto Symbol : Symbols) {
@@ -599,6 +606,9 @@ struct CursorInfoData {
599606
for (auto AvailableAction : AvailableActions) {
600607
AvailableAction.print(OS, Indentation + " ");
601608
}
609+
if (!ForSolverBasedCursorInfoVerification) {
610+
OS << Indentation << "DidReuseAST: " << DidReuseAST << '\n';
611+
}
602612
}
603613

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

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

Lines changed: 60 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,24 @@ static void setLocationInfo(const ValueDecl *VD,
911911
Location.Length = NameLen;
912912
std::tie(Location.Line, Location.Column) = SM.getLineAndColumnInBuffer(
913913
Loc, DeclBufID);
914+
if (auto GeneratedSourceInfo = SM.getGeneratedSourceInfo(DeclBufID)) {
915+
if (GeneratedSourceInfo->kind ==
916+
GeneratedSourceInfo::ReplacedFunctionBody) {
917+
// The location was in a temporary source buffer that just contains the
918+
// function body and which we created while reusing the ASTContext for
919+
// the rest of the file. Map the location back to the original file.
920+
unsigned OriginalBufID = SM.findBufferContainingLoc(
921+
GeneratedSourceInfo->originalSourceRange.Start);
922+
auto OriginalStartOffset = SM.getLocOffsetInBuffer(
923+
GeneratedSourceInfo->originalSourceRange.Start, OriginalBufID);
924+
auto GeneratedStartOffset = SM.getLocOffsetInBuffer(
925+
GeneratedSourceInfo->generatedSourceRange.Start, DeclBufID);
926+
Location.Offset += OriginalStartOffset - GeneratedStartOffset;
927+
assert(SM.findBufferContainingLoc(Loc) == DeclBufID);
928+
std::tie(Location.Line, Location.Column) =
929+
SM.getPresumedLineAndColumnForLoc(Loc, DeclBufID);
930+
}
931+
}
914932
} else if (ClangNode) {
915933
ClangImporter *Importer =
916934
static_cast<ClangImporter*>(Ctx.getClangModuleLoader());
@@ -1146,6 +1164,7 @@ static bool passCursorInfoForDecl(
11461164
bool AddSymbolGraph, ArrayRef<RefactoringInfo> KnownRefactoringInfo,
11471165
SwiftLangSupport &Lang, const CompilerInvocation &Invoc,
11481166
std::string &Diagnostic, ArrayRef<ImmutableTextSnapshotRef> PreviousSnaps,
1167+
bool DidReuseAST,
11491168
std::function<void(const RequestResult<CursorInfoData> &)> Receiver) {
11501169
DeclInfo OrigInfo(Info.getValueD(), Info.getContainerType(), Info.isRef(),
11511170
Info.isDynamic(), Info.getReceiverTypes(), Invoc);
@@ -1215,6 +1234,7 @@ static bool passCursorInfoForDecl(
12151234
CursorInfoData Data;
12161235
Data.Symbols = llvm::makeArrayRef(Symbols);
12171236
Data.AvailableActions = llvm::makeArrayRef(Refactorings);
1237+
Data.DidReuseAST = DidReuseAST;
12181238
Receiver(RequestResult<CursorInfoData>::fromResult(Data));
12191239
return true;
12201240
}
@@ -1554,7 +1574,8 @@ static void resolveCursor(
15541574
bool Success = passCursorInfoForDecl(
15551575
cast<ResolvedValueRefCursorInfo>(CursorInfo), Actionables,
15561576
SymbolGraph, Actions, Lang, CompInvok, Diagnostic,
1557-
getPreviousASTSnaps(), Receiver);
1577+
getPreviousASTSnaps(),
1578+
/*DidReuseAST=*/false, Receiver);
15581579
if (!Success) {
15591580
if (!getPreviousASTSnaps().empty()) {
15601581
// Attempt again using the up-to-date AST.
@@ -1873,7 +1894,8 @@ static void deliverCursorInfoResults(
18731894
Results.getResult().Result)) {
18741895
std::string Diagnostic; // Unused
18751896
passCursorInfoForDecl(*Result, AddRefactorings, AddSymbolGraph, {}, Lang,
1876-
Invoc, Diagnostic, /*PreviousSnaps=*/{}, Receiver);
1897+
Invoc, Diagnostic, /*PreviousSnaps=*/{},
1898+
Results->DidReuseAST, Receiver);
18771899
}
18781900
break;
18791901
}
@@ -1914,8 +1936,10 @@ void SwiftLangSupport::getCursorInfo(
19141936
ResolvedValueRefCursorInfo Info;
19151937
Info.setValueD(const_cast<ValueDecl *>(Entity.Dcl));
19161938
Info.setIsRef(Entity.IsRef);
1917-
passCursorInfoForDecl(Info, Actionables, SymbolGraph, {}, *this,
1918-
Invok, Diagnostic, {}, Receiver);
1939+
passCursorInfoForDecl(
1940+
Info, Actionables, SymbolGraph, {}, *this, Invok, Diagnostic,
1941+
/*PreviousSnaps=*/{},
1942+
/*DidReuseAST=*/false, Receiver);
19191943
}
19201944
} else {
19211945
CursorInfoData Info;
@@ -1940,12 +1964,10 @@ void SwiftLangSupport::getCursorInfo(
19401964

19411965
/// Counts how many symbols \p Res contains.
19421966
auto ResultCount = [](const RequestResult<CursorInfoData> &Res) -> size_t {
1943-
if (Res.isCancelled()) {
1944-
return 0;
1945-
} else if (Res.isError()) {
1946-
return 0;
1947-
} else {
1967+
if (Res.isValue()) {
19481968
return Res.value().Symbols.size();
1969+
} else {
1970+
return 0;
19491971
}
19501972
};
19511973

@@ -1959,7 +1981,8 @@ void SwiftLangSupport::getCursorInfo(
19591981
} else {
19601982
std::string Description;
19611983
llvm::raw_string_ostream OS(Description);
1962-
Res.value().print(OS, /*Indentation=*/"");
1984+
Res.value().print(OS, /*Indentation=*/"",
1985+
/*ForSolverBasedCursorInfoVerification=*/true);
19631986
return OS.str();
19641987
}
19651988
};
@@ -1979,6 +2002,7 @@ void SwiftLangSupport::getCursorInfo(
19792002
// AST-based result.
19802003
std::string SolverBasedResultDescription;
19812004
size_t SolverBasedResultCount = 0;
2005+
bool SolverBasedReusedAST = false;
19822006
if (EnableSolverBasedCursorInfo) {
19832007
std::string InputFileError;
19842008
llvm::SmallString<64> RealInputFilePath;
@@ -1990,6 +2014,9 @@ void SwiftLangSupport::getCursorInfo(
19902014
auto SolverBasedReceiver = [&](const RequestResult<CursorInfoData> &Res) {
19912015
SolverBasedResultCount = ResultCount(Res);
19922016
SolverBasedResultDescription = ResultDescription(Res);
2017+
if (Res.isValue()) {
2018+
SolverBasedReusedAST = Res.value().DidReuseAST;
2019+
}
19932020
};
19942021

19952022
CompilerInvocation CompInvok;
@@ -2053,15 +2080,24 @@ void SwiftLangSupport::getCursorInfo(
20532080
// Thunk around `Receiver` that, if solver-based cursor info is enabled,
20542081
// verifies that the solver-based cursor info result matches the AST-based
20552082
// result.
2056-
auto ReceiverThunk = [Receiver, VerifySolverBasedResult](
2057-
const RequestResult<CursorInfoData> &Res) {
2058-
auto VerificationError = VerifySolverBasedResult(Res);
2059-
if (VerificationError.empty()) {
2060-
Receiver(Res);
2061-
} else {
2062-
Receiver(RequestResult<CursorInfoData>::fromError(VerificationError));
2063-
}
2064-
};
2083+
auto ReceiverThunk =
2084+
[Receiver, VerifySolverBasedResult,
2085+
SolverBasedReusedAST](const RequestResult<CursorInfoData> &Res) {
2086+
auto VerificationError = VerifySolverBasedResult(Res);
2087+
if (VerificationError.empty()) {
2088+
if (Res.isValue()) {
2089+
// Report whether the solver-based implemenatation reused the AST so
2090+
// we can check it in test cases.
2091+
auto Value = Res.value();
2092+
Value.DidReuseAST = SolverBasedReusedAST;
2093+
Receiver(RequestResult<CursorInfoData>::fromResult(Value));
2094+
} else {
2095+
Receiver(Res);
2096+
}
2097+
} else {
2098+
Receiver(RequestResult<CursorInfoData>::fromError(VerificationError));
2099+
}
2100+
};
20652101

20662102
resolveCursor(*this, InputFile, Offset, Length, Actionables, SymbolGraph,
20672103
Invok, /*TryExistingAST=*/true, CancelOnSubsequentRequest,
@@ -2253,10 +2289,11 @@ static void resolveCursorFromUSR(
22532289
}
22542290

22552291
std::string Diagnostic;
2256-
bool Success =
2257-
passCursorInfoForDecl(Info, /*AddRefactorings*/ false,
2258-
/*AddSymbolGraph*/ false, {}, Lang, CompInvok,
2259-
Diagnostic, PreviousASTSnaps, Receiver);
2292+
bool Success = passCursorInfoForDecl(
2293+
Info, /*AddRefactorings*/ false,
2294+
/*AddSymbolGraph*/ false, {}, Lang, CompInvok, Diagnostic,
2295+
PreviousASTSnaps,
2296+
/*DidReuseAST=*/false, Receiver);
22602297
if (!Success) {
22612298
if (!PreviousASTSnaps.empty()) {
22622299
// Attempt again using the up-to-date AST.

tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1991,6 +1991,9 @@ static void printCursorInfo(sourcekitd_variant_t Info, StringRef FilenameIn,
19911991
OS << "-----\n";
19921992
}
19931993
OS << "SECONDARY SYMBOLS END\n";
1994+
OS << "DID REUSE AST CONTEXT: "
1995+
<< sourcekitd_variant_dictionary_get_bool(Info, KeyReusingASTContext)
1996+
<< '\n';
19941997
}
19951998

19961999
static void printRangeInfo(sourcekitd_variant_t Info, StringRef FilenameIn,

tools/SourceKit/tools/sourcekitd/lib/Service/Requests.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2406,6 +2406,8 @@ static void reportCursorInfo(const RequestResult<CursorInfoData> &Result,
24062406
}
24072407
}
24082408

2409+
Elem.setBool(KeyReusingASTContext, Info.DidReuseAST);
2410+
24092411
return Rec(RespBuilder.createResponse());
24102412
}
24112413

0 commit comments

Comments
 (0)