[clang-tools-extra] [clang-tidy] Add option to ignore macros in `readability-simplify-boolean-expr` check (PR #78043)

Danny Mösch via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 13 08:59:55 PST 2024


https://github.com/SimplyDanny updated https://github.com/llvm/llvm-project/pull/78043

>From 55d278f3f33716b5b18d46048df7e664bcdfed6a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moesch at icloud.com>
Date: Sat, 13 Jan 2024 16:36:48 +0100
Subject: [PATCH 1/4] [clang-tidy] Add option to ignore macros in
 `readability-simplify-boolean-expr` check

---
 .../readability/SimplifyBooleanExprCheck.cpp  |  5 +++++
 .../readability/SimplifyBooleanExprCheck.h    |  1 +
 clang-tools-extra/docs/ReleaseNotes.rst       |  4 ++++
 .../readability/simplify-boolean-expr.rst     |  8 ++++++--
 .../simplify-boolean-expr-macros.cpp          | 20 +++++++++++++++++++
 5 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 26d9287f07049e..34a18325ce3173 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -277,6 +277,9 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
   }
 
   bool dataTraverseStmtPre(Stmt *S) {
+    if (Check->IgnoreMacros && S->getBeginLoc().isMacroID()) {
+      return false;
+    }
     if (S && !shouldIgnore(S))
       StmtStack.push_back(S);
     return true;
@@ -583,6 +586,7 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
 SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
                                                    ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
+      IgnoreMacros(Options.get("IgnoreMacros", false)),
       ChainedConditionalReturn(Options.get("ChainedConditionalReturn", false)),
       ChainedConditionalAssignment(
           Options.get("ChainedConditionalAssignment", false)),
@@ -671,6 +675,7 @@ void SimplifyBooleanExprCheck::reportBinOp(const ASTContext &Context,
 }
 
 void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
   Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
   Options.store(Opts, "ChainedConditionalAssignment",
                 ChainedConditionalAssignment);
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
index c4dad24ec39985..ccc6f3d879fc02 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -64,6 +64,7 @@ class SimplifyBooleanExprCheck : public ClangTidyCheck {
                  StringRef Description, SourceRange ReplacementRange,
                  StringRef Replacement);
 
+  const bool IgnoreMacros;
   const bool ChainedConditionalReturn;
   const bool ChainedConditionalAssignment;
   const bool SimplifyDeMorgan;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3437b6cf9b59e9..02cbc0bf07c4b0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -500,6 +500,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   identify calls to static member functions with out-of-class inline definitions.
 
+- Added option `IgnoreMacros` to :doc:`readability-simplify-boolean-expr
+  <clang-tidy/checks/readability/simplify-boolean-expr>` check.
+  It makes the check ignore boolean expressions passed into macros.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
index 18ab84b26a2595..443bcadc9f12e6 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
@@ -82,6 +82,10 @@ Examples:
 Options
 -------
 
+.. option:: IgnoreMacros
+
+   If `true`, ignore boolean expressions passed into macros. Default is `false`.
+
 .. option:: ChainedConditionalReturn
 
    If `true`, conditional boolean return statements at the end of an
@@ -99,8 +103,8 @@ Options
 
 .. option:: SimplifyDeMorganRelaxed
 
-   If `true`, :option:`SimplifyDeMorgan` will also transform negated 
-   conjunctions and disjunctions where there is no negation on either operand. 
+   If `true`, :option:`SimplifyDeMorgan` will also transform negated
+   conjunctions and disjunctions where there is no negation on either operand.
    This option has no effect if :option:`SimplifyDeMorgan` is `false`.
    Default is `false`.
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
new file mode 100644
index 00000000000000..55f586c168beb9
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy -check-suffixes=,MACROS %s readability-simplify-boolean-expr %t
+
+// Ignore expressions in macros.
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+// RUN:     -- -config="{CheckOptions: [{key: readability-simplify-boolean-expr.IgnoreMacros, value: true}]}"
+// RUN:     --
+
+#define NEGATE(expr) !(expr)
+
+bool without_macro(bool a, bool b) {
+    return !(!a && b);
+    // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: boolean expression can be simplified by DeMorgan's theorem
+    // CHECK-FIXES: return a || !b;
+}
+
+bool macro(bool a, bool b) {
+    return NEGATE(!a && b);
+    // CHECK-MESSAGES-MACROS: :[[@LINE-1]]:12: warning: boolean expression can be simplified by DeMorgan's theorem
+    // CHECK-FIXES: return NEGATE(!a && b);
+}

>From b956bea2f3ba97ea7888bbf979387c87ea82b213 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moesch at icloud.com>
Date: Sat, 13 Jan 2024 17:59:04 +0100
Subject: [PATCH 2/4] Check for potential `null` value

---
 .../clang-tidy/readability/SimplifyBooleanExprCheck.cpp      | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 34a18325ce3173..edb67614bd5585 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -277,10 +277,13 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
   }
 
   bool dataTraverseStmtPre(Stmt *S) {
+    if (!S) {
+      return true;
+    }
     if (Check->IgnoreMacros && S->getBeginLoc().isMacroID()) {
       return false;
     }
-    if (S && !shouldIgnore(S))
+    if (!shouldIgnore(S))
       StmtStack.push_back(S);
     return true;
   }

>From c26c9d84abdc63d4aec1da3399776163344a1db1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moesch at icloud.com>
Date: Sat, 13 Jan 2024 17:59:28 +0100
Subject: [PATCH 3/4] Move entry in release notes

---
 clang-tools-extra/docs/ReleaseNotes.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 02cbc0bf07c4b0..f198fc35c5a548 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -492,6 +492,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/non-const-parameter>` check to ignore
   false-positives in initializer list of record.
 
+- Added option `IgnoreMacros` to :doc:`readability-simplify-boolean-expr
+  <clang-tidy/checks/readability/simplify-boolean-expr>` check.
+  It makes the check ignore boolean expressions passed into macros.
+
 - Improved :doc:`readability-simplify-subscript-expr
   <clang-tidy/checks/readability/simplify-subscript-expr>` check by extending
   the default value of the `Types` option to include ``std::span``.
@@ -500,10 +504,6 @@ Changes in existing checks
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   identify calls to static member functions with out-of-class inline definitions.
 
-- Added option `IgnoreMacros` to :doc:`readability-simplify-boolean-expr
-  <clang-tidy/checks/readability/simplify-boolean-expr>` check.
-  It makes the check ignore boolean expressions passed into macros.
-
 Removed checks
 ^^^^^^^^^^^^^^
 

>From b9771f08c51a7a5d43b9f17157f38c6a35e192eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moesch at icloud.com>
Date: Sat, 13 Jan 2024 17:59:39 +0100
Subject: [PATCH 4/4] Use new option style

---
 .../checkers/readability/simplify-boolean-expr-macros.cpp       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
index 55f586c168beb9..5192a7bacdcf6b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
@@ -2,7 +2,7 @@
 
 // Ignore expressions in macros.
 // RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
-// RUN:     -- -config="{CheckOptions: [{key: readability-simplify-boolean-expr.IgnoreMacros, value: true}]}"
+// RUN:     -- -config="{CheckOptions: {readability-simplify-boolean-expr.IgnoreMacros: true}}"
 // RUN:     --
 
 #define NEGATE(expr) !(expr)



More information about the cfe-commits mailing list