[clang-tools-extra] 0478ef2 - [clangd] Exclude builtin headers from system include extraction
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 25 09:19:44 PDT 2023
Author: Sam McCall
Date: 2023-07-25T18:19:37+02:00
New Revision: 0478ef2d366c6f88678e37d54190dcdaa0ec69da
URL: https://github.com/llvm/llvm-project/commit/0478ef2d366c6f88678e37d54190dcdaa0ec69da
DIFF: https://github.com/llvm/llvm-project/commit/0478ef2d366c6f88678e37d54190dcdaa0ec69da.diff
LOG: [clangd] Exclude builtin headers from system include extraction
Most headers that we discover are likely to be fairly portable across at least
clang/gcc, which is why we get away with adding them to clangd's search path.
This is not the case for the compiler builtin headers, which are shipped with
the parser and rely on parser builtins/features. We're better off *not* using
the the scanned builtin headers, and hoping our own intrinsics provide the
interface the code is relying on.
Fixes https://github.com/clangd/clangd/issues/1695
Differential Revision: https://reviews.llvm.org/D156044
Added:
Modified:
clang-tools-extra/clangd/SystemIncludeExtractor.cpp
clang-tools-extra/clangd/test/system-include-extractor.test
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
index 4d359b0058280b..6cce28f53b5771 100644
--- a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -83,17 +83,16 @@ struct DriverArgs {
// Whether certain includes should be part of query.
bool StandardIncludes = true;
bool StandardCXXIncludes = true;
- bool BuiltinIncludes = true;
// Language to use while querying.
std::string Lang;
std::string Sysroot;
std::string ISysroot;
bool operator==(const DriverArgs &RHS) const {
- return std::tie(Driver, StandardIncludes, StandardCXXIncludes,
- BuiltinIncludes, Lang, Sysroot, ISysroot) ==
+ return std::tie(Driver, StandardIncludes, StandardCXXIncludes, Lang,
+ Sysroot, ISysroot) ==
std::tie(RHS.Driver, RHS.StandardIncludes, RHS.StandardCXXIncludes,
- RHS.BuiltinIncludes, RHS.Lang, RHS.Sysroot, ISysroot);
+ RHS.Lang, RHS.Sysroot, ISysroot);
}
DriverArgs(const tooling::CompileCommand &Cmd, llvm::StringRef File) {
@@ -120,8 +119,6 @@ struct DriverArgs {
StandardIncludes = false;
else if (Arg == "-nostdinc++")
StandardCXXIncludes = false;
- else if (Arg == "-nobuiltininc")
- BuiltinIncludes = false;
// Figure out sysroot
else if (Arg.consume_front("--sysroot")) {
if (Arg.consume_front("="))
@@ -156,8 +153,6 @@ struct DriverArgs {
Args.push_back("-nostdinc");
if (!StandardCXXIncludes)
Args.push_back("-nostdinc++");
- if (!BuiltinIncludes)
- Args.push_back("-nobuiltininc++");
if (!Sysroot.empty())
Args.append({"--sysroot", Sysroot});
if (!ISysroot.empty())
@@ -190,7 +185,6 @@ template <> struct DenseMapInfo<DriverArgs> {
Val.Driver,
Val.StandardIncludes,
Val.StandardCXXIncludes,
- Val.BuiltinIncludes,
Val.Lang,
Val.Sysroot,
Val.ISysroot,
@@ -274,6 +268,42 @@ std::optional<DriverInfo> parseDriverOutput(llvm::StringRef Output) {
return std::move(Info);
}
+std::optional<std::string> run(llvm::ArrayRef<llvm::StringRef> Argv,
+ bool OutputIsStderr) {
+ llvm::SmallString<128> OutputPath;
+ if (auto EC = llvm::sys::fs::createTemporaryFile("system-includes", "clangd",
+ OutputPath)) {
+ elog("System include extraction: failed to create temporary file with "
+ "error {0}",
+ EC.message());
+ return std::nullopt;
+ }
+ auto CleanUp = llvm::make_scope_exit(
+ [&OutputPath]() { llvm::sys::fs::remove(OutputPath); });
+
+ std::optional<llvm::StringRef> Redirects[] = {{""}, {""}, {""}};
+ Redirects[OutputIsStderr ? 2 : 1] = OutputPath.str();
+
+ std::string ErrMsg;
+ if (int RC =
+ llvm::sys::ExecuteAndWait(Argv.front(), Argv, /*Env=*/std::nullopt,
+ Redirects, /*SecondsToWait=*/0,
+ /*MemoryLimit=*/0, &ErrMsg)) {
+ elog("System include extraction: driver execution failed with return code: "
+ "{0} - '{1}'. Args: [{2}]",
+ llvm::to_string(RC), ErrMsg, printArgv(Argv));
+ return std::nullopt;
+ }
+
+ auto BufOrError = llvm::MemoryBuffer::getFile(OutputPath);
+ if (!BufOrError) {
+ elog("System include extraction: failed to read {0} with error {1}",
+ OutputPath, BufOrError.getError().message());
+ return std::nullopt;
+ }
+ return BufOrError.get().get()->getBuffer().str();
+}
+
std::optional<DriverInfo>
extractSystemIncludesAndTarget(const DriverArgs &InputArgs,
const llvm::Regex &QueryDriverRegex) {
@@ -300,45 +330,37 @@ extractSystemIncludesAndTarget(const DriverArgs &InputArgs,
return std::nullopt;
}
- llvm::SmallString<128> StdErrPath;
- if (auto EC = llvm::sys::fs::createTemporaryFile("system-includes", "clangd",
- StdErrPath)) {
- elog("System include extraction: failed to create temporary file with "
- "error {0}",
- EC.message());
- return std::nullopt;
- }
- auto CleanUp = llvm::make_scope_exit(
- [&StdErrPath]() { llvm::sys::fs::remove(StdErrPath); });
-
- std::optional<llvm::StringRef> Redirects[] = {{""}, {""}, StdErrPath.str()};
-
llvm::SmallVector<llvm::StringRef> Args = {Driver, "-E", "-v"};
Args.append(InputArgs.render());
// Input needs to go after Lang flags.
Args.push_back("-");
-
- std::string ErrMsg;
- if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/std::nullopt,
- Redirects, /*SecondsToWait=*/0,
- /*MemoryLimit=*/0, &ErrMsg)) {
- elog("System include extraction: driver execution failed with return code: "
- "{0} - '{1}'. Args: [{2}]",
- llvm::to_string(RC), ErrMsg, printArgv(Args));
+ auto Output = run(Args, /*OutputIsStderr=*/true);
+ if (!Output)
return std::nullopt;
- }
- auto BufOrError = llvm::MemoryBuffer::getFile(StdErrPath);
- if (!BufOrError) {
- elog("System include extraction: failed to read {0} with error {1}",
- StdErrPath, BufOrError.getError().message());
+ std::optional<DriverInfo> Info = parseDriverOutput(*Output);
+ if (!Info)
return std::nullopt;
+
+ // The built-in headers are tightly coupled to parser builtins.
+ // (These are clang's "resource dir", GCC's GCC_INCLUDE_DIR.)
+ // We should keep using clangd's versions, so exclude the queried builtins.
+ // They're not specially marked in the -v output, but we can get the path
+ // with `$DRIVER -print-file-name=include`.
+ if (auto BuiltinHeaders =
+ run({Driver, "-print-file-name=include"}, /*OutputIsStderr=*/false)) {
+ auto Path = llvm::StringRef(*BuiltinHeaders).trim();
+ if (!Path.empty() && llvm::sys::path::is_absolute(Path)) {
+ auto Size = Info->SystemIncludes.size();
+ llvm::erase_if(Info->SystemIncludes,
+ [&](llvm::StringRef Entry) { return Path == Entry; });
+ vlog("System includes extractor: builtin headers {0} {1}", Path,
+ (Info->SystemIncludes.size() != Size)
+ ? "excluded"
+ : "not found in driver's response");
+ }
}
- std::optional<DriverInfo> Info =
- parseDriverOutput(BufOrError->get()->getBuffer());
- if (!Info)
- return std::nullopt;
log("System includes extractor: successfully executed {0}\n\tgot includes: "
"\"{1}\"\n\tgot target: \"{2}\"",
Driver, llvm::join(Info->SystemIncludes, ", "), Info->Target);
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test
index 3f3a462bf844a4..2a2c900316a916 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -10,6 +10,7 @@
# %temp_dir%/my/dir2 as include search paths.
# RUN: echo '#!/bin/sh' >> %t.dir/bin/my_driver.sh
# RUN: echo '[ "$0" = "%t.dir/bin/my_driver.sh" ] || exit' >> %t.dir/bin/my_driver.sh
+# RUN: echo '[ "$1" = "-print-file-name=include" ] && echo "%t.dir/builtin" && exit' >> %t.dir/bin/my_driver.sh
# RUN: echo 'args="$*"' >> %t.dir/bin/my_driver.sh
# Check that clangd preserves certain flags like `-nostdinc` from
# original invocation in compile_commands.json.
@@ -21,6 +22,7 @@
# RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/bin/my_driver.sh
# RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/bin/my_driver.sh
# RUN: echo 'echo %t.dir/my/dir2/ >&2' >> %t.dir/bin/my_driver.sh
+# RUN: echo 'echo %t.dir/builtin >&2' >> %t.dir/bin/my_driver.sh
# RUN: echo 'printf "End of search list.\r\n" >&2' >> %t.dir/bin/my_driver.sh
# RUN: chmod +x %t.dir/bin/my_driver.sh
@@ -29,6 +31,8 @@
# RUN: touch %t.dir/my/dir/a.h
# RUN: mkdir -p %t.dir/my/dir2
# RUN: touch %t.dir/my/dir2/b.h
+# RUN: mkdir -p %t.dir/builtin
+# RUN: touch %t.dir/builtin/c.h
# 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.
@@ -53,7 +57,7 @@
"uri": "file://INPUT_DIR/the-file.cpp",
"languageId":"cpp",
"version":1,
- "text":"#include <a.h>\n#include <b.h>\n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n"
+ "text":"#include <a.h>\n#include <b.h>\n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n#if __has_include(<c.h>)\n#error \"wrong-toolchain builtins\"\n#endif\n"
}
}
}
More information about the cfe-commits
mailing list