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

Matt Calabrese rivorus at gmail.com
Mon May 25 14:19:55 PDT 2015


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.


http://reviews.llvm.org/D10014

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






More information about the cfe-commits mailing list