[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D81272#2218050 <https://reviews.llvm.org/D81272#2218050>, @aaron.ballman wrote:

> Thanks to the new info, I think the check basically LGTM. Can you add some negative tests and documentation wording to make it clear that the check doesn't currently handle all logically equivalent predicates, like:
>
>   if (foo) {
>   } else {
>     if (!foo) {
>     }
>   }
>   
>   // or
>   if (foo > 5) {
>     if (foo > 3) {
>     }
>   }
>   
>   // or
>   if (foo > 5) {
>     if (5 < foo) {
>     }
>   }
>
> (I'm assuming these cases aren't handled currently and that handling them isn't necessary to land the patch.)

I'm afraid handling such cases will eventually invoke the same problems the RangeConstraintSolver has, and then the discussion about whether this is good in Tidy instead of CSA will be resurrected... 😦

One could come up with even more elaborate cases. Just hit something like below a few minutes ago while working:

  const CallExpr* const C;
  
  if (auto* CalledFun = dyn_cast_or_null<FunctionDecl>(C->getCalleeDecl())) {
    // ...
    if (const auto* Fun = dyn_cast_or_null<FunctionDecl>(C->getCalleeDecl())) {
      // ...
    }
    // ...
  }

and the rabbit hole goes deeper. But it's pretty interesting how many cases the current check found as-is.


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

https://reviews.llvm.org/D81272