subject:"\[clang\-tools\-extra\] \[clang\-tidy\] avoid false postive when ignore macro \(PR #91757\)"

[clang-tools-extra] [clang-tidy] avoid false postive when ignore macro (PR #91757)

2024-05-10 Thread via cfe-commits


llvmbot wrote:




@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)


Changes

Fixes: #91487


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


4 Files Affected:

- (modified) 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (+36-25) 
- (modified) 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h (+2) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) 
clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
 (+14-4) 


``diff
diff --git 
a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index edb67614bd558..4b4ffc3fe0074 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "SimplifyBooleanExprCheck.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/SaveAndRestore.h"
@@ -280,9 +281,8 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
 if (!S) {
   return true;
 }
-if (Check->IgnoreMacros && S->getBeginLoc().isMacroID()) {
+if (Check->canBeBypassed(S))
   return false;
-}
 if (!shouldIgnore(S))
   StmtStack.push_back(S);
 return true;
@@ -513,17 +513,25 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
 return true;
   }
 
-  static bool isUnaryLNot(const Expr *E) {
-return isa(E) &&
+  static bool isExpectedUnaryLNot(SimplifyBooleanExprCheck *Check,
+  const Expr *E) {
+return !Check->canBeBypassed(E) && isa(E) &&
cast(E)->getOpcode() == UO_LNot;
   }
 
+  static bool isExpectedBinaryOp(SimplifyBooleanExprCheck *Check,
+ const Expr *E) {
+const auto *BinaryOp = dyn_cast(E);
+return !Check->canBeBypassed(E) && BinaryOp && BinaryOp->isLogicalOp() &&
+   BinaryOp->getType()->isBooleanType();
+  }
+
   template 
   static bool checkEitherSide(const BinaryOperator *BO, Functor Func) {
 return Func(BO->getLHS()) || Func(BO->getRHS());
   }
 
-  static bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {
+  bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {
 const auto *BO = 
dyn_cast(E->IgnoreUnlessSpelledInSource());
 if (!BO)
   return false;
@@ -539,15 +547,14 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   return true;
 case BO_LAnd:
 case BO_LOr:
-  if (checkEitherSide(BO, isUnaryLNot))
-return true;
-  if (NestingLevel) {
-if (checkEitherSide(BO, [NestingLevel](const Expr *E) {
-  return nestedDemorgan(E, NestingLevel - 1);
-}))
-  return true;
-  }
-  return false;
+  return checkEitherSide(BO,
+ [this](const Expr *E) {
+   return isExpectedUnaryLNot(Check, E);
+ }) ||
+ (NestingLevel &&
+  checkEitherSide(BO, [this, NestingLevel](const Expr *E) {
+return nestedDemorgan(E, NestingLevel - 1);
+  }));
 default:
   return false;
 }
@@ -556,19 +563,19 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   bool TraverseUnaryOperator(UnaryOperator *Op) {
 if (!Check->SimplifyDeMorgan || Op->getOpcode() != UO_LNot)
   return Base::TraverseUnaryOperator(Op);
-Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
-auto *Parens = dyn_cast(SubImp);
-auto *BinaryOp =
-Parens
-? dyn_cast(Parens->getSubExpr()->IgnoreImplicit())
-: dyn_cast(SubImp);
-if (!BinaryOp || !BinaryOp->isLogicalOp() ||
-!BinaryOp->getType()->isBooleanType())
+const Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
+const auto *Parens = dyn_cast(SubImp);
+const Expr *SubExpr =
+Parens ? Parens->getSubExpr()->IgnoreImplicit() : SubImp;
+if (!isExpectedBinaryOp(Check, SubExpr))
   return Base::TraverseUnaryOperator(Op);
+const auto *BinaryOp = cast(SubExpr);
 if (Check->SimplifyDeMorganRelaxed ||
-checkEitherSide(BinaryOp, isUnaryLNot) ||
-checkEitherSide(BinaryOp,
-[](const Expr *E) { return nestedDemorgan(E, 1); })) {
+checkEitherSide(
+BinaryOp,
+[this](const Expr *E) { return isExpectedUnaryLNot(Check, E); }) ||
+checkEitherSide(
+BinaryOp, [this](const Expr *E) { return nestedDemorgan(E, 1); })) 
{
   if (Check->reportDeMorgan(Context, Op, BinaryOp, !IsProcessing, parent(),
 Parens) &&
   !Che

[clang-tools-extra] [clang-tidy] avoid false postive when ignore macro (PR #91757)

2024-05-10 Thread via cfe-commits


llvmbot wrote:




@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)


Changes

Fixes: #91487


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


4 Files Affected:

- (modified) 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (+36-25) 
- (modified) 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h (+2) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) 
clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
 (+14-4) 


``diff
diff --git 
a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index edb67614bd558..4b4ffc3fe0074 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "SimplifyBooleanExprCheck.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/SaveAndRestore.h"
@@ -280,9 +281,8 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
 if (!S) {
   return true;
 }
-if (Check->IgnoreMacros && S->getBeginLoc().isMacroID()) {
+if (Check->canBeBypassed(S))
   return false;
-}
 if (!shouldIgnore(S))
   StmtStack.push_back(S);
 return true;
@@ -513,17 +513,25 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
 return true;
   }
 
-  static bool isUnaryLNot(const Expr *E) {
-return isa(E) &&
+  static bool isExpectedUnaryLNot(SimplifyBooleanExprCheck *Check,
+  const Expr *E) {
+return !Check->canBeBypassed(E) && isa(E) &&
cast(E)->getOpcode() == UO_LNot;
   }
 
+  static bool isExpectedBinaryOp(SimplifyBooleanExprCheck *Check,
+ const Expr *E) {
+const auto *BinaryOp = dyn_cast(E);
+return !Check->canBeBypassed(E) && BinaryOp && BinaryOp->isLogicalOp() &&
+   BinaryOp->getType()->isBooleanType();
+  }
+
   template 
   static bool checkEitherSide(const BinaryOperator *BO, Functor Func) {
 return Func(BO->getLHS()) || Func(BO->getRHS());
   }
 
-  static bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {
+  bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {
 const auto *BO = 
dyn_cast(E->IgnoreUnlessSpelledInSource());
 if (!BO)
   return false;
@@ -539,15 +547,14 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   return true;
 case BO_LAnd:
 case BO_LOr:
-  if (checkEitherSide(BO, isUnaryLNot))
-return true;
-  if (NestingLevel) {
-if (checkEitherSide(BO, [NestingLevel](const Expr *E) {
-  return nestedDemorgan(E, NestingLevel - 1);
-}))
-  return true;
-  }
-  return false;
+  return checkEitherSide(BO,
+ [this](const Expr *E) {
+   return isExpectedUnaryLNot(Check, E);
+ }) ||
+ (NestingLevel &&
+  checkEitherSide(BO, [this, NestingLevel](const Expr *E) {
+return nestedDemorgan(E, NestingLevel - 1);
+  }));
 default:
   return false;
 }
@@ -556,19 +563,19 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   bool TraverseUnaryOperator(UnaryOperator *Op) {
 if (!Check->SimplifyDeMorgan || Op->getOpcode() != UO_LNot)
   return Base::TraverseUnaryOperator(Op);
-Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
-auto *Parens = dyn_cast(SubImp);
-auto *BinaryOp =
-Parens
-? dyn_cast(Parens->getSubExpr()->IgnoreImplicit())
-: dyn_cast(SubImp);
-if (!BinaryOp || !BinaryOp->isLogicalOp() ||
-!BinaryOp->getType()->isBooleanType())
+const Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
+const auto *Parens = dyn_cast(SubImp);
+const Expr *SubExpr =
+Parens ? Parens->getSubExpr()->IgnoreImplicit() : SubImp;
+if (!isExpectedBinaryOp(Check, SubExpr))
   return Base::TraverseUnaryOperator(Op);
+const auto *BinaryOp = cast(SubExpr);
 if (Check->SimplifyDeMorganRelaxed ||
-checkEitherSide(BinaryOp, isUnaryLNot) ||
-checkEitherSide(BinaryOp,
-[](const Expr *E) { return nestedDemorgan(E, 1); })) {
+checkEitherSide(
+BinaryOp,
+[this](const Expr *E) { return isExpectedUnaryLNot(Check, E); }) ||
+checkEitherSide(
+BinaryOp, [this](const Expr *E) { return nestedDemorgan(E, 1); })) 
{
   if (Check->reportDeMorgan(Context, Op, BinaryOp, !IsProcessing, parent(),
 Parens) &&

[clang-tools-extra] [clang-tidy] avoid false postive when ignore macro (PR #91757)

2024-05-10 Thread Congcong Cai via cfe-commits


https://github.com/HerrCai0907 created 
https://github.com/llvm/llvm-project/pull/91757

Fixes: #91487


>From d340a04ffa50e03eb5267e267a27dcae5fefa223 Mon Sep 17 00:00:00 2001
From: Congcong Cai 
Date: Thu, 9 May 2024 00:51:58 +0800
Subject: [PATCH] [clang-tidy] avoid false postive when ignore macro

Fixes: #91487
---
 .../readability/SimplifyBooleanExprCheck.cpp  | 61 +++
 .../readability/SimplifyBooleanExprCheck.h|  2 +
 clang-tools-extra/docs/ReleaseNotes.rst   |  4 ++
 .../simplify-boolean-expr-macros.cpp  | 18 --
 4 files changed, 56 insertions(+), 29 deletions(-)

diff --git 
a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index edb67614bd558..4b4ffc3fe0074 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "SimplifyBooleanExprCheck.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/SaveAndRestore.h"
@@ -280,9 +281,8 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
 if (!S) {
   return true;
 }
-if (Check->IgnoreMacros && S->getBeginLoc().isMacroID()) {
+if (Check->canBeBypassed(S))
   return false;
-}
 if (!shouldIgnore(S))
   StmtStack.push_back(S);
 return true;
@@ -513,17 +513,25 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
 return true;
   }
 
-  static bool isUnaryLNot(const Expr *E) {
-return isa(E) &&
+  static bool isExpectedUnaryLNot(SimplifyBooleanExprCheck *Check,
+  const Expr *E) {
+return !Check->canBeBypassed(E) && isa(E) &&
cast(E)->getOpcode() == UO_LNot;
   }
 
+  static bool isExpectedBinaryOp(SimplifyBooleanExprCheck *Check,
+ const Expr *E) {
+const auto *BinaryOp = dyn_cast(E);
+return !Check->canBeBypassed(E) && BinaryOp && BinaryOp->isLogicalOp() &&
+   BinaryOp->getType()->isBooleanType();
+  }
+
   template 
   static bool checkEitherSide(const BinaryOperator *BO, Functor Func) {
 return Func(BO->getLHS()) || Func(BO->getRHS());
   }
 
-  static bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {
+  bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {
 const auto *BO = 
dyn_cast(E->IgnoreUnlessSpelledInSource());
 if (!BO)
   return false;
@@ -539,15 +547,14 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   return true;
 case BO_LAnd:
 case BO_LOr:
-  if (checkEitherSide(BO, isUnaryLNot))
-return true;
-  if (NestingLevel) {
-if (checkEitherSide(BO, [NestingLevel](const Expr *E) {
-  return nestedDemorgan(E, NestingLevel - 1);
-}))
-  return true;
-  }
-  return false;
+  return checkEitherSide(BO,
+ [this](const Expr *E) {
+   return isExpectedUnaryLNot(Check, E);
+ }) ||
+ (NestingLevel &&
+  checkEitherSide(BO, [this, NestingLevel](const Expr *E) {
+return nestedDemorgan(E, NestingLevel - 1);
+  }));
 default:
   return false;
 }
@@ -556,19 +563,19 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   bool TraverseUnaryOperator(UnaryOperator *Op) {
 if (!Check->SimplifyDeMorgan || Op->getOpcode() != UO_LNot)
   return Base::TraverseUnaryOperator(Op);
-Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
-auto *Parens = dyn_cast(SubImp);
-auto *BinaryOp =
-Parens
-? dyn_cast(Parens->getSubExpr()->IgnoreImplicit())
-: dyn_cast(SubImp);
-if (!BinaryOp || !BinaryOp->isLogicalOp() ||
-!BinaryOp->getType()->isBooleanType())
+const Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
+const auto *Parens = dyn_cast(SubImp);
+const Expr *SubExpr =
+Parens ? Parens->getSubExpr()->IgnoreImplicit() : SubImp;
+if (!isExpectedBinaryOp(Check, SubExpr))
   return Base::TraverseUnaryOperator(Op);
+const auto *BinaryOp = cast(SubExpr);
 if (Check->SimplifyDeMorganRelaxed ||
-checkEitherSide(BinaryOp, isUnaryLNot) ||
-checkEitherSide(BinaryOp,
-[](const Expr *E) { return nestedDemorgan(E, 1); })) {
+checkEitherSide(
+BinaryOp,
+[this](const Expr *E) { return isExpectedUnaryLNot(Check, E); }) ||
+checkEitherSide(
+BinaryOp, [this](const Expr *E) { return nestedDemorgan(E, 1); })) 
{
   if (Check->reportDeMorgan(Context, Op, BinaryOp, !IsProcessing, parent(),

[clang-tools-extra] [clang-tidy] avoid false postive when ignore macro (PR #91757)

[clang-tools-extra] [clang-tidy] avoid false postive when ignore macro (PR #91757)

[clang-tools-extra] [clang-tidy] avoid false postive when ignore macro (PR #91757)

3 matches

Site Navigation

Mail list logo

Footer information