GCC 4.4 -Wparentheses now warns about expressions such as (!x | y) and (!x & y). Using explicit parentheses, such as in ((!x) | y), silences this warning -- see the release notes: http://gcc.gnu.org/gcc-4.4/changes.html As an example QtWebKit buildbot is using GCC 4.4 here are some of the warnings: JavaScriptCore/interpreter/Interpreter.cpp:1491: warning: suggest parentheses around arithmetic in operand of | JavaScriptCore/interpreter/Interpreter.cpp:1578: warning: suggest parentheses around arithmetic in operand of | WebCore/editing/TextIterator.cpp:221: warning: suggest parentheses around && within || WebCore/editing/VisibleSelection.cpp:240: warning: suggest parentheses around && within || WebCore/html/HTMLLinkElement.cpp:191: warning: suggest parentheses around && within || WebCore/loader/RedirectScheduler.cpp:174: warning: suggest parentheses around && within || WebCore/rendering/RenderBlock.cpp:956: warning: suggest parentheses around && within || WebCore/rendering/RenderBox.cpp:1474: warning: suggest parentheses around && within || WebCore/rendering/RenderTextControlMultiLine.cpp:68: warning: suggest parentheses around && within || WebCore/rendering/style/RenderStyle.cpp:458: warning: suggest parentheses around && within || WebCore/platform/graphics/qt/FontCacheQt.cpp:126: warning: suggest parentheses around + or - inside shift WebCore/platform/graphics/qt/FontCacheQt.cpp:127: warning: suggest parentheses around + or - inside shift WebCore/platform/graphics/qt/FontCacheQt.cpp:128: warning: suggest parentheses around + or - inside shift WebKit/qt/Api/qwebpage.cpp:1213: warning: suggest parentheses around && within || WebKit/qt/Api/qwebpage.cpp:1219: warning: suggest parentheses around && within || WebKit/qt/Api/qwebpage.cpp:1223: warning: suggest parentheses around && within || WebCore/svg/SVGAnimateElement.cpp:67: warning: suggest parentheses around && within || WebCore/svg/SVGAnimationElement.cpp:492: warning: suggest parentheses around && within || WebCore/svg/SVGPreserveAspectRatio.cpp:183: warning: suggest parentheses around && within || WebCore/loader/appcache/ApplicationCacheGroup.cpp:625: warning: suggest parentheses around && within || WebCore/dom/Document.cpp:2494: warning: suggest parentheses around && within ||
Created attachment 42356 [details] 1st try Place the parentheses according to C++ precedence rules.
Comment on attachment 42356 [details] 1st try Some of these make sense. And some of them don't: 1578 if (src1.isInt32() && src2.isInt32() && !((src1.asInt32()) | (src2.asInt32() & 0xc0000000))) // no overflow Why are parens needed around (src1.asInt32()) ??
Created attachment 42425 [details] 2nd try Remove the extra () from around (src1.asInt32()). Eric, thanks for the review - I think you found one case where I put a meaningless extra () in. I hope the rest of the changes looks good.
LGTM. Does our style guide have a say about this? If not, maybe we should add a rule after discussion on the ML?
(In reply to comment #4) > LGTM. Does our style guide have a say about this? If not, maybe we should add a > rule after discussion on the ML? Kenneth, thanks for the review. I think that for warnings that compilers can catch the style guide might be redundant. However we should have a common agreement on the warnings we care about as a community.
Comment on attachment 42425 [details] 2nd try Clearing flags on attachment: 42425 Committed r50675: <http://trac.webkit.org/changeset/50675>
All reviewed patches have been landed. Closing bug.