[PATCH] D10014: Refactor: Simplify boolean conditional return statements in lib/Edit

Richard via cfe-commits cfe-commits at lists.llvm.org
Sat Oct 24 13:12:21 PDT 2015


LegalizeAdulthood added a comment.

In http://reviews.llvm.org/D10014#178294, @calabrese wrote:

> I like the idea of all of these changes (this change list and the others), but just a suggestion -- it looks like when the branch of the original condition returns false, as was the case in the original code here, the internal expressions of the check are transformed via De Morgan's laws in the new code. IMO, this transformation sometimes makes things more complicated rather than less, where by "more complicated" I mean that the transformation produces an overall expression that is constructed from a considerably greater number of logical operations. For instance, here it changed a bunch of || expressions into a bunch of && expressions, with each operand individually prefixed with "!". The equivalent without having applied De Morgan's laws would just have left all of the || expressions as-is and added a single ! around the overall expression parenthesized. It would have also looked much more similar to the original code.
>
> My general thought here is that it might make sense when performing this kind of transformation to first see if the translation via De Morgan's laws actually produces an overall expression that has fewer logical subexpressions, otherwise prefer the minimal transformation of a ! around a parenthesized expression.


This should be addressed in the latest diff.


http://reviews.llvm.org/D10014





More information about the cfe-commits mailing list