[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: 
clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:39
+
+  // Check if PrevS < Mut < NextS
   return MutS &&
----------------
I think this comment should be hoisted above to update the comment on line 32.


================
Comment at: 
clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:56
               binaryOperator(hasOperatorName("&&"),
-                             
hasEitherOperand(ignoringParenImpCasts(declRefExpr(
-                                 hasDeclaration(ImmutableVar)))))))),
+                             
hasEitherOperand(ignoringParenImpCasts(declRefExpr(hasDeclaration(ImmutableVar))
+                                        .bind(OuterIfVar2Str))))))),
----------------
80-col limit issue -- you should run the patch through clang-format.


================
Comment at: 
clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:87
+
+  const auto OuterIfVar = OuterIfVar1 ? OuterIfVar1 : OuterIfVar2;
+  const auto InnerIfVar = InnerIfVar1 ? InnerIfVar1 : InnerIfVar2;
----------------
`const auto *`, but I think this would be better expressed as:
```
const DeclRefExpr *OuterIfVar, *InnerIfVar;
if (const auto *Outer = Result.Nodes.getNodeAs<DeclRefExpr>(OuterIfVar1Str))
  OuterIfVar = Outer;
else
  OuterIfVar = Result.Nodes.getNodeAs<DeclRefExpr>(OuterIfVar2Str);

// Similar for InnerIfVar
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91495/new/

https://reviews.llvm.org/D91495

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits