[clang-tools-extra] [clangd] Add `BuiltinHeaders` flag to Config file (PR #129459)

[Nathan Ridge via cfe-commits]

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 <kammce0 at 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 <kammce0 at 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 <kammce0 at 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 <zeratul976 at 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
 ^^^^^