Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] Add option to ignore macros in readability-simplify-boolean-expr check #78043

Merged
merged 8 commits into from
Jan 14, 2024

Conversation

SimplyDanny
Copy link
Member

@SimplyDanny SimplyDanny commented Jan 13, 2024

Closes #57975.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 13, 2024

@llvm/pr-subscribers-clang-tidy

Author: Danny Mösch (SimplyDanny)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/78043.diff

5 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (+5)
  • (modified) clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h (+1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst (+6-2)
  • (added) clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp (+20)
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..3b556418238db5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -500,6 +500,9 @@ 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` 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);
+}

@SimplyDanny
Copy link
Member Author

Thanks @PiotrZSL for the review! I've addressed your remarks. Would you support me with the test as well?

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

clang-tools-extra/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved
@SimplyDanny SimplyDanny merged commit 5295ca1 into llvm:main Jan 14, 2024
5 checks passed
@SimplyDanny SimplyDanny deleted the ignore-macros-simplify-boolean branch January 14, 2024 00:14
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang-tidy] SimplifyDeMorgan of readability-simplify-boolean-expr should not trigger within macro
3 participants