[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 &amp;&amp; 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits