[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
HighCommander4 wrote: > > It's sad that there's not a single codepath that would handle all these, > > but maybe the perfect is the enemy of the good here. > > The code duplication was my reason for looking at the common path. If you're > fine with duplicating the code I will go ahead and do that. +1, please go ahead and wire up the include cleaner support based on Sam's outline; it seems fairly straightforward, the amount of duplication is not much, and we might as well have the config work consistently across all include-insertion features. The new test + other changes look good as well! https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
HighCommander4 wrote: (In the future, it would greatly help me as a reviewer if rebases are pushed separately from updates to address review comments, so that the latter get their own Compare link.) https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
HighCommander4 wrote:
Anyways, I rebased the old version of the patch locally so that I could
generate an interdiff, which I'll attach for reference:
```diff
diff --git a/clang-tools-extra/clangd/Config.h
b/clang-tools-extra/clangd/Config.h
index 12ed1420c93e..3559591fce00 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -124,7 +124,7 @@ struct Config {
// ::). All nested namespaces are affected as well.
std::vector FullyQualifiedNamespaces;
-// List of regexes for inserting certain headers with <> or "".
+// List of matcher functions for inserting certain headers with <> or "".
std::vector> QuotedHeaders;
std::vector> AngledHeaders;
} Style;
diff --git a/clang-tools-extra/clangd/ConfigFragment.h
b/clang-tools-extra/clangd/ConfigFragment.h
index 81474bbd86a5..f0d1080f6c83 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -302,12 +302,16 @@ struct Fragment {
/// AngledHeaders (i.e. a header matches a regex in both QuotedHeaders and
/// AngledHeaders), system headers use <> and non-system headers use "".
/// These can match any suffix of the header file in question.
+/// Matching is performed against the header text, not its absolute path
+/// within the project.
std::vector> QuotedHeaders;
/// List of regexes for headers that should always be included with a
/// <>-style include. By default, and in case of a conflict with
/// AngledHeaders (i.e. a header matches a regex in both QuotedHeaders and
/// AngledHeaders), system headers use <> and non-system headers use "".
/// These can match any suffix of the header file in question.
+/// Matching is performed against the header text, not its absolute path
+/// within the project.
std::vector> AngledHeaders;
};
StyleBlock Style;
diff --git a/clang-tools-extra/clangd/Headers.h
b/clang-tools-extra/clangd/Headers.h
index e1f3eaa3397b..b91179da253e 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -9,7 +9,6 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H
-#include "Config.h"
#include "Protocol.h"
#include "SourceCode.h"
#include "index/Symbol.h"
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 6d387fec9b38..bf264a5a44ee 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -11,6 +11,7 @@
#include "ClangdServer.h"
#include "CodeComplete.h"
#include "Compiler.h"
+#include "Config.h"
#include "Feature.h"
#include "Matchers.h"
#include "Protocol.h"
@@ -916,6 +917,41 @@ TEST(CompletionTest,
NoIncludeInsertionWhenDeclFoundInFile) {
AllOf(named("Y"), Not(insertInclude();
}
+TEST(CompletionTest, IncludeInsertionRespectsQuotedAngledConfig) {
+ TestTU TU;
+ TU.ExtraArgs.push_back("-I" + testPath("sub"));
+ TU.AdditionalFiles["sub/bar.h"] = "";
+ auto BarURI = URI::create(testPath("sub/bar.h")).toString();
+
+ Symbol Sym = cls("ns::X");
+ Sym.CanonicalDeclaration.FileURI = BarURI.c_str();
+ Sym.IncludeHeaders.emplace_back(BarURI, 1, Symbol::Include);
+ Annotations Test("int main() { ns::^ }");
+ TU.Code = Test.code().str();
+ auto Results = completions(TU, Test.point(), {Sym});
+ // Default for a local path is quoted include
+ EXPECT_THAT(Results.Completions,
+ ElementsAre(AllOf(named("X"), insertInclude("\"bar.h\"";
+ {
+Config C;
+C.Style.AngledHeaders.push_back(
+[](auto header) { return header == "bar.h"; });
+WithContextValue WithCfg(Config::Key, std::move(C));
+Results = completions(TU, Test.point(), {Sym});
+EXPECT_THAT(Results.Completions,
+ElementsAre(AllOf(named("X"), insertInclude("";
+ }
+ {
+Config C;
+C.Style.QuotedHeaders.push_back(
+[](auto header) { return header == "bar.h"; });
+WithContextValue WithCfg(Config::Key, std::move(C));
+Results = completions(TU, Test.point(), {Sym});
+EXPECT_THAT(Results.Completions,
+ElementsAre(AllOf(named("X"), insertInclude("\"bar.h\"";
+ }
+}
+
TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
Annotations Test(R"cpp(
#include "bar.h"
@@ -1134,8 +1170,8 @@ TEST(CodeCompleteTest, NoColonColonAtTheEnd) {
}
TEST(CompletionTests, EmptySnippetDoesNotCrash) {
-// See https://github.com/clangd/clangd/issues/1216
-auto Results = completions(R"cpp(
+ // See https://github.com/clangd/clangd/issues/1216
+ auto Results = completions(R"cpp(
int main() {
auto w = [&](auto &) { return f(f); };
auto f = w([&](auto &) {
@@ -1151,18 +1187,18 @@ TEST(CompletionTests, EmptySnippetDoesNotCrash) {
}
TEST(CompletionTest, Issue1427Crash) {
-
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
HighCommander4 wrote: > The compare button is a bit useless when it has a bunch of changes from main > on it > (https://github.com/llvm/llvm-project/compare/8a2b22e69f82e3f4caa271b57b998d1c03b21d39..67971ca27ef5e2767aba5cfe2cec1021c4de5ec1) Thanks. Yeah, it looks like the "Compare" link next to the force-push notification in the Conversation view is the sort of thing I'm looking for, except that in this case, the patch was rebased since the previous version, and the rebase and the changes to address the review comments were pushed together, and, unlike Phabricator, Github doesn't seem to know to filter out the former. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
ADKaster wrote: Yeah seems llvm prefers not to force push PRs, but to keep stacking commits. The squashed commit will have the PR description as the final commit description. The compare button is a bit useless when it has a bunch of changes from main on it (https://github.com/llvm/llvm-project/compare/8a2b22e69f82e3f4caa271b57b998d1c03b21d39..67971ca27ef5e2767aba5cfe2cec1021c4de5ec1) https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
kleinesfilmroellchen wrote: > Sorry, I'm still struggling with Github reviews compared to Phabricator... > how do I get to an interdiff showing the latest version of the patch compared > to the version I reviewed? I'm fairly sure that GitHub has no built-in feature for this; I've never used it for my own reviews but it's unfortunate that that is missing of course. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
HighCommander4 wrote: Sorry, I'm still struggling with Github reviews compared to Phabricator... how do I get to an interdiff showing the latest version of the patch compared to the version I reviewed? https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
kleinesfilmroellchen wrote: > It's sad that there's not a single codepath that would handle all these, but > maybe the perfect is the enemy of the good here. The code duplication was my reason for looking at the common path. If you're fine with duplicating the code I will go ahead and do that. I have updated the commit with the review comments and added end-to-end tests. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/kleinesfilmroellchen updated
https://github.com/llvm/llvm-project/pull/67749
From 67971ca27ef5e2767aba5cfe2cec1021c4de5ec1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?=
Date: Thu, 25 Jan 2024 18:04:35 +0100
Subject: [PATCH] [clangd] Allow specifying what headers are always included
via "" or <>
Projects can now add config fragments like this to their .clangd:
```yaml
Style:
QuotedHeaders: "src/.*"
AngledHeaders: ["path/sdk/.*", "third-party/.*"]
```
to force headers inserted via the --header-insertion=iwyu mode matching
at least one of the regexes to have <> (AngledHeaders) or ""
(QuotedHeaders) around them, respectively. For other headers (and in
conflicting cases where both styles have a matching regex), the current
system header detection remains.
Ref https://github.com/clangd/clangd/issues/1247
Based on https://reviews.llvm.org/D145843
This solution does not affect other clang tools like clang-format.
---
clang-tools-extra/clangd/CodeComplete.cpp | 15 +++--
clang-tools-extra/clangd/Config.h | 4 ++
clang-tools-extra/clangd/ConfigCompile.cpp| 49
clang-tools-extra/clangd/ConfigFragment.h | 17 ++
clang-tools-extra/clangd/ConfigYAML.cpp | 8 +++
clang-tools-extra/clangd/Headers.cpp | 34 +--
clang-tools-extra/clangd/Headers.h| 10 +++-
clang-tools-extra/clangd/IncludeCleaner.h | 1 -
clang-tools-extra/clangd/ParsedAST.cpp| 3 +-
.../clangd/unittests/CodeCompleteTests.cpp| 56 +++
.../clangd/unittests/ConfigCompileTests.cpp | 38 +
.../clangd/unittests/ConfigYAMLTests.cpp | 8 ++-
.../clangd/unittests/HeadersTests.cpp | 29 +-
13 files changed, 244 insertions(+), 28 deletions(-)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 0e5f08cec440ce..419bd659898506 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -786,8 +787,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext
,
llvm::StringRef SpelledSpecifier = Lexer::getSourceText(
CharSourceRange::getCharRange(SemaSpecifier->getRange()),
CCSema.SourceMgr, clang::LangOptions());
- if (SpelledSpecifier.consume_front("::"))
- Scopes.QueryScopes = {""};
+ if (SpelledSpecifier.consume_front("::"))
+Scopes.QueryScopes = {""};
Scopes.UnresolvedQualifier = std::string(SpelledSpecifier);
// Sema excludes the trailing "::".
if (!Scopes.UnresolvedQualifier->empty())
@@ -1580,7 +1581,7 @@ class CodeCompleteFlow {
CompletionPrefix HeuristicPrefix;
std::optional Filter; // Initialized once Sema runs.
Range ReplacedRange;
- std::vector QueryScopes; // Initialized once Sema runs.
+ std::vector QueryScopes; // Initialized once Sema runs.
std::vector AccessibleScopes; // Initialized once Sema runs.
// Initialized once QueryScopes is initialized, if there are scopes.
std::optional ScopeProximity;
@@ -1639,7 +1640,9 @@ class CodeCompleteFlow {
Inserter.emplace(
SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
SemaCCInput.ParseInput.CompileCommand.Directory,
- >CCSema->getPreprocessor().getHeaderSearchInfo());
+ >CCSema->getPreprocessor().getHeaderSearchInfo(),
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
for (const auto : Includes.MainFileIncludes)
Inserter->addExisting(Inc);
@@ -1722,7 +1725,9 @@ class CodeCompleteFlow {
auto Style = getFormatStyleForFile(FileName, Content, TFS);
// This will only insert verbatim headers.
Inserter.emplace(FileName, Content, Style,
- /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+ /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
auto Identifiers = collectIdentifiers(Content, Style);
std::vector IdentifierResults;
diff --git a/clang-tools-extra/clangd/Config.h
b/clang-tools-extra/clangd/Config.h
index 4371c80a6c5877..3559591fce00cc 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -123,6 +123,10 @@ struct Config {
// declarations, always spell out the whole name (with or without leading
// ::). All nested namespaces are affected as well.
std::vector FullyQualifiedNamespaces;
+
+// List of matcher functions for inserting certain headers with <> or "".
+std::vector> QuotedHeaders;
+std::vector> AngledHeaders;
} Style;
/// Configures code completion feature.
diff --git
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
kleinesfilmroellchen wrote: > It's sad that there's not a single codepath that would handle all these, but > maybe the perfect is the enemy of the good here. The code duplication was my reason for looking at the common path. If you're fine with duplicating the code I will go ahead and do that. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -211,10 +214,12 @@ class IncludeInserter {
// include path of non-verbatim header will not be shortened.
IncludeInserter(StringRef FileName, StringRef Code,
const format::FormatStyle , StringRef BuildDir,
- HeaderSearch *HeaderSearchInfo)
+ HeaderSearch *HeaderSearchInfo, HeaderFilter QuotedHeaders,
kleinesfilmroellchen wrote:
I'm not sure how much difference that makes. The ArrayRef usage is based on
other code I've seen around, and while the config lives for a while, the
IncludeInserter is only briefly used to add an include.
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -296,6 +296,19 @@ struct Fragment {
// ::). All nested namespaces are affected as well.
// Affects availability of the AddUsing tweak.
std::vector> FullyQualifiedNamespaces;
+
+/// List of regexes for headers that should always be included with a
+/// ""-style include. By default, and in case of a conflict with
+/// AngledHeaders (i.e. a header matches a regex in both QuotedHeaders and
+/// AngledHeaders), system headers use <> and non-system headers use "".
+/// These can match any suffix of the header file in question.
kleinesfilmroellchen wrote:
We're matching against the spelling; at least in SerenityOS this is a necessary
distinction: Naming a same-directory header via `"Widget.h"` will result in
quotes, but naming the same header via an "absolute" path like
`` should get you an angled include.
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -296,6 +296,19 @@ struct Fragment {
// ::). All nested namespaces are affected as well.
// Affects availability of the AddUsing tweak.
std::vector> FullyQualifiedNamespaces;
+
+/// List of regexes for headers that should always be included with a
+/// ""-style include. By default, and in case of a conflict with
+/// AngledHeaders (i.e. a header matches a regex in both QuotedHeaders and
+/// AngledHeaders), system headers use <> and non-system headers use "".
+/// These can match any suffix of the header file in question.
sam-mccall wrote:
This should mention whether we're matching against the path to the header, or
its spelling.
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/sam-mccall commented: Sorry about inactivity on my part (here and in general). Thanks for working on this, and thanks @HighCommander4 as always - will leave LGTM to you. Landing this without include-cleaner support is OK with me, though a bit sad. I'm not sure I see the difficulty though: IncludeCleaner.cpp has `generateMissingIncludeDiagnostics()` which already takes the IgnoreHeaders filter, and decides whether to spell with angles or quotes based on the include-cleaner library's suggested spelling (`Angled`). Can we just override that based on the QuotedHeaders etc filters? That wouldn't validate whether the include can actually be included with the flipped quoting style, but it almost always is, and ISTM the other paths also don't validate this. It's sad that there's not a single codepath that would handle all these, but maybe the perfect is the enemy of the good here. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/sam-mccall edited https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/HighCommander4 edited https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -123,6 +123,10 @@ struct Config {
// declarations, always spell out the whole name (with or without leading
// ::). All nested namespaces are affected as well.
std::vector FullyQualifiedNamespaces;
+
+// List of regexes for inserting certain headers with <> or "".
HighCommander4 wrote:
"regexes" --> "matcher functions"
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -539,6 +539,42 @@ TEST_F(ConfigCompileTests, Style) {
Frag.Style.FullyQualifiedNamespaces.push_back(std::string("bar"));
EXPECT_TRUE(compileAndApply());
EXPECT_THAT(Conf.Style.FullyQualifiedNamespaces, ElementsAre("foo", "bar"));
+
+ {
+Frag = {};
+EXPECT_TRUE(Conf.Style.QuotedHeaders.empty())
+<< Conf.Style.QuotedHeaders.size();
+Frag.Style.QuotedHeaders.push_back(Located("foo.h"));
+Frag.Style.QuotedHeaders.push_back(Located(".*inc"));
+EXPECT_TRUE(compileAndApply());
+auto HeaderFilter = [this](llvm::StringRef Path) {
+ for (auto : Conf.Style.QuotedHeaders) {
+if (Filter(Path))
+ return true;
+ }
+ return false;
+};
+EXPECT_TRUE(HeaderFilter("foo.h"));
HighCommander4 wrote:
consider checking `"prefix/foo.h"` as well
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H +#include "Config.h" HighCommander4 wrote: Nothing in this header uses `Config.h` https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -211,10 +214,12 @@ class IncludeInserter {
// include path of non-verbatim header will not be shortened.
IncludeInserter(StringRef FileName, StringRef Code,
const format::FormatStyle , StringRef BuildDir,
- HeaderSearch *HeaderSearchInfo)
+ HeaderSearch *HeaderSearchInfo, HeaderFilter QuotedHeaders,
HighCommander4 wrote:
We're taking `ArrayRef`s here, and passing it `vectors` that live in `Config`.
How confident are we that this is ok from a lifetime point of view, i.e. the
`Config` object will outlive the `IncludeInserter` object? How confident are we
that this will remain the case across future refactorings?
Would it be better to copy the vector instead? Each element is a function which
stores a shared_ptr to a vector of regexes, so we wouldn't be copying the
regexes the themselves.
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/HighCommander4 commented: I took a pass through the implementation. Generally looks good. Would be nice to have a test that exercises the include insertion process in a more end-to-end way. I think something in `CodeCompleteTests.cpp` would work, using `WithContextValue` to set config values ([example](https://searchfox.org/llvm/rev/365aa1574a1b4a3cdee6648227d095d00536ffde/clang-tools-extra/clangd/unittests/InlayHintTests.cpp#77-84,100)), and using the [`insertIncludeText`](https://searchfox.org/llvm/rev/365aa1574a1b4a3cdee6648227d095d00536ffde/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp#83) matcher to make assertions about the spelling of inserted includes. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/HighCommander4 edited https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
kleinesfilmroellchen wrote: After some discussion, I would like to land the feature "partially" as-is without support for "include cleaner" header insertion. (Case 3 in @HighCommander4's elaboration above.) This is motivated by two factors: - Having looked at a way of supporting all three insertion cases, it seems like some significant refactoring is required. The common code between the three cases is either upstream in Clang (`HeaderSearch`), which would probably be a *very* leaky and unclean abstraction. Or downstream in replacement generation (`HeaderIncludes`), where the relevant information to make the decision is no longer available. I'm honestly not excited about performing such a refactor, as I am not particularly familiar with the complex code base. - The other cases seem to be more common in practice, and have already been very useful in my dogfooding with SerenityOS. Within our project this patch would already be appreciated as a great improvement over the status quo. If maintainers agree, I'd like to have this merged before 18. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
HighCommander4 wrote: > @HighCommander4 @sam-mccall do you have any capacity to review this change? I'm happy to try and help, but I haven't really been involved in the development of the include-cleaner component, and based on the findings in my previous comment, the patch would benefit from feedback from someone more familiar with that, to provide guidance about where in the code the config option should be checked to handle the include-cleaner scenario as well. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
HighCommander4 wrote:
> > > What is the relationship between this patch, and clangd 17's ["missing
> > > include"
> > > warning](https://clangd.llvm.org/guides/include-cleaner#missing-include-warning)?
> > > Does the quick-fix for the "missing include" warning also respect these
> > > config options?
>
> As far as I can tell, clangd uses the correct header style for inserted
> includes regardles of how those includes are created (via auto-include on
> code completion or fix suggestions).
I played around with this a bit, and discovered that there are actually
**three** situations in which clangd inserts a header:
1. During code completion ("header insertion")
2. In a quick-fix for an **error** diagnostic on an unresolved name ("include
fixer")
3. In a quick-fix for a **warning** diagnostic (technically, a diagnostic with
severity
[`DiagnosticSeverity.Information`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticSeverity))
on a name that's resolved, but where the file declaring it is not _directly_
included ("include cleaner")
The patch seems to handle the first two, but not the third.
The third scenario can be tested by creating a workspace with the following
files:
.clangd:
```yaml
CompileFlags:
Add: -I.
Diagnostics:
MissingIncludes: Strict
Style:
QuotedHeaders: "quoted/.*"
AngledHeaders: "angled/.*"
```
quoted/quoted.hpp:
```c++
#ifndef QUOTED_HPP_
#define QUOTED_HPP_
void quoted();
#endif
```
angled/angled.hpp:
```c++
#ifndef ANGLED_HPP_
#define ANGLED_HPP_
void angled();
#endif
```
indirect.hpp:
```c++
#include "quoted/quoted.hpp"
#include
```
main.cpp:
```c++
#include "indirect.hpp"
int main() {
quoted();
angled();
}
```
Now, in main.cpp, if you hover over the `angled` token and accept the quick fix
proposed there, the added include is spelled `"angled/angled.hpp"` rather than
``.
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
ADKaster wrote: @HighCommander4 @sam-mccall do you have any capacity to review this change? It would be nice to get it into the 18 release if the window hasn't closed. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/kleinesfilmroellchen updated
https://github.com/llvm/llvm-project/pull/67749
From 8a2b22e69f82e3f4caa271b57b998d1c03b21d39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?=
Date: Thu, 4 Jan 2024 11:53:23 +0100
Subject: [PATCH] [clangd] Allow specifying what headers are always included
via "" or <>
Projects can now add config fragments like this to their .clangd:
```yaml
Style:
QuotedHeaders: "src/.*"
AngledHeaders: ["path/sdk/.*", "third-party/.*"]
```
to force headers inserted via the --header-insertion=iwyu mode matching
at least one of the regexes to have <> (AngledHeaders) or ""
(QuotedHeaders) around them, respectively. For other headers (and in
conflicting cases where both styles have a matching regex), the current
system header detection remains.
Ref https://github.com/clangd/clangd/issues/1247
Based on https://reviews.llvm.org/D145843
This solution does not affect other clang tools like clang-format.
---
clang-tools-extra/clangd/CodeComplete.cpp | 15 --
clang-tools-extra/clangd/Config.h | 4 ++
clang-tools-extra/clangd/ConfigCompile.cpp| 49 +++
clang-tools-extra/clangd/ConfigFragment.h | 13 +
clang-tools-extra/clangd/ConfigYAML.cpp | 8 +++
clang-tools-extra/clangd/Headers.cpp | 34 +++--
clang-tools-extra/clangd/Headers.h| 11 -
clang-tools-extra/clangd/IncludeCleaner.h | 1 -
clang-tools-extra/clangd/ParsedAST.cpp| 3 +-
.../clangd/unittests/ConfigCompileTests.cpp | 36 ++
.../clangd/unittests/ConfigYAMLTests.cpp | 8 ++-
.../clangd/unittests/HeadersTests.cpp | 29 +--
12 files changed, 193 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 0e5f08cec440ce..419bd659898506 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -786,8 +787,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext
,
llvm::StringRef SpelledSpecifier = Lexer::getSourceText(
CharSourceRange::getCharRange(SemaSpecifier->getRange()),
CCSema.SourceMgr, clang::LangOptions());
- if (SpelledSpecifier.consume_front("::"))
- Scopes.QueryScopes = {""};
+ if (SpelledSpecifier.consume_front("::"))
+Scopes.QueryScopes = {""};
Scopes.UnresolvedQualifier = std::string(SpelledSpecifier);
// Sema excludes the trailing "::".
if (!Scopes.UnresolvedQualifier->empty())
@@ -1580,7 +1581,7 @@ class CodeCompleteFlow {
CompletionPrefix HeuristicPrefix;
std::optional Filter; // Initialized once Sema runs.
Range ReplacedRange;
- std::vector QueryScopes; // Initialized once Sema runs.
+ std::vector QueryScopes; // Initialized once Sema runs.
std::vector AccessibleScopes; // Initialized once Sema runs.
// Initialized once QueryScopes is initialized, if there are scopes.
std::optional ScopeProximity;
@@ -1639,7 +1640,9 @@ class CodeCompleteFlow {
Inserter.emplace(
SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
SemaCCInput.ParseInput.CompileCommand.Directory,
- >CCSema->getPreprocessor().getHeaderSearchInfo());
+ >CCSema->getPreprocessor().getHeaderSearchInfo(),
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
for (const auto : Includes.MainFileIncludes)
Inserter->addExisting(Inc);
@@ -1722,7 +1725,9 @@ class CodeCompleteFlow {
auto Style = getFormatStyleForFile(FileName, Content, TFS);
// This will only insert verbatim headers.
Inserter.emplace(FileName, Content, Style,
- /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+ /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
auto Identifiers = collectIdentifiers(Content, Style);
std::vector IdentifierResults;
diff --git a/clang-tools-extra/clangd/Config.h
b/clang-tools-extra/clangd/Config.h
index 4371c80a6c5877..12ed1420c93e19 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -123,6 +123,10 @@ struct Config {
// declarations, always spell out the whole name (with or without leading
// ::). All nested namespaces are affected as well.
std::vector FullyQualifiedNamespaces;
+
+// List of regexes for inserting certain headers with <> or "".
+std::vector> QuotedHeaders;
+std::vector> AngledHeaders;
} Style;
/// Configures code completion feature.
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
kleinesfilmroellchen wrote: > > What is the relationship between this patch, and clangd 17's ["missing > > include" > > warning](https://clangd.llvm.org/guides/include-cleaner#missing-include-warning)? > > Does the quick-fix for the "missing include" warning also respect these > > config options? > > Same question from me -- this is the main reason I personally care about this > PR :) We have directories in our project tree that we'd like to be included > as `#include ` (as those would be installed system-wide in an > open-source version) and others that we'd like to be included as `#include > "dir/foo.h"` (internal headers that are in the same subtree as the C++ > source) and I'd really like clangd in vscode to suggest fixing the missing > include warnings appropriately. Thanks! As far as I can tell, clangd uses the correct header style for inserted includes regardles of how those includes are created (via auto-include on code completion or fix suggestions). https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
tudor wrote: > What is the relationship between this patch, and clangd 17's ["missing > include" > warning](https://clangd.llvm.org/guides/include-cleaner#missing-include-warning)? > Does the quick-fix for the "missing include" warning also respect these > config options? Same question from me -- this is the main reason I personally care about this PR :) We have directories in our project tree that we'd like to be included as `#include ` (as those would be installed system-wide in an open-source version) and others that we'd like to be included as `#include "dir/foo.h"` (internal headers that are in the same subtree as the C++ source) and I'd really like clangd in vscode to suggest fixing the missing include warnings appropriately. Thanks! https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
HighCommander4 wrote: What is the relationship between this patch, and clangd 17's ["missing include" warning](https://clangd.llvm.org/guides/include-cleaner#missing-include-warning)? Does the quick-fix for the "missing include" warning also respect these config options? https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -483,6 +483,56 @@ struct FragmentCompiler {
FullyQualifiedNamespaces.begin(), FullyQualifiedNamespaces.end());
});
}
+auto QuotedFilter = compileHeaderRegexes(F.QuotedHeaders);
+if (QuotedFilter.has_value()) {
+ Out.Apply.push_back(
+ [QuotedFilter = *QuotedFilter](const Params &, Config ) {
+C.Style.QuotedHeaders.emplace_back(QuotedFilter);
+ });
+}
+auto AngledFilter = compileHeaderRegexes(F.AngledHeaders);
+if (AngledFilter.has_value()) {
+ Out.Apply.push_back(
+ [AngledFilter = *AngledFilter](const Params &, Config ) {
+C.Style.AngledHeaders.emplace_back(AngledFilter);
+ });
+}
+ }
+
+ auto compileHeaderRegexes(llvm::ArrayRef>
HeaderPatterns)
+ -> std::optional> {
+// TODO: Share this code with Diagnostics.Includes.IgnoreHeader
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+static llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#else
+static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
+#endif
+auto Filters = std::make_shared>();
+for (auto : HeaderPatterns) {
+ elog("found angle pattern {0}", *HeaderPattern);
kleinesfilmroellchen wrote:
Oops, that's not supposed to be there anymore
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -483,6 +483,56 @@ struct FragmentCompiler {
FullyQualifiedNamespaces.begin(), FullyQualifiedNamespaces.end());
});
}
+auto QuotedFilter = compileHeaderRegexes(F.QuotedHeaders);
+if (QuotedFilter.has_value()) {
+ Out.Apply.push_back(
+ [QuotedFilter = *QuotedFilter](const Params &, Config ) {
+C.Style.QuotedHeaders.emplace_back(QuotedFilter);
+ });
+}
+auto AngledFilter = compileHeaderRegexes(F.AngledHeaders);
+if (AngledFilter.has_value()) {
+ Out.Apply.push_back(
+ [AngledFilter = *AngledFilter](const Params &, Config ) {
+C.Style.AngledHeaders.emplace_back(AngledFilter);
+ });
+}
+ }
+
+ auto compileHeaderRegexes(llvm::ArrayRef>
HeaderPatterns)
+ -> std::optional> {
+// TODO: Share this code with Diagnostics.Includes.IgnoreHeader
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+static llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#else
+static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
+#endif
+auto Filters = std::make_shared>();
+for (auto : HeaderPatterns) {
+ elog("found angle pattern {0}", *HeaderPattern);
ADKaster wrote:
elog seems a bit high for this debugging output.
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/kleinesfilmroellchen updated
https://github.com/llvm/llvm-project/pull/67749
From 2e3f397ffc8967fdc13ced1ec0c4a7dc55a2400d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?=
Date: Mon, 2 Oct 2023 17:51:16 +0200
Subject: [PATCH] [clangd] Allow specifying what headers are always included
via "" or <>
Projects can now add config fragments like this to their .clangd:
```yaml
Style:
QuotedHeaders: "src/.*"
AngledHeaders: ["path/sdk/.*", "third-party/.*"]
```
to force headers inserted via the --header-insertion=iwyu mode matching
at least one of the regexes to have <> (AngledHeaders) or ""
(QuotedHeaders) around them, respectively. For other headers (and in
conflicting cases where both styles have a matching regex), the current
system header detection remains.
Ref https://github.com/clangd/clangd/issues/1247
Based on https://reviews.llvm.org/D145843
This solution does not affect other clang tools like clang-format.
---
clang-tools-extra/clangd/CodeComplete.cpp | 15 --
clang-tools-extra/clangd/Config.h | 4 ++
clang-tools-extra/clangd/ConfigCompile.cpp| 50 +++
clang-tools-extra/clangd/ConfigFragment.h | 13 +
clang-tools-extra/clangd/ConfigYAML.cpp | 8 +++
clang-tools-extra/clangd/Headers.cpp | 34 +++--
clang-tools-extra/clangd/Headers.h| 11 +++-
clang-tools-extra/clangd/IncludeCleaner.h | 1 -
clang-tools-extra/clangd/ParsedAST.cpp| 3 +-
.../clangd/unittests/ConfigCompileTests.cpp | 36 +
.../clangd/unittests/ConfigYAMLTests.cpp | 8 ++-
.../clangd/unittests/HeadersTests.cpp | 29 +--
12 files changed, 194 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 70c4d7e65b76db3..388e032e160474a 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -786,8 +787,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext
,
llvm::StringRef SpelledSpecifier = Lexer::getSourceText(
CharSourceRange::getCharRange(SemaSpecifier->getRange()),
CCSema.SourceMgr, clang::LangOptions());
- if (SpelledSpecifier.consume_front("::"))
- Scopes.QueryScopes = {""};
+ if (SpelledSpecifier.consume_front("::"))
+Scopes.QueryScopes = {""};
Scopes.UnresolvedQualifier = std::string(SpelledSpecifier);
// Sema excludes the trailing "::".
if (!Scopes.UnresolvedQualifier->empty())
@@ -1540,7 +1541,7 @@ class CodeCompleteFlow {
CompletionPrefix HeuristicPrefix;
std::optional Filter; // Initialized once Sema runs.
Range ReplacedRange;
- std::vector QueryScopes; // Initialized once Sema runs.
+ std::vector QueryScopes; // Initialized once Sema runs.
std::vector AccessibleScopes; // Initialized once Sema runs.
// Initialized once QueryScopes is initialized, if there are scopes.
std::optional ScopeProximity;
@@ -1599,7 +1600,9 @@ class CodeCompleteFlow {
Inserter.emplace(
SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
SemaCCInput.ParseInput.CompileCommand.Directory,
- >CCSema->getPreprocessor().getHeaderSearchInfo());
+ >CCSema->getPreprocessor().getHeaderSearchInfo(),
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
for (const auto : Includes.MainFileIncludes)
Inserter->addExisting(Inc);
@@ -1682,7 +1685,9 @@ class CodeCompleteFlow {
auto Style = getFormatStyleForFile(FileName, Content, TFS);
// This will only insert verbatim headers.
Inserter.emplace(FileName, Content, Style,
- /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+ /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
auto Identifiers = collectIdentifiers(Content, Style);
std::vector IdentifierResults;
diff --git a/clang-tools-extra/clangd/Config.h
b/clang-tools-extra/clangd/Config.h
index daae8d1c0c833eb..5d70ed3376715e5 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -121,6 +121,10 @@ struct Config {
// declarations, always spell out the whole name (with or without leading
// ::). All nested namespaces are affected as well.
std::vector FullyQualifiedNamespaces;
+
+// List of regexes for inserting certain headers with <> or "".
+std::vector> QuotedHeaders;
+std::vector> AngledHeaders;
} Style;
/// Configures code completion feature.
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
kleinesfilmroellchen wrote: Thanks for the finding @zyn0217, refactored the config compiler code a little. Tests should pass now and I have re-tested this on SerenityOS and other small project where it works as expected in practice. https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/kleinesfilmroellchen updated
https://github.com/llvm/llvm-project/pull/67749
From 167048c7025a516a5e57009947333c3980017107 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?=
Date: Sun, 1 Oct 2023 14:48:57 +0200
Subject: [PATCH] [clangd] Allow specifying what headers are always included
via "" or <>
Projects can now add config fragments like this to their .clangd:
```yaml
Style:
QuotedHeaders: "src/.*"
AngledHeaders: ["path/sdk/.*", "third-party/.*"]
```
to force headers inserted via the --header-insertion=iwyu mode matching
at least one of the regexes to have <> (AngledHeaders) or ""
(QuotedHeaders) around them, respectively. For other headers (and in
conflicting cases where both styles have a matching regex), the current
system header detection remains.
Ref https://github.com/clangd/clangd/issues/1247
Based on https://reviews.llvm.org/D145843
This solution does not affect other clang tools like clang-format.
---
clang-tools-extra/clangd/CodeComplete.cpp | 15 --
clang-tools-extra/clangd/Config.h | 4 ++
clang-tools-extra/clangd/ConfigCompile.cpp| 48 +++
clang-tools-extra/clangd/ConfigFragment.h | 13 +
clang-tools-extra/clangd/ConfigYAML.cpp | 8
clang-tools-extra/clangd/Headers.cpp | 34 +++--
clang-tools-extra/clangd/Headers.h| 11 -
clang-tools-extra/clangd/IncludeCleaner.h | 1 -
clang-tools-extra/clangd/ParsedAST.cpp| 3 +-
.../clangd/unittests/ConfigCompileTests.cpp | 36 ++
.../clangd/unittests/ConfigYAMLTests.cpp | 8 +++-
.../clangd/unittests/HeadersTests.cpp | 29 +--
12 files changed, 192 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 70c4d7e65b76db3..388e032e160474a 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -786,8 +787,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext
,
llvm::StringRef SpelledSpecifier = Lexer::getSourceText(
CharSourceRange::getCharRange(SemaSpecifier->getRange()),
CCSema.SourceMgr, clang::LangOptions());
- if (SpelledSpecifier.consume_front("::"))
- Scopes.QueryScopes = {""};
+ if (SpelledSpecifier.consume_front("::"))
+Scopes.QueryScopes = {""};
Scopes.UnresolvedQualifier = std::string(SpelledSpecifier);
// Sema excludes the trailing "::".
if (!Scopes.UnresolvedQualifier->empty())
@@ -1540,7 +1541,7 @@ class CodeCompleteFlow {
CompletionPrefix HeuristicPrefix;
std::optional Filter; // Initialized once Sema runs.
Range ReplacedRange;
- std::vector QueryScopes; // Initialized once Sema runs.
+ std::vector QueryScopes; // Initialized once Sema runs.
std::vector AccessibleScopes; // Initialized once Sema runs.
// Initialized once QueryScopes is initialized, if there are scopes.
std::optional ScopeProximity;
@@ -1599,7 +1600,9 @@ class CodeCompleteFlow {
Inserter.emplace(
SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
SemaCCInput.ParseInput.CompileCommand.Directory,
- >CCSema->getPreprocessor().getHeaderSearchInfo());
+ >CCSema->getPreprocessor().getHeaderSearchInfo(),
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
for (const auto : Includes.MainFileIncludes)
Inserter->addExisting(Inc);
@@ -1682,7 +1685,9 @@ class CodeCompleteFlow {
auto Style = getFormatStyleForFile(FileName, Content, TFS);
// This will only insert verbatim headers.
Inserter.emplace(FileName, Content, Style,
- /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+ /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
auto Identifiers = collectIdentifiers(Content, Style);
std::vector IdentifierResults;
diff --git a/clang-tools-extra/clangd/Config.h
b/clang-tools-extra/clangd/Config.h
index daae8d1c0c833eb..5d70ed3376715e5 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -121,6 +121,10 @@ struct Config {
// declarations, always spell out the whole name (with or without leading
// ::). All nested namespaces are affected as well.
std::vector FullyQualifiedNamespaces;
+
+// List of regexes for inserting certain headers with <> or "".
+std::vector> QuotedHeaders;
+std::vector> AngledHeaders;
} Style;
/// Configures code completion feature.
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
@@ -483,6 +483,72 @@ struct FragmentCompiler {
FullyQualifiedNamespaces.begin(), FullyQualifiedNamespaces.end());
});
}
+
+// TODO: Share this code with Diagnostics.Includes.IgnoreHeader
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+static llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#else
+static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
+#endif
+{
+ auto Filters = std::make_shared>();
+ for (auto : F.AngledHeaders) {
+// Anchor on the right.
+std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
+llvm::Regex CompiledRegex(AnchoredPattern, Flags);
+std::string RegexError;
+if (!CompiledRegex.isValid(RegexError)) {
+ diag(Warning,
+ llvm::formatv("Invalid regular expression '{0}': {1}",
+ *HeaderPattern, RegexError)
+ .str(),
+ HeaderPattern.Range);
+ continue;
+}
+Filters->push_back(std::move(CompiledRegex));
+ }
+ if (Filters->empty())
+return;
zyn0217 wrote:
I think this is why you're seeing the puzzling behavior: You've bailed out of
parsing the `QuotedHeaders` block if don't see any `AngledHeaders` field.
Perhaps extract these two blocks into separate functions?
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
kleinesfilmroellchen wrote: I investigated the test failures some more in the context of where the bug affects real uses and I'm unsure as to what's going on. ```yaml Style: QuotedHeaders: ["blahblahblah"] AngledHeaders: ["AK/.*", "Userland/.*", "Kernel/.*", "Applications/.*", "Lib.*/.*"] ``` will match all regexes correctly. ```yaml Style: AngledHeaders: ["AK/.*", "Userland/.*", "Kernel/.*", "Applications/.*", "Lib.*/.*"] ``` will make AngledHeaders completely disappear from the parsed and compiled config; apparently the dictionary key doesn't even exist anymore. Exactly this behavior makes a test fail on CI. ```yaml Style: QuotedHeaders: ["blahblahblah"] ``` will keep QuotedHeaders in the parsed and compiled config and works as expected. There is no apparent difference in how these two keys are handled, I have cross-checked my config parser and compiler code with existing code countless times. Really no clue what's happening here other than QuotedHeaders apparently needs to exist or AngledHeaders vanishes. CC @HighCommander4 https://github.com/llvm/llvm-project/pull/67749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
llvmbot wrote:
@llvm/pr-subscribers-clangd
Changes
Projects can now add config fragments like this to their .clangd:
```yaml
Style:
QuotedHeaders: "src/.*"
AngledHeaders: ["path/sdk/.*", "third-party/.*"]
```
to force headers inserted via the --header-insertion=iwyu mode matching at
least one of the regexes to have (AngledHeaders) or "" (QuotedHeaders)
around them, respectively. For other headers (and in conflicting cases where
both styles have a matching regex), the current system header detection remains.
Ref https://github.com/clangd/clangd/issues/1247
Based on https://reviews.llvm.org/D145843
This solution does not affect other clang tools like clang-format.
CC @HighCommander4 @adkaster. I have no merge rights so someone
else needs to commit this.
---
Patch is 20.08 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/67749.diff
12 Files Affected:
- (modified) clang-tools-extra/clangd/CodeComplete.cpp (+10-5)
- (modified) clang-tools-extra/clangd/Config.h (+4)
- (modified) clang-tools-extra/clangd/ConfigCompile.cpp (+66)
- (modified) clang-tools-extra/clangd/ConfigFragment.h (+13)
- (modified) clang-tools-extra/clangd/ConfigYAML.cpp (+8)
- (modified) clang-tools-extra/clangd/Headers.cpp (+29-5)
- (modified) clang-tools-extra/clangd/Headers.h (+9-2)
- (modified) clang-tools-extra/clangd/IncludeCleaner.h (-1)
- (modified) clang-tools-extra/clangd/ParsedAST.cpp (+2-1)
- (modified) clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp (+34)
- (modified) clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp (+7-1)
- (modified) clang-tools-extra/clangd/unittests/HeadersTests.cpp (+26-3)
``diff
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 70c4d7e65b76db3..388e032e160474a 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -786,8 +787,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext
,
llvm::StringRef SpelledSpecifier = Lexer::getSourceText(
CharSourceRange::getCharRange(SemaSpecifier->getRange()),
CCSema.SourceMgr, clang::LangOptions());
- if (SpelledSpecifier.consume_front("::"))
- Scopes.QueryScopes = {""};
+ if (SpelledSpecifier.consume_front("::"))
+Scopes.QueryScopes = {""};
Scopes.UnresolvedQualifier = std::string(SpelledSpecifier);
// Sema excludes the trailing "::".
if (!Scopes.UnresolvedQualifier->empty())
@@ -1540,7 +1541,7 @@ class CodeCompleteFlow {
CompletionPrefix HeuristicPrefix;
std::optional Filter; // Initialized once Sema runs.
Range ReplacedRange;
- std::vector QueryScopes; // Initialized once Sema runs.
+ std::vector QueryScopes; // Initialized once Sema runs.
std::vector AccessibleScopes; // Initialized once Sema runs.
// Initialized once QueryScopes is initialized, if there are scopes.
std::optional ScopeProximity;
@@ -1599,7 +1600,9 @@ class CodeCompleteFlow {
Inserter.emplace(
SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
SemaCCInput.ParseInput.CompileCommand.Directory,
- >CCSema->getPreprocessor().getHeaderSearchInfo());
+ >CCSema->getPreprocessor().getHeaderSearchInfo(),
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
for (const auto : Includes.MainFileIncludes)
Inserter->addExisting(Inc);
@@ -1682,7 +1685,9 @@ class CodeCompleteFlow {
auto Style = getFormatStyleForFile(FileName, Content, TFS);
// This will only insert verbatim headers.
Inserter.emplace(FileName, Content, Style,
- /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+ /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
auto Identifiers = collectIdentifiers(Content, Style);
std::vector IdentifierResults;
diff --git a/clang-tools-extra/clangd/Config.h
b/clang-tools-extra/clangd/Config.h
index daae8d1c0c833eb..5d70ed3376715e5 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -121,6 +121,10 @@ struct Config {
// declarations, always spell out the whole name (with or without leading
// ::). All nested namespaces are affected as well.
std::vector FullyQualifiedNamespaces;
+
+// List of regexes for inserting certain headers with <> or "".
+std::vector> QuotedHeaders;
+std::vector> AngledHeaders;
} Style;
/// Configures code completion feature.
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp
b/clang-tools-extra/clangd/ConfigCompile.cpp
index
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 2f3b7d33f421b723728262a6978041d93f1f8c5c
d2bb8241dfd396c1c77413918f1983b1afe16517 --
clang-tools-extra/clangd/CodeComplete.cpp clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
clang-tools-extra/clangd/unittests/HeadersTests.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang-tools-extra/clangd/Headers.h
b/clang-tools-extra/clangd/Headers.h
index f1ec44422775..e1f3eaa3397b 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -33,7 +33,7 @@
namespace clang {
namespace clangd {
-
+
using HeaderFilter = llvm::ArrayRef>;
/// Returns true if \p Include is literal include like "path" or .
``
https://github.com/llvm/llvm-project/pull/67749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Allow specifying what headers are always included via "" or <> (PR #67749)
https://github.com/kleinesfilmroellchen created
https://github.com/llvm/llvm-project/pull/67749
Projects can now add config fragments like this to their .clangd:
```yaml
Style:
QuotedHeaders: "src/.*"
AngledHeaders: ["path/sdk/.*", "third-party/.*"]
```
to force headers inserted via the --header-insertion=iwyu mode matching at
least one of the regexes to have <> (AngledHeaders) or "" (QuotedHeaders)
around them, respectively. For other headers (and in conflicting cases where
both styles have a matching regex), the current system header detection remains.
Ref https://github.com/clangd/clangd/issues/1247
Based on https://reviews.llvm.org/D145843
This solution does not affect other clang tools like clang-format.
CC @HighCommander4 @adkaster. I have no merge rights so someone else needs to
commit this.
From d2bb8241dfd396c1c77413918f1983b1afe16517 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?=
Date: Fri, 29 Sep 2023 00:22:12 +0200
Subject: [PATCH] [clangd] Allow specifying what headers are always included
via "" or <>
Projects can now add config fragments like this to their .clangd:
```yaml
Style:
QuotedHeaders: "src/.*"
AngledHeaders: ["path/sdk/.*", "third-party/.*"]
```
to force headers inserted via the --header-insertion=iwyu mode matching
at least one of the regexes to have <> (AngledHeaders) or ""
(QuotedHeaders) around them, respectively. For other headers (and in
conflicting cases where both styles have a matching regex), the current
system header detection remains.
Ref https://github.com/clangd/clangd/issues/1247
Based on https://reviews.llvm.org/D145843
This solution does not affect other clang tools like clang-format.
---
clang-tools-extra/clangd/CodeComplete.cpp | 15 +++--
clang-tools-extra/clangd/Config.h | 4 ++
clang-tools-extra/clangd/ConfigCompile.cpp| 66 +++
clang-tools-extra/clangd/ConfigFragment.h | 13
clang-tools-extra/clangd/ConfigYAML.cpp | 8 +++
clang-tools-extra/clangd/Headers.cpp | 34 --
clang-tools-extra/clangd/Headers.h| 11 +++-
clang-tools-extra/clangd/IncludeCleaner.h | 1 -
clang-tools-extra/clangd/ParsedAST.cpp| 3 +-
.../clangd/unittests/ConfigCompileTests.cpp | 34 ++
.../clangd/unittests/ConfigYAMLTests.cpp | 8 ++-
.../clangd/unittests/HeadersTests.cpp | 29 +++-
12 files changed, 208 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 70c4d7e65b76db3..388e032e160474a 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -786,8 +787,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext
,
llvm::StringRef SpelledSpecifier = Lexer::getSourceText(
CharSourceRange::getCharRange(SemaSpecifier->getRange()),
CCSema.SourceMgr, clang::LangOptions());
- if (SpelledSpecifier.consume_front("::"))
- Scopes.QueryScopes = {""};
+ if (SpelledSpecifier.consume_front("::"))
+Scopes.QueryScopes = {""};
Scopes.UnresolvedQualifier = std::string(SpelledSpecifier);
// Sema excludes the trailing "::".
if (!Scopes.UnresolvedQualifier->empty())
@@ -1540,7 +1541,7 @@ class CodeCompleteFlow {
CompletionPrefix HeuristicPrefix;
std::optional Filter; // Initialized once Sema runs.
Range ReplacedRange;
- std::vector QueryScopes; // Initialized once Sema runs.
+ std::vector QueryScopes; // Initialized once Sema runs.
std::vector AccessibleScopes; // Initialized once Sema runs.
// Initialized once QueryScopes is initialized, if there are scopes.
std::optional ScopeProximity;
@@ -1599,7 +1600,9 @@ class CodeCompleteFlow {
Inserter.emplace(
SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
SemaCCInput.ParseInput.CompileCommand.Directory,
- >CCSema->getPreprocessor().getHeaderSearchInfo());
+ >CCSema->getPreprocessor().getHeaderSearchInfo(),
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
for (const auto : Includes.MainFileIncludes)
Inserter->addExisting(Inc);
@@ -1682,7 +1685,9 @@ class CodeCompleteFlow {
auto Style = getFormatStyleForFile(FileName, Content, TFS);
// This will only insert verbatim headers.
Inserter.emplace(FileName, Content, Style,
- /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+ /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+ Config::current().Style.QuotedHeaders,
+ Config::current().Style.AngledHeaders);
auto Identifiers = collectIdentifiers(Content, Style);
std::vector
