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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
