diff --git a/clang-tools-extra/clangd/Cancellation.h b/clang-tools-extra/clangd/Cancellation.h --- a/clang-tools-extra/clangd/Cancellation.h +++ b/clang-tools-extra/clangd/Cancellation.h @@ -71,20 +71,24 @@ /// Defines a new task whose cancellation may be requested. /// The returned Context defines the scope of the task. -/// When the context is active, isCancelled() is false until the Canceler is -/// invoked, and true afterwards. -std::pair cancelableTask(); +/// When the context is active, isCancelled() is 0 until the Canceler is +/// invoked, and equal to Reason afterwards. +/// Conventionally, Reason may be the LSP error code to return. +std::pair cancelableTask(int Reason = 1); -/// True if the current context is within a cancelable task which was cancelled. +/// If the current context is within a cancelled task, returns the reason. /// (If the context is within multiple nested tasks, true if any are cancelled). -/// Always false if there is no active cancelable task. +/// Always zero if there is no active cancelable task. /// This isn't free (context lookup) - don't call it in a tight loop. -bool isCancelled(const Context &Ctx = Context::current()); +int isCancelled(const Context &Ctx = Context::current()); /// Conventional error when no result is returned due to cancellation. class CancelledError : public llvm::ErrorInfo { public: static char ID; + const int Reason; + + CancelledError(int Reason) : Reason(Reason) {} void log(llvm::raw_ostream &OS) const override { OS << "Task was cancelled."; diff --git a/clang-tools-extra/clangd/Cancellation.cpp b/clang-tools-extra/clangd/Cancellation.cpp --- a/clang-tools-extra/clangd/Cancellation.cpp +++ b/clang-tools-extra/clangd/Cancellation.cpp @@ -16,27 +16,28 @@ // We don't want a cancelable scope to "shadow" an enclosing one. struct CancelState { - std::shared_ptr> Cancelled; + std::shared_ptr> Cancelled; const CancelState *Parent; }; static Key StateKey; -std::pair cancelableTask() { +std::pair cancelableTask(int Reason) { + assert(Reason != 0 && "Can't detect cancellation if Reason is zero"); CancelState State; - State.Cancelled = std::make_shared>(); + State.Cancelled = std::make_shared>(); State.Parent = Context::current().get(StateKey); return { Context::current().derive(StateKey, State), - [Flag(State.Cancelled)] { *Flag = true; }, + [Reason, Flag(State.Cancelled)] { *Flag = Reason; }, }; } -bool isCancelled(const Context &Ctx) { +int isCancelled(const Context &Ctx) { for (const CancelState *State = Ctx.get(StateKey); State != nullptr; State = State->Parent) - if (State->Cancelled->load()) - return true; - return false; + if (int Reason = State->Cancelled->load()) + return Reason; + return 0; } } // namespace clangd diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -409,7 +409,8 @@ // - cleans up the entry in RequestCancelers when it's no longer needed // If a client reuses an ID, the last wins and the first cannot be canceled. Context cancelableRequestContext(const llvm::json::Value &ID) { - auto Task = cancelableTask(); + auto Task = cancelableTask( + /*Reason=*/static_cast(ErrorCode::RequestCancelled)); auto StrID = llvm::to_string(ID); // JSON-serialize ID for map key. auto Cookie = NextRequestCookie++; // No lock, only called on main thread. { diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -214,8 +214,8 @@ this](llvm::Expected IP) mutable { if (!IP) return CB(IP.takeError()); - if (isCancelled()) - return CB(llvm::make_error()); + if (auto Reason = isCancelled()) + return CB(llvm::make_error(Reason)); llvm::Optional SpecFuzzyFind; if (!IP->Preamble) { diff --git a/clang-tools-extra/clangd/JSONTransport.cpp b/clang-tools-extra/clangd/JSONTransport.cpp --- a/clang-tools-extra/clangd/JSONTransport.cpp +++ b/clang-tools-extra/clangd/JSONTransport.cpp @@ -5,6 +5,7 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===----------------------------------------------------------------------===// +#include "Cancellation.h" #include "Logger.h" #include "Protocol.h" // For LSPError #include "Shutdown.h" @@ -22,7 +23,21 @@ // FIXME: encode cancellation errors using RequestCancelled or ContentModified // as appropriate. if (llvm::Error Unhandled = llvm::handleErrors( - std::move(E), [&](const LSPError &L) -> llvm::Error { + std::move(E), + [&](const CancelledError &C) -> llvm::Error { + switch (C.Reason) { + case static_cast(ErrorCode::ContentModified): + Code = ErrorCode::ContentModified; + Message = "Request cancelled because the document was modified"; + break; + default: + Code = ErrorCode::RequestCancelled; + Message = "Request cancelled"; + break; + } + return llvm::Error::success(); + }, + [&](const LSPError &L) -> llvm::Error { Message = L.Message; Code = L.Code; return llvm::Error::success(); diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -50,6 +50,7 @@ // Defined by the protocol. RequestCancelled = -32800, + ContentModified = -32801, }; // Models an LSP error as an llvm::Error. class LSPError : public llvm::ErrorInfo { diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -648,8 +648,8 @@ llvm::unique_function)> Action, TUScheduler::ASTActionInvalidation Invalidation) { auto Task = [=, Action = std::move(Action)]() mutable { - if (isCancelled()) - return Action(llvm::make_error()); + if (auto Reason = isCancelled()) + return Action(llvm::make_error(Reason)); llvm::Optional> AST = IdleASTs.take(this); if (!AST) { StoreDiags CompilerInvocationDiagConsumer; @@ -955,7 +955,8 @@ Canceler Invalidate = nullptr; if (Invalidation) { WithContext WC(std::move(Ctx)); - std::tie(Ctx, Invalidate) = cancelableTask(); + std::tie(Ctx, Invalidate) = cancelableTask( + /*Reason=*/static_cast(ErrorCode::ContentModified)); } Requests.push_back({std::move(Task), std::string(Name), steady_clock::now(), std::move(Ctx), UpdateType, Invalidation, diff --git a/clang-tools-extra/clangd/unittests/CancellationTests.cpp b/clang-tools-extra/clangd/unittests/CancellationTests.cpp --- a/clang-tools-extra/clangd/unittests/CancellationTests.cpp +++ b/clang-tools-extra/clangd/unittests/CancellationTests.cpp @@ -45,12 +45,13 @@ } struct NestedTasks { + enum { OuterReason = 1, InnerReason = 2 }; std::pair Outer, Inner; NestedTasks() { - Outer = cancelableTask(); + Outer = cancelableTask(OuterReason); { WithContext WithOuter(Outer.first.clone()); - Inner = cancelableTask(); + Inner = cancelableTask(InnerReason); } } }; @@ -59,13 +60,13 @@ // Cancelling inner task works but leaves outer task unaffected. NestedTasks CancelInner; CancelInner.Inner.second(); - EXPECT_TRUE(isCancelled(CancelInner.Inner.first)); + EXPECT_EQ(NestedTasks::InnerReason, isCancelled(CancelInner.Inner.first)); EXPECT_FALSE(isCancelled(CancelInner.Outer.first)); // Cancellation of outer task is inherited by inner task. NestedTasks CancelOuter; CancelOuter.Outer.second(); - EXPECT_TRUE(isCancelled(CancelOuter.Inner.first)); - EXPECT_TRUE(isCancelled(CancelOuter.Outer.first)); + EXPECT_EQ(NestedTasks::OuterReason, isCancelled(CancelOuter.Inner.first)); + EXPECT_EQ(NestedTasks::OuterReason, isCancelled(CancelOuter.Outer.first)); } TEST(CancellationTest, AsynCancellationTest) { diff --git a/clang-tools-extra/clangd/unittests/JSONTransportTests.cpp b/clang-tools-extra/clangd/unittests/JSONTransportTests.cpp --- a/clang-tools-extra/clangd/unittests/JSONTransportTests.cpp +++ b/clang-tools-extra/clangd/unittests/JSONTransportTests.cpp @@ -5,6 +5,7 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===----------------------------------------------------------------------===// +#include "Cancellation.h" #include "Protocol.h" #include "Transport.h" #include "gmock/gmock.h" @@ -70,6 +71,9 @@ if (Method == "err") Target.reply( ID, llvm::make_error("trouble at mill", ErrorCode(88))); + else if (Method == "invalidated") // gone out skew on treadle + Target.reply(ID, llvm::make_error( + static_cast(ErrorCode::ContentModified))); else Target.reply(ID, std::move(Params)); return true; @@ -140,7 +144,7 @@ --- {"jsonrpc": "2.0", "id": "xyz", "error": {"code": 99, "message": "bad!"}} --- -{"jsonrpc": "2.0", "method": "err", "id": "wxyz", "params": "boom!"} +{"jsonrpc": "2.0", "method": "invalidated", "id": "wxyz", "params": "boom!"} --- {"jsonrpc": "2.0", "method": "exit"} )jsonrpc", @@ -154,7 +158,7 @@ Reply(1234): 5678 Call foo("abcd"): "efgh" Reply("xyz"): error = 99: bad! -Call err("wxyz"): "boom!" +Call invalidated("wxyz"): "boom!" Notification exit: null )"; EXPECT_EQ(trim(E.log()), trim(WantLog)); @@ -171,11 +175,11 @@ "jsonrpc": "2.0", "result": "efgh" })" - "Content-Length: 105\r\n\r\n" + "Content-Length: 145\r\n\r\n" R"({ "error": { - "code": 88, - "message": "trouble at mill" + "code": -32801, + "message": "Request cancelled because the document was modified" }, "id": "wxyz", "jsonrpc": "2.0" diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -414,7 +414,9 @@ EXPECT_FALSE(bool(AST)); llvm::Error E = AST.takeError(); EXPECT_TRUE(E.isA()); - consumeError(std::move(E)); + handleAllErrors(std::move(E), [&](const CancelledError &E) { + EXPECT_EQ(E.Reason, static_cast(ErrorCode::ContentModified)); + }); }, TUScheduler::InvalidateOnUpdate); S.runWithAST(