Skip to content

[SourceKit] Re-enable cancellation of non-completion requests #60573

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
Aug 17, 2022
Merged

[SourceKit] Re-enable cancellation of non-completion requests #60573

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
7 changes: 7 additions & 0 deletions test/SourceKit/Diagnostics/cancel_diags.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// RUN: not %sourcekitd-test -req=diags %s -print-raw-response -id=diag -async -- %s == -cancel=diag 2>&1 | %FileCheck %s

func foo(x: Invalid1, y: Invalid2) {
x / y / x / y / x / y / x / y
Copy link
Contributor

Choose a reason for hiding this comment

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

Not the biggest fan of these tests (same with the CursorInfoTest cases). It's okay for this PR, but IMO we should add a wait either somewhere in type checking or just in ASTBuildOperation that continues once it's set. That would allow something like:

// RUN: not %sourcekitd-test -req=diags %s -print-raw-response -id=diag -async -wait -- %s == -cancel=diag == -continue

Could even have different sections we wait at, eg. before type checking, during, after, etc.

}

// CHECK: error response (Request Cancelled)
11 changes: 11 additions & 0 deletions tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,7 @@ ASTUnitRef ASTBuildOperation::buildASTUnit(std::string &Error) {
}
return nullptr;
}
CompIns.getASTContext().CancellationFlag = CancellationFlag;
registerIDERequestFunctions(CompIns.getASTContext().evaluator);
if (TracedOp.enabled()) {
TracedOp.start(TraceInfo);
Expand Down Expand Up @@ -1112,9 +1113,19 @@ ASTUnitRef ASTBuildOperation::buildASTUnit(std::string &Error) {
// don't block any other AST processing for the same SwiftInvocation.

if (auto SF = CompIns.getPrimarySourceFile()) {
if (CancellationFlag->load(std::memory_order_relaxed)) {
return nullptr;
}
// Disable cancellation while performing SILGen. If the cancellation flag
// is set, type checking performed during SILGen checks the cancellation
// flag and might thus fail, which SILGen cannot handle.
llvm::SaveAndRestore<std::shared_ptr<std::atomic<bool>>> DisableCancellationDuringSILGen(CompIns.getASTContext().CancellationFlag, nullptr);
SILOptions SILOpts = Invocation.getSILOptions();
auto &TC = CompIns.getSILTypes();
std::unique_ptr<SILModule> SILMod = performASTLowering(*SF, TC, SILOpts);
if (CancellationFlag->load(std::memory_order_relaxed)) {
return nullptr;
}
runSILDiagnosticPasses(*SILMod);
}
}
Expand Down
4 changes: 4 additions & 0 deletions tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1621,6 +1621,10 @@ static void computeDiagnostics(
auto Diagnostics = DiagConsumer.getDiagnosticsForBuffer(BufferID);
Receiver(RequestResult<DiagnosticsResult>::fromResult(Diagnostics));
}

void cancelled() override {
Receiver(RequestResult<DiagnosticsResult>::cancelled());
}
};

auto Consumer = std::make_shared<DiagnosticsConsumer>(std::move(Receiver));
Expand Down
6 changes: 2 additions & 4 deletions unittests/SourceKit/SwiftLang/CursorInfoTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,7 @@ TEST_F(CursorInfoTest, CursorInfoMustWaitDueTokenRace) {
EXPECT_EQ(strlen("fog"), Info.Length);
}

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

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