https://github.com/HighCommander4 updated https://github.com/llvm/llvm-project/pull/129459
>From 103a6843690182640d661e71517faf693c9e1c7d Mon Sep 17 00:00:00 2001 From: Khalil Estell <kamm...@gmail.com> Date: Sat, 1 Mar 2025 14:29:42 -0800 Subject: [PATCH 1/4] [clangd] Add `BuiltinHeaders` flag to Config file Usage: ``` CompileFlags: BuiltinHeaders: QueryDriver ``` The two supported options are `QueryDriver` and `Clangd`. This Controls whether Clangd should include its own built-in headers (like stddef.h), or use only QueryDriver's built-in headers. Use the option value 'Clangd' (default) to use Clangd's headers, and use 'QueryDriver' to indicate QueryDriver's headers. --- clang-tools-extra/clangd/Config.h | 2 + clang-tools-extra/clangd/ConfigCompile.cpp | 12 ++++++ clang-tools-extra/clangd/ConfigFragment.h | 6 +++ clang-tools-extra/clangd/ConfigYAML.cpp | 4 ++ .../clangd/SystemIncludeExtractor.cpp | 39 ++++++++++++------- clang-tools-extra/docs/ReleaseNotes.rst | 7 +++- 6 files changed, 53 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index 586d031d58481..33718231d9b55 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -59,6 +59,7 @@ struct Config { std::optional<std::string> FixedCDBPath; }; + enum class BuiltinHeaderPolicy { Clangd, QueryDriver }; /// Controls how the compile command for the current file is determined. struct { /// Edits to apply to the compile command, in sequence. @@ -66,6 +67,7 @@ struct Config { Edits; /// Where to search for compilation databases for this file's flags. CDBSearchSpec CDBSearch = {CDBSearchSpec::Ancestors, std::nullopt}; + BuiltinHeaderPolicy BuiltinHeaders = BuiltinHeaderPolicy::Clangd; } CompileFlags; enum class BackgroundPolicy { Build, Skip }; diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index aa2561e081047..31530c206acd7 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -290,6 +290,18 @@ struct FragmentCompiler { }); } + if (F.BuiltinHeaders) { + if (auto Val = + compileEnum<Config::BuiltinHeaderPolicy>("BuiltinHeaders", + *F.BuiltinHeaders) + .map("Clangd", Config::BuiltinHeaderPolicy::Clangd) + .map("QueryDriver", Config::BuiltinHeaderPolicy::QueryDriver) + .value()) + Out.Apply.push_back([Val](const Params &, Config &C) { + C.CompileFlags.BuiltinHeaders = *Val; + }); + } + if (F.CompilationDatabase) { std::optional<Config::CDBSearchSpec> Spec; if (**F.CompilationDatabase == "Ancestors") { diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index 9535b20253b13..c6b89cde51039 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -170,6 +170,12 @@ struct Fragment { /// - Ancestors: search all parent directories (the default) /// - std::nullopt: do not use a compilation database, just default flags. std::optional<Located<std::string>> CompilationDatabase; + + /// Controls whether Clangd should include its own built-in headers (like + /// stddef.h), or use only the QueryDriver's built-in headers. Use the + /// option value 'Clangd' (default) to indicate Clangd's headers, and use + /// 'QueryDriver' to indicate QueryDriver's headers. + std::optional<Located<std::string>> BuiltinHeaders; }; CompileFlagsBlock CompileFlags; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index 95cc5c1f9f1cf..a46d45df0d319 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -104,6 +104,10 @@ class Parser { if (auto Values = scalarValues(N)) F.Remove = std::move(*Values); }); + Dict.handle("BuiltinHeaders", [&](Node &N) { + if (auto BuiltinHeaders = scalarValue(N, "BuiltinHeaders")) + F.BuiltinHeaders = *BuiltinHeaders; + }); Dict.handle("CompilationDatabase", [&](Node &N) { F.CompilationDatabase = scalarValue(N, "CompilationDatabase"); }); diff --git a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp index c1c2e9fab9664..e9f4814738afb 100644 --- a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp +++ b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp @@ -30,6 +30,7 @@ // in the paths that are explicitly included by the user. #include "CompileCommands.h" +#include "Config.h" #include "GlobalCompilationDatabase.h" #include "support/Logger.h" #include "support/Threading.h" @@ -401,22 +402,30 @@ extractSystemIncludesAndTarget(const DriverArgs &InputArgs, 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(Info->SystemIncludes, Path); - vlog("System includes extractor: builtin headers {0} {1}", Path, - (Info->SystemIncludes.size() != Size) - ? "excluded" - : "not found in driver's response"); + switch (Config::current().CompileFlags.BuiltinHeaders) { + case Config::BuiltinHeaderPolicy::Clangd: { + // 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(Info->SystemIncludes, Path); + vlog("System includes extractor: builtin headers {0} {1}", Path, + (Info->SystemIncludes.size() != Size) + ? "excluded" + : "not found in driver's response"); + } } + break; + } + case Config::BuiltinHeaderPolicy::QueryDriver: + vlog("System includes extractor: builtin headers from query driver"); + break; } log("System includes extractor: successfully executed {0}\n\tgot includes: " diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 71edb704b49d6..2d942c8f856d1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -58,6 +58,9 @@ Semantic Highlighting Compile flags ^^^^^^^^^^^^^ +- Added `BuiltinHeaders` which controls if system includes are extracted from + Clangd or solely from the QueryDriver. + Hover ^^^^^ @@ -112,7 +115,7 @@ Changes in existing checks <clang-tidy/checks/bugprone/unchecked-optional-access>` fixing false positives from smart pointer accessors repeated in checking ``has_value`` and accessing ``value``. The option `IgnoreSmartPointerDereference` should - no longer be needed and will be removed. Also fixing false positive from + no longer be needed and will be removed. Also fixing false positive from const reference accessors to objects containing optional member. - Improved :doc:`bugprone-unsafe-functions @@ -131,7 +134,7 @@ Changes in existing checks - Improved :doc:`performance/unnecessary-value-param <clang-tidy/checks/performance/unnecessary-value-param>` check performance by - tolerating fix-it breaking compilation when functions is used as pointers + tolerating fix-it breaking compilation when functions is used as pointers to avoid matching usage of functions within the current compilation unit. - Improved :doc:`performance-move-const-arg >From 67a5679f03ba59e1cfd6b61a92200955a547dc1a Mon Sep 17 00:00:00 2001 From: Khalil Estell <kamm...@gmail.com> Date: Sun, 2 Mar 2025 16:25:14 -0800 Subject: [PATCH 2/4] Fix BuiltinHeader comment to better fit --- clang-tools-extra/clangd/ConfigFragment.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index c6b89cde51039..6f95474b9c008 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -171,10 +171,12 @@ struct Fragment { /// - std::nullopt: do not use a compilation database, just default flags. std::optional<Located<std::string>> CompilationDatabase; - /// Controls whether Clangd should include its own built-in headers (like - /// stddef.h), or use only the QueryDriver's built-in headers. Use the + /// Controls whether Clangd should use its own built-in system headers (like + /// stddef.h), or use the system headers from the query driver. Use the /// option value 'Clangd' (default) to indicate Clangd's headers, and use - /// 'QueryDriver' to indicate QueryDriver's headers. + /// 'QueryDriver' to indicate QueryDriver's headers. `Clangd` is the + /// fallback if no query driver is supplied or if the query driver regex + /// string fails to match the compiler used in the CDB. std::optional<Located<std::string>> BuiltinHeaders; }; CompileFlagsBlock CompileFlags; >From 4cb27301bb4b12672d9450d6912b22814e83b94b Mon Sep 17 00:00:00 2001 From: Khalil Estell <kamm...@gmail.com> Date: Sat, 8 Mar 2025 07:28:58 -0800 Subject: [PATCH 3/4] Update comments & logs --- clang-tools-extra/clangd/Config.h | 3 +++ clang-tools-extra/clangd/SystemIncludeExtractor.cpp | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index 33718231d9b55..3f8a3c9b060f6 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -67,6 +67,9 @@ struct Config { Edits; /// Where to search for compilation databases for this file's flags. CDBSearchSpec CDBSearch = {CDBSearchSpec::Ancestors, std::nullopt}; + + /// Whether to use clangd's own builtin headers, or ones from the system + /// include extractor, if available. BuiltinHeaderPolicy BuiltinHeaders = BuiltinHeaderPolicy::Clangd; } CompileFlags; diff --git a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp index e9f4814738afb..9399b910025b6 100644 --- a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp +++ b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp @@ -424,7 +424,7 @@ extractSystemIncludesAndTarget(const DriverArgs &InputArgs, break; } case Config::BuiltinHeaderPolicy::QueryDriver: - vlog("System includes extractor: builtin headers from query driver"); + vlog("System includes extractor: Using builtin headers from query driver."); break; } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2d942c8f856d1..65d5c00243769 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -58,8 +58,8 @@ Semantic Highlighting Compile flags ^^^^^^^^^^^^^ -- Added `BuiltinHeaders` which controls if system includes are extracted from - Clangd or solely from the QueryDriver. +- Added `BuiltinHeaders` config key which controls if system includes are + extracted from clangd or solely from the query driver. Hover ^^^^^ >From 406f339dca9b7d888e47e79d484bc02dbf6f1755 Mon Sep 17 00:00:00 2001 From: Nathan Ridge <zeratul...@hotmail.com> Date: Sat, 8 Mar 2025 19:07:14 -0500 Subject: [PATCH 4/4] Slight re-word of release note --- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 65d5c00243769..3edcd5890f92e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -58,8 +58,8 @@ Semantic Highlighting Compile flags ^^^^^^^^^^^^^ -- Added `BuiltinHeaders` config key which controls if system includes are - extracted from clangd or solely from the query driver. +- Added `BuiltinHeaders` config key which controls whether clangd's built-in + headers are used or ones extracted from the driver. Hover ^^^^^ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits