[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0478ef2d366c: [clangd] Exclude builtin headers from system
include extraction (authored by sammccall).
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 \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n"
+ "text":"#include \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n#if __has_include()\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 run(llvm::ArrayRef 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 Redirects[] = {{""}, {""}, {""}};
+ Re
[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
madscientist added a comment. (just a note that another way to do this check is to look for some common intrinsic file in each directory generated by the compiler driver, and remove any directory containing those files. But, if the `-print-file-name=include` trick works reliably that's good too). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156044/new/ https://reviews.llvm.org/D156044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks a lot! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156044/new/ https://reviews.llvm.org/D156044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
madscientist added a comment. I built latest main with this patch applied and it does fix the issues in my environment. My headers that include xmmintrin.h etc. don't throw errors in clangd. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156044/new/ https://reviews.llvm.org/D156044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
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 \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n"
+ "text":"#include \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n#if __has_include()\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 run(llvm::ArrayRef 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 Redirects[] = {{""}, {""}, {""}};
+ Redirects[OutputIsStderr ? 2 : 1] = OutputPath.str();
+
+ std::string ErrMsg;
+ if
[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
sammccall updated this revision to Diff 543253.
sammccall added a comment.
remove (broken) support for -nobuiltininc
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 \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n"
+ "text":"#include \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n#if __has_include()\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,7 +83,6 @@
// 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;
@@ -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())
@@ -274,6 +269,42 @@
return std::move(Info);
}
+std::optional run(llvm::ArrayRef 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 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
extractSystemIncludesAndTarget(const DriverArgs &InputArgs,
[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction
sammccall created this revision.
sammccall added reviewers: kadircet, madscientist.
Herald added a subscriber: arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
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
Repository:
rG LLVM Github Monorepo
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 \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n"
+ "text":"#include \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif\n#if __has_include()\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
@@ -157,7 +157,7 @@
if (!StandardCXXIncludes)
Args.push_back("-nostdinc++");
if (!BuiltinIncludes)
- Args.push_back("-nobuiltininc++");
+ Args.push_back("-nobuiltininc");
if (!Sysroot.empty())
Args.append({"--sysroot", Sysroot});
if (!ISysroot.empty())
@@ -274,6 +274,42 @@
return std::move(Info);
}
+std::optional run(llvm::ArrayRef 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 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
extractSystemIncludesAndTarget(const DriverAr
