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

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 8 09:24:14 PDT 2020


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





More information about the cfe-commits mailing list