sammccall updated this revision to Diff 543291.
sammccall added a comment.
fix incomplete cleanup of dead option
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156044/new/
https://reviews.llvm.org/D156044
Files:
clang-tools-extra/clangd/SystemIncludeExtractor.cpp
clang-tools-extra/clangd/test/system-include-extractor.test
Index: clang-tools-extra/clangd/test/system-include-extractor.test
===================================================================
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ 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"
}
}
}
Index: clang-tools-extra/clangd/SystemIncludeExtractor.cpp
===================================================================
--- clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -83,17 +83,16 @@
// 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 @@
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 @@
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 @@
Val.Driver,
Val.StandardIncludes,
Val.StandardCXXIncludes,
- Val.BuiltinIncludes,
Val.Lang,
Val.Sysroot,
Val.ISysroot,
@@ -274,6 +268,42 @@
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 @@
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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
