820536 - -Wlogical-op-parentheses warning in gfx/2d/DrawTargetCG.cpp

Status: RESOLVED DUPLICATE of bug 839384

(Core :: Graphics, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

A compiler warning on something like this:
if (a == b && c == d || a == f && c == g)
may as well get rid of the warning and make things "safer" to read.

/Users/msreckovic/Development/mozilla-central/gfx/2d/DrawTargetCG.cpp:987:41: warning:

      '&&' within '||' [-Wlogical-op-parentheses]

  ...aType == NATIVE_SURFACE_CGCONTEXT && GetContextType(mCg) == CG_CONTEXT_TYPE_BITMAP ||

     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~

/Users/msreckovic/Development/mozilla-central/gfx/2d/DrawTargetCG.cpp:987:41: note:

      place parentheses around the '&&' expression to silence this warning

  ...aType == NATIVE_SURFACE_CGCONTEXT && GetContextType(mCg) == CG_CONTEXT_TYPE_BITMAP ...

                                       ^

     (                                                                                 )

/Users/msreckovic/Development/mozilla-central/gfx/2d/DrawTargetCG.cpp:988:53: warning:

      '&&' within '||' [-Wlogical-op-parentheses]

  ...aType == NATIVE_SURFACE_CGCONTEXT_ACCELERATED && GetContextType(mCg) == CG_CONTEXT_TYPE_IOSURFACE...

     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/Users/msreckovic/Development/mozilla-central/gfx/2d/DrawTargetCG.cpp:988:53: note:

      place parentheses around the '&&' expression to silence this warning

  ...aType == NATIVE_SURFACE_CGCONTEXT_ACCELERATED && GetContextType(mCg) == CG_CONTEXT_TYPE_IOSURFACE)…

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Add the reference to the beta bug to clean all the warnings.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Add explicit parentheses in the compound logical statement

C++ operator precedence being what it is, the original:
if (a == b && c == d || a == f && c == g) {
is the same as:
if (((a == b) && (c == d)) || ((a == f) && (c == g))) {
But without the compiler warning.  No functional/logical change intended.

Comment 3

[Benoit Girard (:BenWa)]

Comment on attachment 691038 [details] [diff] [review]
Add explicit parentheses in the compound logical statement

Review of attachment 691038 [details] [diff] [review]:
-----------------------------------------------------------------

Less warning noise masking useful warnings \o/.
NIT: I think adding parentheses around (a == b) is a bit to far.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Updated with code review comments

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 691100 [details] [diff] [review]
Updated with code review comments

FYI, checkin-needed will get your patches landed much faster in the future

Comment 6

[Ryan VanderMeulen [:RyanVM]]

dholbert fixed this in bug 839384 in the mean time :)