[clang-tools-extra] [clangd] Add config option to analyze unused system headers (PR #87208)

2024-03-31 Thread Vadim D. via cfe-commits 

https://github.com/vvd170501 updated 
https://github.com/llvm/llvm-project/pull/87208

>From 35db92dfd0c2b43fc7afde5b093598763c4b8c89 Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Mon, 1 Apr 2024 02:26:14 +0300
Subject: [PATCH] Add config option to analyze unused system headers

---
 clang-tools-extra/clangd/Config.h |  1 +
 clang-tools-extra/clangd/ConfigCompile.cpp| 57 +++
 clang-tools-extra/clangd/ConfigFragment.h |  4 ++
 clang-tools-extra/clangd/ConfigYAML.cpp   |  4 ++
 clang-tools-extra/clangd/IncludeCleaner.cpp   | 34 +++
 clang-tools-extra/clangd/IncludeCleaner.h | 13 +
 clang-tools-extra/clangd/ParsedAST.cpp|  3 +-
 .../clangd/unittests/ConfigCompileTests.cpp   |  6 ++
 .../clangd/unittests/ConfigYAMLTests.cpp  | 15 +
 .../clangd/unittests/IncludeCleanerTests.cpp  | 15 +
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 11 files changed, 112 insertions(+), 45 deletions(-)

diff --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index 4371c80a6c5877..9629854abff31b 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -114,6 +114,7 @@ struct Config {
 /// these regexes.
 struct {
   std::vector> IgnoreHeader;
+  bool AnalyzeSystemHeaders = false;
 } Includes;
   } Diagnostics;
 
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 5bb2eb4a9f803f..f74c870adfb73d 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -572,32 +572,43 @@ struct FragmentCompiler {
 #else
 static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
 #endif
-auto Filters = std::make_shared>();
-for (auto  : F.IgnoreHeader) {
-  // 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;
+std::shared_ptr> Filters;
+if (!F.IgnoreHeader.empty()) {
+  Filters = std::make_shared>();
+  for (auto  : F.IgnoreHeader) {
+// 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));
   }
-  Filters->push_back(std::move(CompiledRegex));
 }
-if (Filters->empty())
+std::optional AnalyzeSystemHeaders;
+if (F.AnalyzeSystemHeaders.has_value())
+  AnalyzeSystemHeaders = **F.AnalyzeSystemHeaders;
+if (!Filters && !AnalyzeSystemHeaders.has_value())
   return;
-auto Filter = [Filters](llvm::StringRef Path) {
-  for (auto  : *Filters)
-if (Regex.match(Path))
-  return true;
-  return false;
-};
-Out.Apply.push_back([Filter](const Params &, Config ) {
-  C.Diagnostics.Includes.IgnoreHeader.emplace_back(Filter);
+Out.Apply.push_back([Filters = std::move(Filters),
+ AnalyzeSystemHeaders](const Params &, Config ) {
+  if (Filters) {
+auto Filter = [Filters](llvm::StringRef Path) {
+  for (auto  : *Filters)
+if (Regex.match(Path))
+  return true;
+  return false;
+};
+C.Diagnostics.Includes.IgnoreHeader.emplace_back(std::move(Filter));
+  }
+  if (AnalyzeSystemHeaders.has_value())
+C.Diagnostics.Includes.AnalyzeSystemHeaders = *AnalyzeSystemHeaders;
 });
   }
 
diff --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 7fa61108c78a05..ac8b88c2454128 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -254,6 +254,10 @@ struct Fragment {
   /// unused or missing. These can match any suffix of the header file in
   /// question.
   std::vector> IgnoreHeader;
+
+  /// If false (default), unused system headers will be ignored.
+  /// Standard library headers are analyzed regardless of this option.
+  std::optional> AnalyzeSystemHeaders;
 };
 IncludesBlock Includes;
 
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp 
b/clang-tools-extra/clangd/ConfigYAML.cpp
index ce09af819247ae..7608af42005386 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++

[clang-tools-extra] [clangd] Add config option to analyze unused system headers (PR #87208)

2024-03-31 Thread Vadim D. via cfe-commits 

https://github.com/vvd170501 updated 
https://github.com/llvm/llvm-project/pull/87208

>From 3e29095dbc2240b7a61d8d261224501a85753e7d Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Mon, 1 Apr 2024 02:26:14 +0300
Subject: [PATCH] Add config option to analyze unused system headers

---
 clang-tools-extra/clangd/Config.h |  1 +
 clang-tools-extra/clangd/ConfigCompile.cpp| 57 +++
 clang-tools-extra/clangd/ConfigFragment.h |  4 ++
 clang-tools-extra/clangd/ConfigYAML.cpp   |  4 ++
 clang-tools-extra/clangd/IncludeCleaner.cpp   | 34 +++
 clang-tools-extra/clangd/IncludeCleaner.h | 13 +
 clang-tools-extra/clangd/ParsedAST.cpp|  3 +-
 .../clangd/unittests/ConfigCompileTests.cpp   |  6 ++
 .../clangd/unittests/ConfigYAMLTests.cpp  | 15 +
 .../clangd/unittests/IncludeCleanerTests.cpp  | 15 +
 clang-tools-extra/docs/ReleaseNotes.rst   |  4 ++
 11 files changed, 111 insertions(+), 45 deletions(-)

diff --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index 4371c80a6c5877..9629854abff31b 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -114,6 +114,7 @@ struct Config {
 /// these regexes.
 struct {
   std::vector> IgnoreHeader;
+  bool AnalyzeSystemHeaders = false;
 } Includes;
   } Diagnostics;
 
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 5bb2eb4a9f803f..f74c870adfb73d 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -572,32 +572,43 @@ struct FragmentCompiler {
 #else
 static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
 #endif
-auto Filters = std::make_shared>();
-for (auto  : F.IgnoreHeader) {
-  // 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;
+std::shared_ptr> Filters;
+if (!F.IgnoreHeader.empty()) {
+  Filters = std::make_shared>();
+  for (auto  : F.IgnoreHeader) {
+// 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));
   }
-  Filters->push_back(std::move(CompiledRegex));
 }
-if (Filters->empty())
+std::optional AnalyzeSystemHeaders;
+if (F.AnalyzeSystemHeaders.has_value())
+  AnalyzeSystemHeaders = **F.AnalyzeSystemHeaders;
+if (!Filters && !AnalyzeSystemHeaders.has_value())
   return;
-auto Filter = [Filters](llvm::StringRef Path) {
-  for (auto  : *Filters)
-if (Regex.match(Path))
-  return true;
-  return false;
-};
-Out.Apply.push_back([Filter](const Params &, Config ) {
-  C.Diagnostics.Includes.IgnoreHeader.emplace_back(Filter);
+Out.Apply.push_back([Filters = std::move(Filters),
+ AnalyzeSystemHeaders](const Params &, Config ) {
+  if (Filters) {
+auto Filter = [Filters](llvm::StringRef Path) {
+  for (auto  : *Filters)
+if (Regex.match(Path))
+  return true;
+  return false;
+};
+C.Diagnostics.Includes.IgnoreHeader.emplace_back(std::move(Filter));
+  }
+  if (AnalyzeSystemHeaders.has_value())
+C.Diagnostics.Includes.AnalyzeSystemHeaders = *AnalyzeSystemHeaders;
 });
   }
 
diff --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 7fa61108c78a05..ac8b88c2454128 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -254,6 +254,10 @@ struct Fragment {
   /// unused or missing. These can match any suffix of the header file in
   /// question.
   std::vector> IgnoreHeader;
+
+  /// If false (default), unused system headers will be ignored.
+  /// Standard library headers are analyzed regardless of this option.
+  std::optional> AnalyzeSystemHeaders;
 };
 IncludesBlock Includes;
 
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp 
b/clang-tools-extra/clangd/ConfigYAML.cpp
index ce09af819247ae..7608af42005386 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++

[clang-tools-extra] [clangd] Add config option to analyze unused system headers (PR #87208)

2024-03-31 Thread Vadim D. via cfe-commits 

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Add config option to analyze unused system headers (PR #87208)

2024-03-31 Thread via cfe-commits 

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 82c6eeed08b1c8267f6e92d594c910fe57a9775e 
f222e44f5586297ddd61ac0efff4a7115a766c53 -- 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/IncludeCleaner.cpp 
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/IncludeCleanerTests.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 570f3cfc3c..f74c870adf 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -584,7 +584,8 @@ struct FragmentCompiler {
   diag(Warning,
llvm::formatv("Invalid regular expression '{0}': {1}",
  *HeaderPattern, RegexError)
-   .str(), HeaderPattern.Range);
+   .str(),
+   HeaderPattern.Range);
   continue;
 }
 Filters->push_back(std::move(CompiledRegex));

``




https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Add config option to analyze unused system headers (PR #87208)

2024-03-31 Thread via cfe-commits 

github-actions[bot] wrote:

⚠️ We detected that you are using a GitHub private e-mail address to contribute 
to the repo.
  Please turn off [Keep my email addresses 
private](https://github.com/settings/emails) setting in your account.
  See [LLVM 
Discourse](https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it)
 for more information.


https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Add config option to analyze unused system headers (PR #87208)

2024-03-31 Thread Vadim D. via cfe-commits 

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/87208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Add config option to analyze unused system headers (PR #87208)

2024-03-31 Thread Vadim D. via cfe-commits 

https://github.com/vvd170501 created 
https://github.com/llvm/llvm-project/pull/87208

This PR adds a new `AnalyzeSystemHeaders` option to `Includes` section of 
clangd config. This option will allow to mark all system headers, not just the 
ones from standard library, as unused.

Why this change is useful?
In our codebase all includes from outside of local directory are written with 
angle brackets. For example,
```cpp
// foo.cpp
#include "foo.h"  // main header
#include "foo_utils.h"  // From the same directory
#include   // Other directory, path is relative the 
repository root
#include   // Header from standard library
```

I'd like to use the builtin include-cleaner of clangd to detect unused 
includes, but currently it ignores non-standard system headers

I understand that enabling unused include detection for system headers will 
probably produce some false positives due to lack of support for umbrella 
headers, but they can be filtered out by `IgnoreHeader`. This is not ideal, but 
still better than not having unused include detection for system headers at all.

>From f222e44f5586297ddd61ac0efff4a7115a766c53 Mon Sep 17 00:00:00 2001
From: vvd170501 <36827317+vvd170...@users.noreply.github.com>
Date: Mon, 1 Apr 2024 01:12:11 +0300
Subject: [PATCH] Add config option to analyze unused system headers

---
 clang-tools-extra/clangd/Config.h |  1 +
 clang-tools-extra/clangd/ConfigCompile.cpp| 56 +++
 clang-tools-extra/clangd/ConfigFragment.h |  4 ++
 clang-tools-extra/clangd/ConfigYAML.cpp   |  4 ++
 clang-tools-extra/clangd/IncludeCleaner.cpp   | 34 +++
 clang-tools-extra/clangd/IncludeCleaner.h | 13 +
 clang-tools-extra/clangd/ParsedAST.cpp|  3 +-
 .../clangd/unittests/ConfigCompileTests.cpp   |  6 ++
 .../clangd/unittests/ConfigYAMLTests.cpp  | 15 +
 .../clangd/unittests/IncludeCleanerTests.cpp  | 15 +
 clang-tools-extra/docs/ReleaseNotes.rst   |  4 ++
 11 files changed, 110 insertions(+), 45 deletions(-)

diff --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index 4371c80a6c5877..9629854abff31b 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -114,6 +114,7 @@ struct Config {
 /// these regexes.
 struct {
   std::vector> IgnoreHeader;
+  bool AnalyzeSystemHeaders = false;
 } Includes;
   } Diagnostics;
 
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 5bb2eb4a9f803f..570f3cfc3ce2ed 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -572,32 +572,42 @@ struct FragmentCompiler {
 #else
 static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
 #endif
-auto Filters = std::make_shared>();
-for (auto  : F.IgnoreHeader) {
-  // 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;
+std::shared_ptr> Filters;
+if (!F.IgnoreHeader.empty()) {
+  Filters = std::make_shared>();
+  for (auto  : F.IgnoreHeader) {
+// 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));
   }
-  Filters->push_back(std::move(CompiledRegex));
 }
-if (Filters->empty())
+std::optional AnalyzeSystemHeaders;
+if (F.AnalyzeSystemHeaders.has_value())
+  AnalyzeSystemHeaders = **F.AnalyzeSystemHeaders;
+if (!Filters && !AnalyzeSystemHeaders.has_value())
   return;
-auto Filter = [Filters](llvm::StringRef Path) {
-  for (auto  : *Filters)
-if (Regex.match(Path))
-  return true;
-  return false;
-};
-Out.Apply.push_back([Filter](const Params &, Config ) {
-  C.Diagnostics.Includes.IgnoreHeader.emplace_back(Filter);
+Out.Apply.push_back([Filters = std::move(Filters),
+ AnalyzeSystemHeaders](const Params &, Config ) {
+  if (Filters) {
+auto Filter = [Filters](llvm::StringRef Path) {
+  for (auto  : *Filters)
+if (Regex.match(Path))
+  return true;
+  return false;
+};
+