zinovy.nis updated this revision to Diff 304561. zinovy.nis added a comment.
Use `IgnoreParenImpCasts` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91037/new/ https://reviews.llvm.org/D91037 Files: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp @@ -1073,6 +1073,34 @@ } } +// ExprWithCleanups doesn't crash +int positive_expr_with_cleanups() { + class RetT { + public: + RetT(const int _code) : code_(_code) {} + bool Ok() const { return code_ == 0; } + static RetT Test(bool &_isSet) { return 0; } + + private: + int code_; + }; + + bool isSet = false; + if (RetT::Test(isSet).Ok() && isSet) { + if (RetT::Test(isSet).Ok() && isSet) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {{isSet}} + } + } + if (isSet) { + if ((RetT::Test(isSet).Ok() && isSet)) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {{isSet}} + } + } + return 0; +} + //===--- Special Negatives ------------------------------------------------===// // Aliasing Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp @@ -82,7 +82,9 @@ // For standalone condition variables and for "or" binary operations we simply // remove the inner `if`. - const auto *BinOpCond = dyn_cast<BinaryOperator>(InnerIf->getCond()); + const auto *BinOpCond = + dyn_cast<BinaryOperator>(InnerIf->getCond()->IgnoreParenImpCasts()); + if (isa<DeclRefExpr>(InnerIf->getCond()->IgnoreParenImpCasts()) || (BinOpCond && BinOpCond->getOpcode() == BO_LOr)) { SourceLocation IfBegin = InnerIf->getBeginLoc(); @@ -129,7 +131,8 @@ // For "and" binary operations we remove the "and" operation with the // condition variable from the inner if. } else { - const auto *CondOp = cast<BinaryOperator>(InnerIf->getCond()); + const auto *CondOp = + cast<BinaryOperator>(InnerIf->getCond()->IgnoreParenImpCasts()); const auto *LeftDRE = dyn_cast<DeclRefExpr>(CondOp->getLHS()->IgnoreParenImpCasts()); if (LeftDRE && LeftDRE->getDecl() == CondVar) {
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp @@ -1073,6 +1073,34 @@ } } +// ExprWithCleanups doesn't crash +int positive_expr_with_cleanups() { + class RetT { + public: + RetT(const int _code) : code_(_code) {} + bool Ok() const { return code_ == 0; } + static RetT Test(bool &_isSet) { return 0; } + + private: + int code_; + }; + + bool isSet = false; + if (RetT::Test(isSet).Ok() && isSet) { + if (RetT::Test(isSet).Ok() && isSet) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {{isSet}} + } + } + if (isSet) { + if ((RetT::Test(isSet).Ok() && isSet)) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {{isSet}} + } + } + return 0; +} + //===--- Special Negatives ------------------------------------------------===// // Aliasing Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp @@ -82,7 +82,9 @@ // For standalone condition variables and for "or" binary operations we simply // remove the inner `if`. - const auto *BinOpCond = dyn_cast<BinaryOperator>(InnerIf->getCond()); + const auto *BinOpCond = + dyn_cast<BinaryOperator>(InnerIf->getCond()->IgnoreParenImpCasts()); + if (isa<DeclRefExpr>(InnerIf->getCond()->IgnoreParenImpCasts()) || (BinOpCond && BinOpCond->getOpcode() == BO_LOr)) { SourceLocation IfBegin = InnerIf->getBeginLoc(); @@ -129,7 +131,8 @@ // For "and" binary operations we remove the "and" operation with the // condition variable from the inner if. } else { - const auto *CondOp = cast<BinaryOperator>(InnerIf->getCond()); + const auto *CondOp = + cast<BinaryOperator>(InnerIf->getCond()->IgnoreParenImpCasts()); const auto *LeftDRE = dyn_cast<DeclRefExpr>(CondOp->getLHS()->IgnoreParenImpCasts()); if (LeftDRE && LeftDRE->getDecl() == CondVar) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits