Skip to content

Commit e22e443

Browse files
committed
[SourceKit] Re-enable cancellation of non-completion requests
This enables the ability to cancel requests, which aren’t code completion requests, again. Previous crashes in SILGen are prevented by disabling cancellation during the SIL stages. Instead, we add dedicated cancellation checkpoints before and after SIL. rdar://98390926
1 parent 50c79f9 commit e22e443

File tree

4 files changed

+24
-4
lines changed

4 files changed

+24
-4
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// RUN: not %sourcekitd-test -req=diags %s -print-raw-response -id=diag -async -- %s == -cancel=diag 2>&1 | %FileCheck %s
2+
3+
func foo(x: Invalid1, y: Invalid2) {
4+
x / y / x / y / x / y / x / y
5+
}
6+
7+
// CHECK: error response (Request Cancelled)

tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,7 @@ ASTUnitRef ASTBuildOperation::buildASTUnit(std::string &Error) {
10691069
}
10701070
return nullptr;
10711071
}
1072+
CompIns.getASTContext().CancellationFlag = CancellationFlag;
10721073
registerIDERequestFunctions(CompIns.getASTContext().evaluator);
10731074
if (TracedOp.enabled()) {
10741075
TracedOp.start(TraceInfo);
@@ -1112,9 +1113,19 @@ ASTUnitRef ASTBuildOperation::buildASTUnit(std::string &Error) {
11121113
// don't block any other AST processing for the same SwiftInvocation.
11131114

11141115
if (auto SF = CompIns.getPrimarySourceFile()) {
1116+
if (CancellationFlag->load(std::memory_order_relaxed)) {
1117+
return nullptr;
1118+
}
1119+
// Disable cancellation while performing SILGen. If the cancellation flag
1120+
// is set, type checking performed during SILGen checks the cancellation
1121+
// flag and might thus fail, which SILGen cannot handle.
1122+
llvm::SaveAndRestore<std::shared_ptr<std::atomic<bool>>> DisableCancellationDuringSILGen(CompIns.getASTContext().CancellationFlag, nullptr);
11151123
SILOptions SILOpts = Invocation.getSILOptions();
11161124
auto &TC = CompIns.getSILTypes();
11171125
std::unique_ptr<SILModule> SILMod = performASTLowering(*SF, TC, SILOpts);
1126+
if (CancellationFlag->load(std::memory_order_relaxed)) {
1127+
return nullptr;
1128+
}
11181129
runSILDiagnosticPasses(*SILMod);
11191130
}
11201131
}

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,6 +1621,10 @@ static void computeDiagnostics(
16211621
auto Diagnostics = DiagConsumer.getDiagnosticsForBuffer(BufferID);
16221622
Receiver(RequestResult<DiagnosticsResult>::fromResult(Diagnostics));
16231623
}
1624+
1625+
void cancelled() override {
1626+
Receiver(RequestResult<DiagnosticsResult>::cancelled());
1627+
}
16241628
};
16251629

16261630
auto Consumer = std::make_shared<DiagnosticsConsumer>(std::move(Receiver));

unittests/SourceKit/SwiftLang/CursorInfoTest.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -424,8 +424,7 @@ TEST_F(CursorInfoTest, CursorInfoMustWaitDueTokenRace) {
424424
EXPECT_EQ(strlen("fog"), Info.Length);
425425
}
426426

427-
// Disabled until we re-enable cancellation (rdar://91251055)
428-
TEST_F(CursorInfoTest, DISABLED_CursorInfoCancelsPreviousRequest) {
427+
TEST_F(CursorInfoTest, CursorInfoCancelsPreviousRequest) {
429428
// TODO: This test case relies on the following snippet being slow to type
430429
// check so that the first cursor info request takes longer to execute than it
431430
// takes time to schedule the second request. If that is fixed, we need to
@@ -475,8 +474,7 @@ TEST_F(CursorInfoTest, DISABLED_CursorInfoCancelsPreviousRequest) {
475474
llvm::report_fatal_error("Did not receive a resonse for the first request");
476475
}
477476

478-
// Disabled until we re-enable cancellation (rdar://91251055)
479-
TEST_F(CursorInfoTest, DISABLED_CursorInfoCancellation) {
477+
TEST_F(CursorInfoTest, CursorInfoCancellation) {
480478
// TODO: This test case relies on the following snippet being slow to type
481479
// check so that the first cursor info request takes longer to execute than it
482480
// takes time to schedule the second request. If that is fixed, we need to

0 commit comments

Comments
 (0)