Skip to content

[clangd] Add BuiltinHeaders flag to Config file #129459

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 4 commits into from
Mar 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang-tools-extra/clangd/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,18 @@ 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.
std::vector<llvm::unique_function<void(std::vector<std::string> &) const>>
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;

enum class BackgroundPolicy { Build, Skip };
Expand Down
12 changes: 12 additions & 0 deletions clang-tools-extra/clangd/ConfigCompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
8 changes: 8 additions & 0 deletions clang-tools-extra/clangd/ConfigFragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ 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 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. `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;

Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/clangd/ConfigYAML.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
Expand Down
39 changes: 24 additions & 15 deletions clang-tools-extra/clangd/SystemIncludeExtractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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: Using builtin headers from query driver.");
break;
}

log("System includes extractor: successfully executed {0}\n\tgot includes: "
Expand Down
7 changes: 5 additions & 2 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ Semantic Highlighting
Compile flags
^^^^^^^^^^^^^

- Added `BuiltinHeaders` config key which controls whether clangd's built-in
headers are used or ones extracted from the driver.

Hover
^^^^^

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading