[llvm-commits] [llvm] r141168 - /llvm/trunk/lib/Analysis/ScalarEvolution.cpp

[Andrew Trick]

On Oct 5, 2011, at 1:20 AM, Chandler Carruth wrote:
> On Wed, Oct 5, 2011 at 1:10 AM, Andrew Trick <atrick at apple.com> wrote:
> Thanks for cleaning up. But broken? No, I just can't train myself to insert redundant parens in this idiom.
> 
> Heh, and on the other side it took me thinking about this more than I like admitting to convince myself that indeed, ((X || Y) && "...") is equivalent to (X || (Y && "..."))... Oh well, probably means that's enough coding for one evening. ;]

I don't want anyone to have to think about binary operator precedence to read my code--I'm not picking on you. My point was that the compiler should know this idiom is ok if it's going to warn people in the general case. I only brought it up on the list because I make this mistake *all the time* using LLVM's assert idiom, so your cleanup won't be the last of its kind. After many years using asserts heavily in C++ (with good macros, not the silly keyword),  the mechanical process of shuffling expressions around between if statements and asserts and adding comments to asserts is completely subconscious. I never visually parse the string as part of the expression then go out of my way to add parens in the rare case that they're needed.

clang doesn't warn about this, which is good, but there is some value in emulating gcc's bad behavior.

-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111005/cd153a22/attachment.html>