[clang-tools-extra] [include-cleaner] rename enabled flags to `disable-*` (PR #132991)
Mohamed Emad via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 25 13:26:44 PDT 2025
https://github.com/hulxv updated https://github.com/llvm/llvm-project/pull/132991
>From c476948593a80ed31765cdd711a626e4e03930ab Mon Sep 17 00:00:00 2001
From: hulxv <hulxxv at gmail.com>
Date: Tue, 25 Mar 2025 22:56:51 +0200
Subject: [PATCH 1/7] [include-cleaner] rename enabled flags to `disable-*`
---
.../include-cleaner/test/tool.cpp | 4 ++--
.../include-cleaner/tool/IncludeCleaner.cpp | 20 +++++++++----------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/test/tool.cpp b/clang-tools-extra/include-cleaner/test/tool.cpp
index d72d2317ce2b1..8b723a5bf40e2 100644
--- a/clang-tools-extra/include-cleaner/test/tool.cpp
+++ b/clang-tools-extra/include-cleaner/test/tool.cpp
@@ -6,11 +6,11 @@ int x = foo();
// CHANGE: - "foobar.h"
// CHANGE-NEXT: + "foo.h"
-// RUN: clang-include-cleaner -remove=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
+// RUN: clang-include-cleaner -disable-remove -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
// INSERT-NOT: - "foobar.h"
// INSERT: + "foo.h"
-// RUN: clang-include-cleaner -insert=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
+// RUN: clang-include-cleaner -disable-insert -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
// REMOVE: - "foobar.h"
// REMOVE-NOT: + "foo.h"
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index 1d9458ffc4d32..472611073f732 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -91,16 +91,16 @@ cl::opt<bool> Edit{
cl::cat(IncludeCleaner),
};
-cl::opt<bool> Insert{
- "insert",
- cl::desc("Allow header insertions"),
- cl::init(true),
+cl::opt<bool> DisableInsert{
+ "disable-insert",
+ cl::desc("DIsable header insertions"),
+ cl::init(false),
cl::cat(IncludeCleaner),
};
-cl::opt<bool> Remove{
- "remove",
- cl::desc("Allow header removals"),
- cl::init(true),
+cl::opt<bool> DisableRemove{
+ "disable-remove",
+ cl::desc("Disable header removals"),
+ cl::init(false),
cl::cat(IncludeCleaner),
};
@@ -183,9 +183,9 @@ class Action : public clang::ASTFrontendAction {
auto Results =
analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI,
getCompilerInstance().getPreprocessor(), HeaderFilter);
- if (!Insert)
+ if (DisableInsert)
Results.Missing.clear();
- if (!Remove)
+ if (DisableRemove)
Results.Unused.clear();
std::string Final = fixIncludes(Results, AbsPath, Code, getStyle(AbsPath));
>From e2f78ab69f656313fc87b004506abc1deb096189 Mon Sep 17 00:00:00 2001
From: hulxv <hulxxv at gmail.com>
Date: Fri, 18 Apr 2025 08:59:03 +0200
Subject: [PATCH 2/7] [include-cleaner] return `--remove` and `--insert` to be
in deprecation period
---
.../include-cleaner/tool/IncludeCleaner.cpp | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index 472611073f732..7a07d09ce277d 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -90,10 +90,21 @@ cl::opt<bool> Edit{
cl::desc("Apply edits to analyzed source files"),
cl::cat(IncludeCleaner),
};
-
+cl::opt<bool> Insert{
+ "insert",
+ cl::desc("Allow header insertions"),
+ cl::init(true),
+ cl::cat(IncludeCleaner),
+};
+cl::opt<bool> Remove{
+ "remove",
+ cl::desc("Allow header removals"),
+ cl::init(true),
+ cl::cat(IncludeCleaner),
+};
cl::opt<bool> DisableInsert{
"disable-insert",
- cl::desc("DIsable header insertions"),
+ cl::desc("Disable header insertions"),
cl::init(false),
cl::cat(IncludeCleaner),
};
@@ -183,9 +194,9 @@ class Action : public clang::ASTFrontendAction {
auto Results =
analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI,
getCompilerInstance().getPreprocessor(), HeaderFilter);
- if (DisableInsert)
+ if (!Insert || DisableInsert)
Results.Missing.clear();
- if (DisableRemove)
+ if (!Remove || DisableRemove)
Results.Unused.clear();
std::string Final = fixIncludes(Results, AbsPath, Code, getStyle(AbsPath));
>From b7dd110307c0ba467022775398b9bd2ab66517f5 Mon Sep 17 00:00:00 2001
From: hulxv <hulxxv at gmail.com>
Date: Fri, 25 Apr 2025 19:30:55 +0300
Subject: [PATCH 3/7] [clang-tools-extra] add note for deprecation of `-remove`
and `-insert`
---
clang-tools-extra/docs/ReleaseNotes.rst | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 72aa05eb4dcd1..8b39fa09e2839 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -88,6 +88,14 @@ Improvements to clang-doc
Improvements to clang-query
---------------------------
+Improvements to include-cleaner
+-------------------------------
+- Deprecated the ``-insert`` and ``-remove`` command line options, and added
+ the ``-disable-remove`` and ``-disable-insert`` command line options as
+ replacements. The previous command line options were confusing because they
+ did not imply the default state of the option (which is inserts and removes
+ being enabled). The new options are easier to understand the semantics of.
+
Improvements to clang-tidy
--------------------------
>From 69321153dd78fe92aea60d9e7c11f6559fbf7ca1 Mon Sep 17 00:00:00 2001
From: hulxv <hulxxv at gmail.com>
Date: Fri, 25 Apr 2025 22:46:56 +0300
Subject: [PATCH 4/7] [include-cleaner] warning when the deprecated commands
are used
---
.../include-cleaner/tool/IncludeCleaner.cpp | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index 7a07d09ce277d..ce1b2545f7101 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -92,13 +92,13 @@ cl::opt<bool> Edit{
};
cl::opt<bool> Insert{
"insert",
- cl::desc("Allow header insertions"),
+ cl::desc("Allow header insertions (deprecated. Use -disable-insert instead)"),
cl::init(true),
cl::cat(IncludeCleaner),
};
cl::opt<bool> Remove{
"remove",
- cl::desc("Allow header removals"),
+ cl::desc("Allow header removals (deprecated. Use -disable-remove instead)"),
cl::init(true),
cl::cat(IncludeCleaner),
};
@@ -194,6 +194,21 @@ class Action : public clang::ASTFrontendAction {
auto Results =
analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI,
getCompilerInstance().getPreprocessor(), HeaderFilter);
+
+ if (!Insert) {
+ llvm::errs() << "`-insert=0` is deprecated in favor of `-disable-insert`. "
+ "The old flag was confusing since it suggested that inserts "
+ "were disabled by default, when they were actually enabled. "
+ "See https://github.com/llvm/llvm-project/pull/132991\n";
+ }
+
+ if (!Remove) {
+ llvm::errs() << "`-remove=0` is deprecated in favor of `-disable-remove`. "
+ "The old flag was confusing since it suggested that removes "
+ "were disabled by default, when they were actually enabled. "
+ "See https://github.com/llvm/llvm-project/pull/132991\n";
+ }
+
if (!Insert || DisableInsert)
Results.Missing.clear();
if (!Remove || DisableRemove)
>From 573eed8007c05bbeffff3c2dd507c5cc2110520f Mon Sep 17 00:00:00 2001
From: hulxv <hulxxv at gmail.com>
Date: Fri, 25 Apr 2025 22:55:35 +0300
Subject: [PATCH 5/7] [include-cleaner] refer to the main issue instead of the
PR
---
clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index ce1b2545f7101..dd853950d543c 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -199,14 +199,14 @@ class Action : public clang::ASTFrontendAction {
llvm::errs() << "`-insert=0` is deprecated in favor of `-disable-insert`. "
"The old flag was confusing since it suggested that inserts "
"were disabled by default, when they were actually enabled. "
- "See https://github.com/llvm/llvm-project/pull/132991\n";
+ "See https://github.com/llvm/llvm-project/issues/132983\n";
}
if (!Remove) {
llvm::errs() << "`-remove=0` is deprecated in favor of `-disable-remove`. "
"The old flag was confusing since it suggested that removes "
"were disabled by default, when they were actually enabled. "
- "See https://github.com/llvm/llvm-project/pull/132991\n";
+ "See https://github.com/llvm/llvm-project/issues/132983\n";
}
if (!Insert || DisableInsert)
>From dd552fc44d0c1e6a53b48839b7435e64624b680a Mon Sep 17 00:00:00 2001
From: hulxv <hulxxv at gmail.com>
Date: Fri, 25 Apr 2025 23:06:25 +0300
Subject: [PATCH 6/7] improve the warning message
---
.../include-cleaner/tool/IncludeCleaner.cpp | 21 +++++++++++--------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index dd853950d543c..d33d9b12d4236 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -92,7 +92,8 @@ cl::opt<bool> Edit{
};
cl::opt<bool> Insert{
"insert",
- cl::desc("Allow header insertions (deprecated. Use -disable-insert instead)"),
+ cl::desc(
+ "Allow header insertions (deprecated. Use -disable-insert instead)"),
cl::init(true),
cl::cat(IncludeCleaner),
};
@@ -196,17 +197,19 @@ class Action : public clang::ASTFrontendAction {
getCompilerInstance().getPreprocessor(), HeaderFilter);
if (!Insert) {
- llvm::errs() << "`-insert=0` is deprecated in favor of `-disable-insert`. "
- "The old flag was confusing since it suggested that inserts "
- "were disabled by default, when they were actually enabled. "
- "See https://github.com/llvm/llvm-project/issues/132983\n";
+ llvm::errs()
+ << "[WARNING] -insert is deprecated in favor of `-disable-insert`. "
+ "The old flag was confusing since it suggested that inserts "
+ "were disabled by default, when they were actually enabled. "
+ "See https://github.com/llvm/llvm-project/issues/132983\n";
}
if (!Remove) {
- llvm::errs() << "`-remove=0` is deprecated in favor of `-disable-remove`. "
- "The old flag was confusing since it suggested that removes "
- "were disabled by default, when they were actually enabled. "
- "See https://github.com/llvm/llvm-project/issues/132983\n";
+ llvm::errs()
+ << "[WARNING] -remove is deprecated in favor of `-disable-remove`. "
+ "The old flag was confusing since it suggested that removes "
+ "were disabled by default, when they were actually enabled. "
+ "See https://github.com/llvm/llvm-project/issues/132983\n";
}
if (!Insert || DisableInsert)
>From 30ea7f16e5055b3ef7a061bedb9a1fdb16f6c4b0 Mon Sep 17 00:00:00 2001
From: hulxv <hulxxv at gmail.com>
Date: Fri, 25 Apr 2025 23:26:29 +0300
Subject: [PATCH 7/7] [include-cleaner] testing display warnings of deprecated
commands
---
clang-tools-extra/include-cleaner/test/tool.cpp | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/clang-tools-extra/include-cleaner/test/tool.cpp b/clang-tools-extra/include-cleaner/test/tool.cpp
index 8b723a5bf40e2..2b931aff4f1fd 100644
--- a/clang-tools-extra/include-cleaner/test/tool.cpp
+++ b/clang-tools-extra/include-cleaner/test/tool.cpp
@@ -58,3 +58,20 @@ int x = foo();
// RUN: FileCheck --match-full-lines --check-prefix=EDIT3 %s < %t.cpp
// EDIT3: #include "foo.h"
// EDIT3-NOT: {{^}}#include "foobar.h"{{$}}
+
+// RUN: clang-include-cleaner -insert=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
+// RUN: FileCheck --check-prefix=DEPRECATED-INSERT %s
+// DEPRECATED-INSERT: [WARNING] -insert is deprecated in favor of `-disable-insert`
+// DEPRECATED-INSERT: The old flag was confusing since it suggested that inserts
+// DEPRECATED-INSERT: were disabled by default, when they were actually enabled
+
+// RUN: clang-include-cleaner -remove=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
+// RUN: FileCheck --check-prefix=DEPRECATED-REMOVE %s
+// DEPRECATED-REMOVE: [WARNING] -remove is deprecated in favor of `-disable-remove`
+// DEPRECATED-REMOVE: The old flag was confusing since it suggested that removes
+// DEPRECATED-REMOVE: were disabled by default, when they were actually enabled
+
+// RUN: clang-include-cleaner -insert=false -remove=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
+// RUN: FileCheck --check-prefix=DEPRECATED-BOTH %s
+// DEPRECATED-BOTH: [WARNING] -insert is deprecated in favor of `-disable-insert`
+// DEPRECATED-BOTH: [WARNING] -remove is deprecated in favor of `-disable-remove`
More information about the cfe-commits
mailing list