Skip to content

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

Merged
merged 2 commits into from
Oct 24, 2023

Conversation

cjc25
Copy link
Contributor

@cjc25 cjc25 commented Oct 17, 2023

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 stdlib 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

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
@llvmbot llvmbot added the clangd label Oct 17, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2023

@llvm/pr-subscribers-clangd

Author: Chris Carlon (cjc25)

Changes

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 stdlib 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


Full diff: https://github.com/llvm/llvm-project/pull/69283.diff

2 Files Affected:

  • (modified) clang-tools-extra/clangd/SystemIncludeExtractor.cpp (+14-2)
  • (modified) clang-tools-extra/clangd/test/system-include-extractor.test (+3-2)
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,
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Collaborator

@HighCommander4 HighCommander4 left a comment

Choose a reason for hiding this comment

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

LGTM

@HighCommander4 HighCommander4 merged commit 3935a29 into llvm:main Oct 24, 2023
@cjc25 cjc25 deleted the clangd-stdlib-support branch October 25, 2023 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SystemIncludesExtractor stripping -stdlib with -query-driver
3 participants