-
Notifications
You must be signed in to change notification settings - Fork 15.3k
release/21.x: [clangd] Clangd running with --experimental-modules-support crashes when the compilation database is unavailable (#153802)
#168810
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.
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
base: release/21.x
Are you sure you want to change the base?
Conversation
… when the compilation database is unavailable (llvm#153802) fixes llvm#132413 (cherry picked from commit 5b55899)
|
@ChuanqiXu9 What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clangd Author: None (llvmbot) ChangesBackport 5b55899 Requested by: @ChuanqiXu9 Full diff: https://github.com/llvm/llvm-project/pull/168810.diff 2 Files Affected:
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index 7c0eb9651feaa..c6afd0bc07cbd 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -833,6 +833,10 @@ bool OverlayCDB::setCompileCommand(PathRef File,
std::unique_ptr<ProjectModules>
OverlayCDB::getProjectModules(PathRef File) const {
auto MDB = DelegatingCDB::getProjectModules(File);
+ if (!MDB) {
+ log("Failed to get compilation Database for {0}", File);
+ return {};
+ }
MDB->setCommandMangler([&Mangler = Mangler](tooling::CompileCommand &Command,
PathRef CommandPath) {
Mangler(Command, CommandPath);
diff --git a/clang-tools-extra/clangd/test/modules_no_cdb.test b/clang-tools-extra/clangd/test/modules_no_cdb.test
new file mode 100644
index 0000000000000..8f92be2c7b3f3
--- /dev/null
+++ b/clang-tools-extra/clangd/test/modules_no_cdb.test
@@ -0,0 +1,66 @@
+# A smoke test to check that clangd works without compilation database
+#
+# Windows have different escaping modes.
+# FIXME: We should add one for windows.
+# UNSUPPORTED: system-windows
+#
+# RUN: rm -fr %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+#
+# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc
+#
+# RUN: clangd -experimental-modules-support -lit-test < %t/definition.jsonrpc \
+# RUN: | FileCheck -strict-whitespace %t/definition.jsonrpc
+
+#--- A.h
+void printA();
+
+#--- Use.cpp
+#include "A.h"
+void foo() {
+ print
+}
+
+#--- definition.jsonrpc.tmpl
+{
+ "jsonrpc": "2.0",
+ "id": 0,
+ "method": "initialize",
+ "params": {
+ "processId": 123,
+ "rootPath": "clangd",
+ "capabilities": {
+ "textDocument": {
+ "completion": {
+ "completionItem": {
+ "snippetSupport": true
+ }
+ }
+ }
+ },
+ "trace": "off"
+ }
+}
+---
+{
+ "jsonrpc": "2.0",
+ "method": "textDocument/didOpen",
+ "params": {
+ "textDocument": {
+ "uri": "file://DIR/Use.cpp",
+ "languageId": "cpp",
+ "version": 1,
+ "text": "#include \"A.h\"\nvoid foo() {\n print\n}\n"
+ }
+ }
+}
+
+# CHECK: "message"{{.*}}printA{{.*}}(fix available)
+
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file://DIR/Use.cpp"},"context":{"triggerKind":1},"position":{"line":2,"character":6}}}
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
|
|
|
HighCommander4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1: a small (basically one line) low-risk fix, and fixes a crash that many users are running into
LGTM |
Backport 5b55899
Requested by: @ChuanqiXu9