Bug 31040 – Use explicit parentheses to silence gcc 4.4 -Wparentheses warnings

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Laszlo Gombos]

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 ||

[Laszlo Gombos]

Created attachment 42356 [details]
1st try

Place the parentheses according to C++ precedence rules.

[Eric Seidel (no email)]

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()) ??

[Laszlo Gombos]

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.

[Kenneth Rohde Christiansen]

LGTM. Does our style guide have a say about this? If not, maybe we should add a rule after discussion on the ML?

[Laszlo Gombos]

(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.

[WebKit Commit Bot]

Comment on attachment 42425 [details]
2nd try

Clearing flags on attachment: 42425

Committed r50675: <http://trac.webkit.org/changeset/50675>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.