Skip to content

Commit 85d60a4

Browse files
authored
[clangd] Add BuiltinHeaders config option (#129459)
This option, under `CompileFlags`, governs whether clangd uses its own built-in headers (`Clangd` option value) or the built-in headers of the driver in the file's compile command (`QueryDriver` option value, applicable to cases where `--query-driver` is used to instruct clangd to ask the driver for its system include paths). The default value is `Clangd`, preserving clangd's current defaut behaviour. Fixes clangd/clangd#2074
1 parent a892a5d commit 85d60a4

File tree

6 files changed

+58
-17
lines changed

6 files changed

+58
-17
lines changed

clang-tools-extra/clangd/Config.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,18 @@ struct Config {
5959
std::optional<std::string> FixedCDBPath;
6060
};
6161

62+
enum class BuiltinHeaderPolicy { Clangd, QueryDriver };
6263
/// Controls how the compile command for the current file is determined.
6364
struct {
6465
/// Edits to apply to the compile command, in sequence.
6566
std::vector<llvm::unique_function<void(std::vector<std::string> &) const>>
6667
Edits;
6768
/// Where to search for compilation databases for this file's flags.
6869
CDBSearchSpec CDBSearch = {CDBSearchSpec::Ancestors, std::nullopt};
70+
71+
/// Whether to use clangd's own builtin headers, or ones from the system
72+
/// include extractor, if available.
73+
BuiltinHeaderPolicy BuiltinHeaders = BuiltinHeaderPolicy::Clangd;
6974
} CompileFlags;
7075

7176
enum class BackgroundPolicy { Build, Skip };

clang-tools-extra/clangd/ConfigCompile.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,18 @@ struct FragmentCompiler {
290290
});
291291
}
292292

293+
if (F.BuiltinHeaders) {
294+
if (auto Val =
295+
compileEnum<Config::BuiltinHeaderPolicy>("BuiltinHeaders",
296+
*F.BuiltinHeaders)
297+
.map("Clangd", Config::BuiltinHeaderPolicy::Clangd)
298+
.map("QueryDriver", Config::BuiltinHeaderPolicy::QueryDriver)
299+
.value())
300+
Out.Apply.push_back([Val](const Params &, Config &C) {
301+
C.CompileFlags.BuiltinHeaders = *Val;
302+
});
303+
}
304+
293305
if (F.CompilationDatabase) {
294306
std::optional<Config::CDBSearchSpec> Spec;
295307
if (**F.CompilationDatabase == "Ancestors") {

clang-tools-extra/clangd/ConfigFragment.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,14 @@ struct Fragment {
170170
/// - Ancestors: search all parent directories (the default)
171171
/// - std::nullopt: do not use a compilation database, just default flags.
172172
std::optional<Located<std::string>> CompilationDatabase;
173+
174+
/// Controls whether Clangd should use its own built-in system headers (like
175+
/// stddef.h), or use the system headers from the query driver. Use the
176+
/// option value 'Clangd' (default) to indicate Clangd's headers, and use
177+
/// 'QueryDriver' to indicate QueryDriver's headers. `Clangd` is the
178+
/// fallback if no query driver is supplied or if the query driver regex
179+
/// string fails to match the compiler used in the CDB.
180+
std::optional<Located<std::string>> BuiltinHeaders;
173181
};
174182
CompileFlagsBlock CompileFlags;
175183

clang-tools-extra/clangd/ConfigYAML.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ class Parser {
104104
if (auto Values = scalarValues(N))
105105
F.Remove = std::move(*Values);
106106
});
107+
Dict.handle("BuiltinHeaders", [&](Node &N) {
108+
if (auto BuiltinHeaders = scalarValue(N, "BuiltinHeaders"))
109+
F.BuiltinHeaders = *BuiltinHeaders;
110+
});
107111
Dict.handle("CompilationDatabase", [&](Node &N) {
108112
F.CompilationDatabase = scalarValue(N, "CompilationDatabase");
109113
});

clang-tools-extra/clangd/SystemIncludeExtractor.cpp

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
// in the paths that are explicitly included by the user.
3131

3232
#include "CompileCommands.h"
33+
#include "Config.h"
3334
#include "GlobalCompilationDatabase.h"
3435
#include "support/Logger.h"
3536
#include "support/Threading.h"
@@ -401,22 +402,30 @@ extractSystemIncludesAndTarget(const DriverArgs &InputArgs,
401402
if (!Info)
402403
return std::nullopt;
403404

404-
// The built-in headers are tightly coupled to parser builtins.
405-
// (These are clang's "resource dir", GCC's GCC_INCLUDE_DIR.)
406-
// We should keep using clangd's versions, so exclude the queried builtins.
407-
// They're not specially marked in the -v output, but we can get the path
408-
// with `$DRIVER -print-file-name=include`.
409-
if (auto BuiltinHeaders =
410-
run({Driver, "-print-file-name=include"}, /*OutputIsStderr=*/false)) {
411-
auto Path = llvm::StringRef(*BuiltinHeaders).trim();
412-
if (!Path.empty() && llvm::sys::path::is_absolute(Path)) {
413-
auto Size = Info->SystemIncludes.size();
414-
llvm::erase(Info->SystemIncludes, Path);
415-
vlog("System includes extractor: builtin headers {0} {1}", Path,
416-
(Info->SystemIncludes.size() != Size)
417-
? "excluded"
418-
: "not found in driver's response");
405+
switch (Config::current().CompileFlags.BuiltinHeaders) {
406+
case Config::BuiltinHeaderPolicy::Clangd: {
407+
// The built-in headers are tightly coupled to parser builtins.
408+
// (These are clang's "resource dir", GCC's GCC_INCLUDE_DIR.)
409+
// We should keep using clangd's versions, so exclude the queried
410+
// builtins. They're not specially marked in the -v output, but we can
411+
// get the path with `$DRIVER -print-file-name=include`.
412+
if (auto BuiltinHeaders = run({Driver, "-print-file-name=include"},
413+
/*OutputIsStderr=*/false)) {
414+
auto Path = llvm::StringRef(*BuiltinHeaders).trim();
415+
if (!Path.empty() && llvm::sys::path::is_absolute(Path)) {
416+
auto Size = Info->SystemIncludes.size();
417+
llvm::erase(Info->SystemIncludes, Path);
418+
vlog("System includes extractor: builtin headers {0} {1}", Path,
419+
(Info->SystemIncludes.size() != Size)
420+
? "excluded"
421+
: "not found in driver's response");
422+
}
419423
}
424+
break;
425+
}
426+
case Config::BuiltinHeaderPolicy::QueryDriver:
427+
vlog("System includes extractor: Using builtin headers from query driver.");
428+
break;
420429
}
421430

422431
log("System includes extractor: successfully executed {0}\n\tgot includes: "

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ Semantic Highlighting
5858
Compile flags
5959
^^^^^^^^^^^^^
6060

61+
- Added `BuiltinHeaders` config key which controls whether clangd's built-in
62+
headers are used or ones extracted from the driver.
63+
6164
Hover
6265
^^^^^
6366

@@ -112,7 +115,7 @@ Changes in existing checks
112115
<clang-tidy/checks/bugprone/unchecked-optional-access>` fixing false
113116
positives from smart pointer accessors repeated in checking ``has_value``
114117
and accessing ``value``. The option `IgnoreSmartPointerDereference` should
115-
no longer be needed and will be removed. Also fixing false positive from
118+
no longer be needed and will be removed. Also fixing false positive from
116119
const reference accessors to objects containing optional member.
117120

118121
- Improved :doc:`bugprone-unsafe-functions
@@ -135,7 +138,7 @@ Changes in existing checks
135138

136139
- Improved :doc:`performance/unnecessary-value-param
137140
<clang-tidy/checks/performance/unnecessary-value-param>` check performance by
138-
tolerating fix-it breaking compilation when functions is used as pointers
141+
tolerating fix-it breaking compilation when functions is used as pointers
139142
to avoid matching usage of functions within the current compilation unit.
140143

141144
- Improved :doc:`performance-move-const-arg

0 commit comments

Comments
 (0)