[clang-tools-extra] [clang-tidy] Fix false negative for binary logical expressions for tw… (PR #230846)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Abhay Singh (Abhay13332)

<details>
<summary>Changes</summary>

Fixes #<!-- -->230601

This PR addresses the false-negative scenario where `bugprone-redundant-branch-condition` fails to trigger on simple two-variable logical conjunction expressions (e.g., `if (x && y) { if (y) ... }`). 

Note: This is a partial fix. As noted in the code's `// FIXME` comments, handling longer conjunctive and disjunctive chain clauses (3 or more variables) requires further extension is not yet touched.


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


2 Files Affected:

- (modified) clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp (+3-4) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-branch-condition.cpp (+15-2) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
index 8d608dd4b0e66..f6c6373969b34 100644
--- a/clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -76,10 +76,9 @@ void RedundantBranchConditionCheck::registerMatchers(MatchFinder *Finder) {
       ifStmt(
           hasCondition(anyOf(
               declRefExpr(hasDeclaration(ImmutableVar)).bind(OuterIfVar1Str),
-              binaryOperator(
-                  hasOperatorName("&&"),
-                  hasEitherOperand(declRefExpr(hasDeclaration(ImmutableVar))
-                                       .bind(OuterIfVar2Str))))),
+              binaryOperator(hasOperatorName("&&"),
+                             forEach((declRefExpr(hasDeclaration(ImmutableVar))
+                                          .bind(OuterIfVar2Str)))))),
           hasThen(hasDescendant(
               ifStmt(hasCondition(anyOf(
                          declRefExpr(hasDeclaration(
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-branch-condition.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-branch-condition.cpp
index ad2238ff9984f..1111c27ef01f4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-branch-condition.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/redundant-branch-condition.cpp
@@ -12,7 +12,7 @@ bool tryToExtinguishByVal(bool);
 void tryPutFireOut();
 bool callTheFD();
 void scream();
-
+bool isFireFighterComing();
 bool someOtherCondition();
 
 //===--- Basic Positives --------------------------------------------------===//
@@ -278,13 +278,26 @@ void positive_direct_outer_and_rhs() {
     // CHECK-FIXES: {{^\ *$}}
   }
 }
+void positive_direct_outer_and_rhs_bool() {
+  bool onFire = isBurning();
+  bool isComing = isFireFighterComing();
+  if (isComing && onFire) {
+    if (onFire) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+      // CHECK-FIXES: {{^\ *$}}
+      scream();
+    }
+    // CHECK-FIXES: {{^\ *$}}
+  }
+}
+
 
 void positive_indirect_outer_and_rhs() {
   bool onFire = isBurning();
   if (fireFighters < 10 && onFire) {
     if (someOtherCondition()) {
       if (onFire) {
-        // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+        // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
         // CHECK-FIXES: {{^\ *$}}
         scream();
       }

``````````

</details>


https://github.com/llvm/llvm-project/pull/230846