llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Krzysztof Jędruczyk (kjedruczyk) <details> <summary>Changes</summary> shouldRunCompletion() checked the Expected<> from positionToOffset() via operator!() but never consumed the error with takeError(). This caused an assertion failure when a TriggerCharacter completion request had a position beyond the document bounds. LLM was used to generate the unit test. Fixes: #<!-- -->196072 --- Full diff: https://github.com/llvm/llvm-project/pull/196112.diff 2 Files Affected: - (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+2-1) - (modified) clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp (+20) ``````````diff diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index ebd42abd2dd61..811c3b33d27ca 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1801,7 +1801,8 @@ bool ClangdLSPServer::shouldRunCompletion( if (!Offset) { vlog("could not convert position '{0}' to offset for file '{1}'", Params.position, Params.textDocument.uri.file()); - return true; + llvm::consumeError(Offset.takeError()); + return true; // completion code will log the error for invalid position. } return allowImplicitCompletion(*Code, *Offset); } diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp index 95bf5e54fc792..0aca1433614dc 100644 --- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -492,6 +492,26 @@ TEST_F(LSPTest, DiagModuleTest) { EXPECT_THAT(Client.diagnostics("foo.cpp"), llvm::ValueIs(testing::ElementsAre(diagMessage(DiagMsg)))); } + +// Regression test for https://github.com/llvm/llvm-project/issues/196072. +TEST_F(LSPTest, CompletionOutOfRangePosition) { + auto &Client = start(); + Client.didOpen("foo.cpp", "int x;"); + auto &Reply = Client.call( + "textDocument/completion", + llvm::json::Object{ + {"textDocument", Client.documentID("foo.cpp")}, + {"position", llvm::json::Object{{"line", 97}, {"character", 0}}}, + {"context", + llvm::json::Object{ + {"triggerKind", 2}, + {"triggerCharacter", ">"}, + }}, + }); + auto Result = Reply.take(); + ASSERT_TRUE(!!Result) << "Expected a response, not a server crash"; +} + } // namespace } // namespace clangd } // namespace clang `````````` </details> https://github.com/llvm/llvm-project/pull/196112 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
