[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
HighCommander4 wrote: Given that we haven't heard objections, I think we should proceed with using resolved paths in `IncludeInserter` as well. It would probably be best to do that before the clangd 21 release, so that we don't ship a release where two different features are interpreting the config keys in different ways. @Harald-R, are you interested in writing a patch to use resolved paths in `IncludeInserter`? I'm happy to provide more detailed code pointers if you'd like. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
HighCommander4 wrote: @kleinesfilmroellchen (or anyone else reading this thread), any objection to switching to the approach suggested by Kadir in the previous comment, of matching against resolved paths? https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
kadircet wrote: I am rather worried that we'll lose ability to filter effectively when we match against spelled headers. Maybe the actual use cases in the wild and what I have mind are different, but I would expect users to have a situation like; - they pass a `-Ithird_party/qt/include` in their compile flags - this results in clangd spelling inclusions as `#include "QtWidgets/QWidgets"` - users will want to have all such includes spelled with `<>` instead. - if filter is by resolved path, they can just create filters corresponding to include search paths, i.e. `AngledHeaders: third_party/qt/include/.*` - if filter is by spelling instead either the user needs to overgeneralize and have a filter like `Qt.*/.*` or spell multiple directory names like `QtWidgets/.*`, `QtCharts/.*`, .. the situation gets more problematic when the library is flat in hierarchy without any directory names. Hence I'd still lean towards matching against resolved path instead, I'd also like to highlight that we're matching against a `suffix` of the resolved path. Hence it also covers regexes that just match spelled headers(except objc frameworks, but they're very explicit from compiler flags already), but it gives people more precision when needed. FWIW, clang-include-cleaner's header filters are also based on matching against suffix of a resolved path. As for `either absolute path or path relative to the execution`, this is mostly for the cases where a header is referring to some source file outside of the project (e.g. some system-installed library), that being said path handling is hard and it can at times refer to in-project sources through absolute paths as well. But in general I think this shouldn't matter, as we're matching against suffix of that path. P.S, when matching against spellings, we're also matching against a suffix of those spellings today. It might be worthwhile to re-consider that too if we're going down that path. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
Harald-R wrote: Created https://github.com/llvm/llvm-project/pull/143411 to do the filtering based on the spelling instead of the resolved path. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
Harald-R wrote: @HighCommander4 thank you for the background! To me it sounds like going back to matching against the spelling makes sense. I can submit a patch later today if that's okay with @kadircet as well. Otherwise, this PR could be reverted and I can resubmit it later without the spelling check. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
HighCommander4 wrote: Apologies for not having had a chance to circle back to this sooner. I read over the discussion of the [original patch](https://github.com/llvm/llvm-project/pull/67749) that added support for the `QuotedHeaders` and `AngledHeaders` config keys, and it turns out the "resolved path vs. spelling" question does have a back-story. @sam-mccall [commented](https://github.com/llvm/llvm-project/pull/67749#discussion_r1465335864) during that review: > This should mention whether we're matching against the path to the header, or > its spelling. and @kleinesfilmroellchen [responded](https://github.com/llvm/llvm-project/pull/67749#discussion_r1466440861): > 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. This is what the patch ended up implementing, and documenting [in `ConfigFragment.h`](https://searchfox.org/llvm/rev/55c86c5f77437c15fd29936cc8ad48b2097660b3/clang-tools-extra/clangd/ConfigFragment.h#318-319): ```c++ /// Matching is performed against the header text, not its absolute path /// within the project. ``` So, currently there is an inconsistency between the documentation and the `IncludeInserter` implementation on the one hand (uses the spelling), and the include-cleaner implementation on the other (uses the resolved path). I guess we should resolve that inconsistency, in one direction or another. In addition to the matter of accommodating project styles, one potential challenge I see with using the resolved path is that the documentation of `include_cleaner::Header::resolvedPath()` [states](https://searchfox.org/llvm/rev/55c86c5f77437c15fd29936cc8ad48b2097660b3/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h#139-140): > /// For phiscal files, **either absolute path or path relative to the > execution > /// root**. Otherwise just the spelling without surrounding quotes/brackets. Having to write regexes that match an "either/or" seems like a bit of a headache for users. @kadircet, thoughts on how to proceed? https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/Harald-R updated
https://github.com/llvm/llvm-project/pull/140594
>From aa1a523ff10ce6176c8f9244bbc76340ae463097 Mon Sep 17 00:00:00 2001
From: Harald-R
Date: Sun, 18 May 2025 19:07:59 +0300
Subject: [PATCH 1/9] Follow style configuration in clangd include cleaner
---
clang-tools-extra/clangd/IncludeCleaner.cpp | 21 ---
clang-tools-extra/clangd/IncludeCleaner.h | 9
clang-tools-extra/clangd/ParsedAST.cpp| 3 ++-
.../clangd/unittests/IncludeCleanerTests.cpp | 15 +++--
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 9e1f6bb977226..4fedea35334f6 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS,
-Cfg.Diagnostics.Includes.IgnoreHeader);
+Cfg.Diagnostics.Includes.IgnoreHeader,
+Cfg.Style.AngledHeaders);
}
tidy::ClangTidyCheckFactories
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 0ee748c1ed2d0..fda1b
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/Harald-R updated
https://github.com/llvm/llvm-project/pull/140594
>From 2e261ba9fa08f7c482f9c9e11385d0975468b76f Mon Sep 17 00:00:00 2001
From: Harald-R
Date: Sun, 18 May 2025 19:07:59 +0300
Subject: [PATCH 1/9] Follow style configuration in clangd include cleaner
---
clang-tools-extra/clangd/IncludeCleaner.cpp | 21 ---
clang-tools-extra/clangd/IncludeCleaner.h | 9
clang-tools-extra/clangd/ParsedAST.cpp| 3 ++-
.../clangd/unittests/IncludeCleanerTests.cpp | 15 +++--
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 9e1f6bb977226..4fedea35334f6 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS,
-Cfg.Diagnostics.Includes.IgnoreHeader);
+Cfg.Diagnostics.Includes.IgnoreHeader,
+Cfg.Style.AngledHeaders);
}
tidy::ClangTidyCheckFactories
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 0ee748c1ed2d0..fda1b
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
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 HEAD~1 HEAD --extensions h,cpp --
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 7034685e6..ec733cbe9 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -282,7 +282,8 @@ $insert_vector[[]]
TU.AdditionalFiles["system/e.h"] = guard("#include ");
TU.AdditionalFiles["system/f.h"] = guard("void f();");
- TU.AdditionalFiles["system/quoted2_wrapper.h"] = guard("#include
");
+ TU.AdditionalFiles["system/quoted2_wrapper.h"] =
+ guard("#include ");
TU.AdditionalFiles["system/quoted2.h"] = guard("void quoted2();");
TU.ExtraArgs.push_back("-isystem" + testPath("system"));
@@ -340,12 +341,12 @@ $insert_vector[[]]
withFix({Fix(MainFile.range("insert_quoted"),
"#include \"quoted.h\"\n", "#include \"quoted.h\""),
FixMessage("add all missing includes")})),
- AllOf(
- Diag(MainFile.range("quoted2"),
- "No header providing \"quoted2\" is directly included"),
- withFix({Fix(MainFile.range("insert_quoted2"),
- "#include \"quoted2.h\"\n", "#include
\"quoted2.h\""),
- FixMessage("add all missing includes")})),
+ AllOf(Diag(MainFile.range("quoted2"),
+ "No header providing \"quoted2\" is directly included"),
+withFix(
+{Fix(MainFile.range("insert_quoted2"),
+ "#include \"quoted2.h\"\n", "#include \"quoted2.h\""),
+ FixMessage("add all missing includes")})),
AllOf(Diag(MainFile.range("bar"),
"No header providing \"ns::Bar\" is directly included"),
withFix({Fix(MainFile.range("insert_d"),
``
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -151,6 +164,11 @@ std::vector generateMissingIncludeDiagnostics(
if (!Replacement.has_value())
continue;
+if (Angled && Spelling.front() == '\"') {
Harald-R wrote:
@kadircet you are right. Forgot to update the spelling fix when I changed the
implementation to include the quoted filter. Fixed and added a test that forces
a system include to use the quoted notation.
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/Harald-R updated
https://github.com/llvm/llvm-project/pull/140594
>From 2e261ba9fa08f7c482f9c9e11385d0975468b76f Mon Sep 17 00:00:00 2001
From: Harald-R
Date: Sun, 18 May 2025 19:07:59 +0300
Subject: [PATCH 1/8] Follow style configuration in clangd include cleaner
---
clang-tools-extra/clangd/IncludeCleaner.cpp | 21 ---
clang-tools-extra/clangd/IncludeCleaner.h | 9
clang-tools-extra/clangd/ParsedAST.cpp| 3 ++-
.../clangd/unittests/IncludeCleanerTests.cpp | 15 +++--
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 9e1f6bb977226..4fedea35334f6 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS,
-Cfg.Diagnostics.Includes.IgnoreHeader);
+Cfg.Diagnostics.Includes.IgnoreHeader,
+Cfg.Style.AngledHeaders);
}
tidy::ClangTidyCheckFactories
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 0ee748c1ed2d0..fda1b
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -151,6 +164,11 @@ std::vector generateMissingIncludeDiagnostics(
if (!Replacement.has_value())
continue;
+if (Angled && Spelling.front() == '\"') {
kadircet wrote:
yikes, we also need to do this transformation in the other way as well :/
Might be easier to:
```cpp
if (Angled != (Spelling.front() == '<')) {
Spelling.front() = Angled ? '<' : '"';
Spelling.back() = Angled ? '>' : '"';
}
```
(I guess we're also lacking some tests for the quoted direction)
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/kadircet approved this pull request. thanks, mostly LGTM, please wait on a final look from @HighCommander4 though, he has more context about this feature overall, maybe the discrepancy on path vs spelling based match has a story. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -141,16 +143,41 @@ std::vector generateMissingIncludeDiagnostics(
AST.getPreprocessor().getHeaderSearchInfo(), MainFile});
llvm::StringRef HeaderRef{Spelling};
-bool Angled = HeaderRef.starts_with("<");
+
+bool IsAngled = false;
Harald-R wrote:
Made the change:
https://github.com/llvm/llvm-project/pull/140594/commits/6739bd8ec96fe0d47e7566e459a9c2d1b433e0a8
If this PR gets merged, I will submit another one for fixing the other instance
in Headers.cpp.
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/Harald-R updated
https://github.com/llvm/llvm-project/pull/140594
>From 322468b6354c45c0c81aa3ae28b900262d795a8e Mon Sep 17 00:00:00 2001
From: Harald-R
Date: Sun, 18 May 2025 19:07:59 +0300
Subject: [PATCH 1/7] Follow style configuration in clangd include cleaner
---
clang-tools-extra/clangd/IncludeCleaner.cpp | 21 ---
clang-tools-extra/clangd/IncludeCleaner.h | 9
clang-tools-extra/clangd/ParsedAST.cpp| 3 ++-
.../clangd/unittests/IncludeCleanerTests.cpp | 15 +++--
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 9e1f6bb977226..4fedea35334f6 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS,
-Cfg.Diagnostics.Includes.IgnoreHeader);
+Cfg.Diagnostics.Includes.IgnoreHeader,
+Cfg.Style.AngledHeaders);
}
tidy::ClangTidyCheckFactories
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 0ee748c1ed2d0..fda1b
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -141,16 +143,41 @@ std::vector generateMissingIncludeDiagnostics(
AST.getPreprocessor().getHeaderSearchInfo(), MainFile});
llvm::StringRef HeaderRef{Spelling};
-bool Angled = HeaderRef.starts_with("<");
+
+bool IsAngled = false;
kadircet wrote:
we shouldn't be matching against `HeaderRef`, but instead matching against
`ResolvedPath` and only when `SymbolWithMissingInclude.Providers.front().kind()
== Physical`
nit:
```cpp
bool Angled = HeaderRef.starts_with("<");
if (SymbolWithMissingInclude.Providers.front().kind() == ..Physical) {
for (auto &Filter : Angled ? QuotedHeaders : AngledHeaders) {
if (Filter(ResolvedPath)) {
Angled = !Angled;
break;
}
}
}
std::optional Replacement = HeaderIncludes.insert(
HeaderRef.trim("\"<>"), Angled, tooling::IncludeDirective::Include);
```
[logic in the include
inserter](https://github.com/llvm/llvm-project/blob/3033f202f6707937cd28c2473479db134993f96f/clang-tools-extra/clangd/Headers.cpp#L307)
seem to have the same bug (we should match against paths, not spellings).
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/kadircet requested changes to this pull request. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
Harald-R wrote: @HighCommander4 @kadircet many thanks for the reviews! Added a commit which takes the `QuotedHeaders` filter into account: https://github.com/llvm/llvm-project/pull/140594/commits/7564e9cd65421b5c7e3c6428f7f073fa6f5e3ec5. There is some duplication with the [logic in the include inserter](https://github.com/llvm/llvm-project/blob/3033f202f6707937cd28c2473479db134993f96f/clang-tools-extra/clangd/Headers.cpp#L307); not sure if this should be solved now or via a dedicated refactoring of the inclusion logic in all places, as was proposed [here](https://github.com/llvm/llvm-project/pull/67749#issuecomment-1904680030). https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -262,6 +264,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
TU.Filename = "main.cpp";
TU.AdditionalFiles["a.h"] = guard("#include \"b.h\"");
TU.AdditionalFiles["b.h"] = guard("void b();");
+ TU.AdditionalFiles["a_angled.h"] = guard("#include \"b_angled.h\"");
Harald-R wrote:
I like the proposed suggestion. Made the changes:
https://github.com/llvm/llvm-project/pull/140594/commits/8f568794c39851868734bf2c08b79d802e8d0ce4
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/Harald-R updated
https://github.com/llvm/llvm-project/pull/140594
>From 3b293aa3af6d5755bfd2a83c20726ec8fb871bba Mon Sep 17 00:00:00 2001
From: Harald-R
Date: Sun, 18 May 2025 19:07:59 +0300
Subject: [PATCH 1/6] Follow style configuration in clangd include cleaner
---
clang-tools-extra/clangd/IncludeCleaner.cpp | 21 ---
clang-tools-extra/clangd/IncludeCleaner.h | 9
clang-tools-extra/clangd/ParsedAST.cpp| 3 ++-
.../clangd/unittests/IncludeCleanerTests.cpp | 15 +++--
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 9e1f6bb977226..4fedea35334f6 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS,
-Cfg.Diagnostics.Includes.IgnoreHeader);
+Cfg.Diagnostics.Includes.IgnoreHeader,
+Cfg.Style.AngledHeaders);
}
tidy::ClangTidyCheckFactories
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 0ee748c1ed2d0..fda1b
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -262,6 +264,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
TU.Filename = "main.cpp";
TU.AdditionalFiles["a.h"] = guard("#include \"b.h\"");
TU.AdditionalFiles["b.h"] = guard("void b();");
+ TU.AdditionalFiles["a_angled.h"] = guard("#include \"b_angled.h\"");
HighCommander4 wrote:
```c++
TU.ExtraArgs.push_back("-I" + testPath("."));
```
should be sufficient to get `#include ` to compile.
Will leave the naming up to you as it's a minor point. (Possible suggestion:
`b_angled` --> `angled`, and `a_angled` --> `angled_wrapper`?)
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/HighCommander4 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
HighCommander4 wrote: > > But since we have both config options, maybe there is a reason for > > overwrites in this direction as well. > > Hmm, I was mainly focusing on the quoted -> angled case, hence why only the > `AngledHeaders` filter is used. But I have to agree that following the config > fully makes sense. I see other instances, like > `IncludeInserter::calculateIncludePath` do make use of both filters. I can > make the change, to overwrite the inclusion style when the configs dictate > that, if that is the desired behavior here. +1, it makes sense to check both filters. Thanks Kadir for catching that. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -297,7 +301,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
Findings.UnusedIncludes.clear();
std::vector Diags = issueIncludeCleanerDiagnostics(
AST, TU.Code, Findings, MockFS(),
- {[](llvm::StringRef Header) { return Header.ends_with("buzz.h"); }});
+ {[](llvm::StringRef Header) { return Header.ends_with("buzz.h"); }},
Harald-R wrote:
Done:
https://github.com/llvm/llvm-project/pull/140594/commits/da30c7d30208dfc8e6507f7263995d55fd18e306
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -262,6 +264,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
TU.Filename = "main.cpp";
TU.AdditionalFiles["a.h"] = guard("#include \"b.h\"");
TU.AdditionalFiles["b.h"] = guard("void b();");
+ TU.AdditionalFiles["a_angled.h"] = guard("#include \"b_angled.h\"");
Harald-R wrote:
> if a_angled.h is not part of the Angled filter, it should probably be called
> a_quoted.h (or c.h or something else that doesn't include "angled")
I can rename `a_angled.h` to `a_quoted.h` or something similar, if that helps
avoid confusion. Here I wanted to follow the example with `a.h` and `b.h`,
where `a.h` only includes `b.h` and is used inside the test itself, as it is
necessary for the `TU.build()` call to be successful; otherwise, the `b` symbol
would be undeclared when this `build()` method is called during the test
execution.
Here, the `a_angled.h` and `b_angled.h` headers follow the same pattern, albeit
the names could be reworked. I something like `c.h` is preferred, I can replace
it, if it doesn't bring confusion with the other `dir/c.h` header.
> to avoid confusion, let's make the include style of b_angled.h angled here in
> the contents of a_angled.h as well
Using angled brackets here leads to an error:
```
TestTU failed to build (suppress with /*error-ok*/):
[2:9-2:21] in included file: 'b_angled.h' file not found with include;
use "quotes" instead, notes: {[/clangd-test/a_angled.h:1:9-1:21] error occurred
here}
```
I believe its parent directory would need to be added to the SYSTEM include
path.
Let me know which rename makes most sense and whether this header should be in
the SYSTEM path, and I will push a commit with the changes.
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -306,6 +311,12 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
withFix({Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
"#include \"b.h\""),
FixMessage("add all missing includes")})),
+ AllOf(Diag(MainFile.range("b_angled"),
+ "No header providing \"b_angled\" is directly included"),
+withFix(
+{Fix(MainFile.range("insert_b_angled"),
+ "#include \n", "#include \"b_angled.h\""),
Harald-R wrote:
Good point. I was under the impression that the `Message` field referred to the
original code (which in this test had an instance of quoted inclusion), but it
seems that it refers to the fix itself, so it should be angled as well. I was
mainly testing the code with the example
[here](https://github.com/llvm/llvm-project/pull/67749#issuecomment-1903375837)
and missed this. Made some changes to the way the `Message` field is generated
in the code to reflect the correct inclusion style:
https://github.com/llvm/llvm-project/pull/140594/commits/4896e03140150427f32bca07824dace0612e22e7
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
Harald-R wrote:
Done:
https://github.com/llvm/llvm-project/pull/140594/commits/331bf03fa3bacb0e2beaf9b5d3da3202090b8c1e
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
Harald-R wrote: > But since we have both config options, maybe there is a reason for overwrites > in this direction as well. Hmm, I was mainly focusing on the quoted -> angled case, hence why only the `AngledHeaders` filter is used. But I have to agree that following the config fully makes sense. I see other instances, like `IncludeInserter::calculateIncludePath` do make use of both filters. I can make the change, to overwrite the inclusion style when the configs dictate that, if that is the desired behavior here. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/Harald-R updated
https://github.com/llvm/llvm-project/pull/140594
>From 24071d12afea27104aa099a669470ebb985ab1e5 Mon Sep 17 00:00:00 2001
From: Harald-R
Date: Sun, 18 May 2025 19:07:59 +0300
Subject: [PATCH 1/4] Follow style configuration in clangd include cleaner
---
clang-tools-extra/clangd/IncludeCleaner.cpp | 21 ---
clang-tools-extra/clangd/IncludeCleaner.h | 9
clang-tools-extra/clangd/ParsedAST.cpp| 3 ++-
.../clangd/unittests/IncludeCleanerTests.cpp | 15 +++--
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 9e1f6bb977226..4fedea35334f6 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS,
-Cfg.Diagnostics.Includes.IgnoreHeader);
+Cfg.Diagnostics.Includes.IgnoreHeader,
+Cfg.Style.AngledHeaders);
}
tidy::ClangTidyCheckFactories
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 0ee748c1ed2d0..fda1b
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
kadircet wrote: just a high level question, I wasn't following the recent developments closely, but we seem to have both an `AngledHeaders` and `QuotedHeaders` option. Why are we only following one here? I guess the signals for angled includes are much stricter (search paths need to be marked with `-isystem`), hence we don't need to overwrite an angle back to a quote. But since we have both config options, maybe there is a reason for overwrites in this direction as well. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -297,7 +301,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
Findings.UnusedIncludes.clear();
std::vector Diags = issueIncludeCleanerDiagnostics(
AST, TU.Code, Findings, MockFS(),
- {[](llvm::StringRef Header) { return Header.ends_with("buzz.h"); }});
+ {[](llvm::StringRef Header) { return Header.ends_with("buzz.h"); }},
HighCommander4 wrote:
nit: a couple of parameter hint comments (`/*IgnoreHeader=*/`,
`/*AngledHeaders=*/`) would aid readability here
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -306,6 +311,12 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
withFix({Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
"#include \"b.h\""),
FixMessage("add all missing includes")})),
+ AllOf(Diag(MainFile.range("b_angled"),
+ "No header providing \"b_angled\" is directly included"),
+withFix(
+{Fix(MainFile.range("insert_b_angled"),
+ "#include \n", "#include \"b_angled.h\""),
HighCommander4 wrote:
Shouldn't the message (third parameter to `Fix()`) also be `#include
`?
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -262,6 +264,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
TU.Filename = "main.cpp";
TU.AdditionalFiles["a.h"] = guard("#include \"b.h\"");
TU.AdditionalFiles["b.h"] = guard("void b();");
+ TU.AdditionalFiles["a_angled.h"] = guard("#include \"b_angled.h\"");
HighCommander4 wrote:
* if `a_angled.h` is not part of the `Angled` filter, it should probably be
called `a_quoted.h` (or `c.h` or something else that doesn't include "angled")
* to avoid confusion, let's make the include style of `b_angled.h` angled here
in the contents of `a_angled.h` as well
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
HighCommander4 wrote:
nit: iterate using `auto &`, the way we do
[here](https://searchfox.org/llvm/rev/b9a709832cd96ff3405dd0da9c436d560d8a4f1c/clang-tools-extra/clangd/IncludeCleaner.cpp#63)
https://github.com/llvm/llvm-project/pull/140594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
HighCommander4 wrote: > @kleinesfilmroellchen @HighCommander4 would it be possible to take a look > when you have some time? Many thanks! Yep, will do. Added myself as reviewer. https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
Harald-R wrote: @kleinesfilmroellchen @HighCommander4 would it be possible to take a look when you have some time? Many thanks! https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
llvmbot wrote:
@llvm/pr-subscribers-clang-tools-extra
Author: None (Harald-R)
Changes
The missing include diagnostic has the capability to introduce the necessary
headers into the source file. However, it does not currently follow the
inclusion style found in the `.clangd` file. For example, if the file
explicitly mentions that headers should be include with angled brackets, they
could be included with quotes instead. More details in
https://github.com/llvm/llvm-project/issues/138740. This PR fixes this gap, so
that the style configuration is followed.
---
Full diff: https://github.com/llvm/llvm-project/pull/140594.diff
4 Files Affected:
- (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+14-7)
- (modified) clang-tools-extra/clangd/IncludeCleaner.h (+4-5)
- (modified) clang-tools-extra/clangd/ParsedAST.cpp (+2-1)
- (modified) clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp (+13-2)
``diff
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 3f63daaf400db..a47bfafe8a92c 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS,
-Cfg.Diagnostics.Includes.IgnoreH
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
llvmbot wrote:
@llvm/pr-subscribers-clangd
Author: None (Harald-R)
Changes
The missing include diagnostic has the capability to introduce the necessary
headers into the source file. However, it does not currently follow the
inclusion style found in the `.clangd` file. For example, if the file
explicitly mentions that headers should be include with angled brackets, they
could be included with quotes instead. More details in
https://github.com/llvm/llvm-project/issues/138740. This PR fixes this gap, so
that the style configuration is followed.
---
Full diff: https://github.com/llvm/llvm-project/pull/140594.diff
4 Files Affected:
- (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+14-7)
- (modified) clang-tools-extra/clangd/IncludeCleaner.h (+4-5)
- (modified) clang-tools-extra/clangd/ParsedAST.cpp (+2-1)
- (modified) clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp (+13-2)
``diff
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 3f63daaf400db..a47bfafe8a92c 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS,
-Cfg.Diagnostics.Includes.IgnoreHeader);
+
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/140594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Follow style configuration in clangd when inserting missing includes (PR #140594)
https://github.com/Harald-R created
https://github.com/llvm/llvm-project/pull/140594
The missing include diagnostic has the capability to introduce the necessary
headers into the source file. However, it does not currently follow the
inclusion style found in the `.clangd` file. For example, if the file
explicitly mentions that headers should be include with angled brackets, they
could be included with quotes instead. More details in
https://github.com/llvm/llvm-project/issues/138740. This PR fixes this gap, so
that the style configuration is followed.
>From 710ca263290d4367591add8a93e0aa8bf34f8e9e Mon Sep 17 00:00:00 2001
From: Harald-R
Date: Sun, 18 May 2025 19:07:59 +0300
Subject: [PATCH] Follow style configuration in clangd include cleaner
---
clang-tools-extra/clangd/IncludeCleaner.cpp | 21 ---
clang-tools-extra/clangd/IncludeCleaner.h | 9
clang-tools-extra/clangd/ParsedAST.cpp| 3 ++-
.../clangd/unittests/IncludeCleanerTests.cpp | 15 +++--
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index dc4c8fc498b1f..ce16d060a1f06 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
std::vector generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef MissingIncludes,
-llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS)
{
+llvm::StringRef Code, HeaderFilter IgnoreHeaders,
+HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) {
std::vector Result;
const SourceManager &SM = AST.getSourceManager();
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
@@ -142,6 +143,13 @@ std::vector generateMissingIncludeDiagnostics(
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
+for (auto Filter : AngledHeaders) {
+ if (Filter(HeaderRef)) {
+Angled = true;
+break;
+ }
+}
+
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
// turns out to be the same as one of the unresolved includes in the
@@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc,
return false; // no header provides the symbol
}
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeaders) {
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) {
trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
std::vector UnusedIncludes = generateUnusedIncludeDiagnostics(
AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
std::optional RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics(
- AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS);
+ AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS);
std::optional AddAllMissing =
addAllMissingIncludes(MissingIncludeDiags);
std::optional FixAll;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h
b/clang-tools-extra/clangd/IncludeCleaner.h
index 3f6e3b2fd45b6..884feb69488d5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,11 +57,10 @@ IncludeCleanerFindings
computeIncludeCleanerFindings(ParsedAST &AST,
bool AnalyzeAngledIncludes = false);
-std::vector
-issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
- const IncludeCleanerFindings &Findings,
- const ThreadsafeFS &TFS,
- HeaderFilter IgnoreHeader = {});
+std::vector issueIncludeCleanerDiagnostics(
+ParsedAST &AST, llvm::StringRef Code,
+const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS,
+HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {});
/// Converts the clangd include representation to include-cleaner
/// include representation.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 3f63daaf400db..a47bfafe8a92c 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -382,7 +382,8 @@ std::vector getIncludeCleanerDiags(ParsedAST &AST,
llvm::StringRef Code,
if (SuppressUnused)
Findings.UnusedIncludes.clear();
return issue
