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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

These test cases don't show it, from what i can see this will erroneously match:

  if (onFire) {
    tryPutFireOut();
  } else {
    if (isHot && onFire) {
      scream();
    }
  } 

It appears that this will change the second if to:

  if (isHot) {
    scream();
  }

Rather than simply deleting the if as it will always evaluate to false. To fix 
this you could wrap the `hasDescendant` matcher inside a `hasThen` matcher.

Another case is if the condition variable is on the RHS of a redundant inner if 
condition, you can't delete the inner if when the LHS potentially has side 
effects.

  if (onFire) {
    if (callTheFD() || onFire) {
      scream();
    }
  }

If `callTheFD` has side effects this can't be replaced with this:

  if (onFire) {
    scream();
  }

You could however replace it with:

  if (onFire) {
    callTheFD();
    scream();
  }

For this simply check if the LHS has side effects.


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

https://reviews.llvm.org/D81272



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