-
Notifications
You must be signed in to change notification settings - Fork 14.3k
clangd
: support -stdlib=
flags from compile_commands.json
.
#69283
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
Conversation
The `--stdlib` flag can affect the system headers used by `clang` during compilation. By default, `clang` will use the platform-installed C++ standard headers, but with `--stdlib=libc++`, `clang` can use headers included in the distribution for its `libc++` implementation. Prior to this patch, if `compile_commands.json` specified `-stdlib=libc++` or an equivalent form and `--query-driver` took effect, `clangd` would ignore it and index based on the platform's headers. When these mismatch, e.g. due to version differences, `clangd`'s completions and the actual compilation can differ. fixes clangd/clangd#1784
@llvm/pr-subscribers-clangd Author: Chris Carlon (cjc25) ChangesThe Prior to this patch, if fixes clangd/clangd#1784 Full diff: https://github.com/llvm/llvm-project/pull/69283.diff 2 Files Affected:
diff --git a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
index 74bae786425c829..158d4a940dee333 100644
--- a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -88,12 +88,14 @@ struct DriverArgs {
std::string Sysroot;
std::string ISysroot;
std::string Target;
+ std::string Stdlib;
bool operator==(const DriverArgs &RHS) const {
return std::tie(Driver, StandardIncludes, StandardCXXIncludes, Lang,
- Sysroot, ISysroot, Target) ==
+ Sysroot, ISysroot, Target, Stdlib) ==
std::tie(RHS.Driver, RHS.StandardIncludes, RHS.StandardCXXIncludes,
- RHS.Lang, RHS.Sysroot, RHS.ISysroot, RHS.Target);
+ RHS.Lang, RHS.Sysroot, RHS.ISysroot, RHS.Target,
+ RHS.Stdlib);
}
DriverArgs(const tooling::CompileCommand &Cmd, llvm::StringRef File) {
@@ -136,6 +138,13 @@ struct DriverArgs {
} else if (Arg.consume_front("-target")) {
if (Arg.empty() && I + 1 < E)
Target = Cmd.CommandLine[I + 1];
+ } else if (Arg.consume_front("--stdlib")) {
+ if (Arg.consume_front("="))
+ Stdlib = Arg.str();
+ else if (Arg.empty() && I + 1 < E)
+ Stdlib = Cmd.CommandLine[I + 1];
+ } else if (Arg.consume_front("-stdlib=")) {
+ Stdlib = Arg.str();
}
}
@@ -175,6 +184,8 @@ struct DriverArgs {
Args.append({"-isysroot", ISysroot});
if (!Target.empty())
Args.append({"-target", Target});
+ if (!Stdlib.empty())
+ Args.append({"--stdlib", Stdlib});
return Args;
}
@@ -203,6 +214,7 @@ template <> struct DenseMapInfo<DriverArgs> {
Val.Driver,
Val.StandardIncludes,
Val.StandardCXXIncludes,
+ Val.Stdlib,
Val.Lang,
Val.Sysroot,
Val.ISysroot,
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test
index 66882e424bb9216..cbb3018b2fa7349 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -18,6 +18,7 @@
# RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> %t.dir/bin/my_driver.sh
# RUN: echo '[ -z "${args##*"-isysroot /isysroot"*}" ] || exit' >> %t.dir/bin/my_driver.sh
# RUN: echo '[ -z "${args##*"-target arm-linux-gnueabihf"*}" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"--stdlib libc++"*}" ] || exit' >> %t.dir/bin/my_driver.sh
# RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
# RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> %t.dir/bin/my_driver.sh
# RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/bin/my_driver.sh
@@ -37,7 +38,7 @@
# Generate a compile_commands.json that will query the mock driver we've
# created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp --target=arm-linux-gnueabihf -nostdinc --sysroot /my/sysroot/path -isysroot/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp --target=arm-linux-gnueabihf -nostdinc --sysroot /my/sysroot/path -isysroot/isysroot -stdlib=libc++", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
@@ -75,7 +76,7 @@
{"jsonrpc":"2.0","method":"exit"}
# Generate a different compile_commands.json which does not point to the mock driver
-# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp --target=arm-linux-gnueabihf -nostdinc --sysroot /my/sysroot/path -isysroot/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp --target=arm-linux-gnueabihf -nostdinc --sysroot /my/sysroot/path -isysroot/isysroot -stdlib=libc++", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
# Generate a clangd config file which points to the mock driver instead
# RUN: echo 'CompileFlags:' > %t.dir/.clangd
|
@@ -203,6 +214,7 @@ template <> struct DenseMapInfo<DriverArgs> { | |||
Val.Driver, | |||
Val.StandardIncludes, | |||
Val.StandardCXXIncludes, | |||
Val.Stdlib, |
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.
nit: it looks like the previous patch which introduced Target
forgot to add it here, could you add please?
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.
Done, and resorted these to match the order in DriverArgs
definition so it'll be easier to tell when things are missing in the future.
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.
LGTM
The
--stdlib
flag can affect the system headers used byclang
during compilation. By default,clang
will use the platform-installed C++ standard headers, but with--stdlib=libc++
,clang
can use headers included in the distribution for itslibc++
implementation.Prior to this patch, if
compile_commands.json
specified-stdlib=libc++
or an equivalent form and--query-driver
took effect,clangd
would ignorestdlib
and index based on the platform's headers. When these mismatch, e.g. due to version differences,clangd
's completions and the actual compilation can differ.fixes clangd/clangd#1784