[PATCH] Refactor: Simplify boolean expressions in lib/AST

[John McCall]

In http://reviews.llvm.org/D8532#145849, @LegalizeAdulthood wrote:

> Thanks for your feedback John, this is very helpful in improving this check.
>
> In http://reviews.llvm.org/D8532#145845, @rjmccall wrote:
>
> > [...] Why would we want to do this?
>
>
> So basically you're saying the tool is reporting a false positive for code that is of the form `if (e) return true; else return false;` when this code fragment appears as the last in a series if `if`/`else if` clauses?


In my opinion, yes.

> When it is a single instance of that code fragment, you don't have a problem with it?


Right.  I have no problem with turning "if (<condition>) { return true; } else { return false; }" into "return <condition>;".  It's really the chained use that's kindof suspect to me.

I guess my argument is that, if you've already got more than one interesting condition requiring special behavior, it's better to be consistent across the entire if/else chain, especially because having added cases in the past means it's more likely that you'll add more cases in the future.  If you've only got one condition, it's better to be more compact until it proves inadequate.

> If that understanding is correct, then I can see that this might be a matter of style or preference.  This is a readability check, after all, and most things in readability have some aspect of a subjective viewpoint.

> 

> If the default behavior were to not consider such clauses when they are at the end of a larger if/else clause and there was an option for including those cases as well, would that address the issue?


That would completely address my objection, yes, thank you.


http://reviews.llvm.org/D8532

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/