Bug 145126

Summary:

[cmake] Suppress parentheses-equality warnings

Product:

WebKit

Reporter:

Csaba Osztrogonác <ossy>

Component:

New Bugs

Assignee:

Csaba Osztrogonác <ossy>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ossy

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

145121

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Description Csaba Osztrogonác 2015-05-18 03:38:11 PDT

../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:450:25: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:450:25: note: remove extraneous parentheses around the comparison to silence this warning
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:452:25: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:452:25: note: remove extraneous parentheses around the comparison to silence this warning
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:454:25: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:454:25: note: remove extraneous parentheses around the comparison to silence this warning
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:546:31: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:546:31: note: remove extraneous parentheses around the comparison to silence this warning
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:862:38: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
../../Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:862:38: note: remove extraneous parentheses around the comparison to silence this warning

NPVARIANT_IS_* defines already contain parentheses, if we use this macro in 
an if, we will get something like: "if (( ... == ... ))". But unfortunately 
double parentheses means in the clang terminology that we explicitly want
to use assignment (=) instead of comparision (==).

Comment 1 Csaba Osztrogonác 2015-05-18 03:41:27 PDT

Created attachment 253322 [details]
Patch

Removed extra parentheses to make clang happy ... the result is ugly, but it works ... Better fix is very welcome.

Comment 2 Csaba Osztrogonác 2015-05-18 03:43:45 PDT

and one more in TestNetscapePlugIn/main.cpp:

../../Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:420:52: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
../../Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:420:52: note: remove extraneous parentheses around the comparison to silence this warning

Comment 3 Csaba Osztrogonác 2015-05-18 03:45:11 PDT

Created attachment 253323 [details]
Patch

Comment 4 Alexey Proskuryakov 2015-05-18 12:07:45 PDT

Comment on attachment 253323 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253323&action=review

> Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:450
> +    if NPVARIANT_IS_STRING(variant)

This looks worse than before. Can this warning be disabled instead? Did it catch any bugs ever?

Comment 5 Csaba Osztrogonác 2015-05-18 13:11:20 PDT

(In reply to comment #4)
> Comment on attachment 253323 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=253323&action=review
> 
> > Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:450
> > +    if NPVARIANT_IS_STRING(variant)
> 
> This looks worse than before. Can this warning be disabled instead? Did it
> catch any bugs ever?

Yes, I told it is ugly. But I don't want to disable this useful warning, 
because it is responsible for the opposite and typical programming error:
if (a=b). We do want to get warning for this serious bug. Any better idea
how to supress this warning only here without disabling it in general?

Comment 6 Csaba Osztrogonác 2015-05-20 02:32:31 PDT

Comment on attachment 253323 [details]
Patch

I was wrong, fortunately Wparentheses-equality and Wparantheses
are diffent warnings. Let's suppress only Wparentheses-equality.

Comment 7 Csaba Osztrogonác 2015-05-21 07:15:15 PDT

Created attachment 253524 [details]
Patch

Comment 8 Csaba Osztrogonác 2015-05-21 07:16:31 PDT

It works in itself for the most part of WebKit, but will have 
no effect in TestNetscapePlugIn until bug145264 is fixed.

Comment 9 Darin Adler 2015-05-26 13:09:03 PDT

Comment on attachment 253524 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253524&action=review

> Source/cmake/WebKitHelpers.cmake:18
> +        # Suppress -Wparentheses-equality warning of Clang

Why?

Comment 10 Darin Adler 2015-05-26 13:09:38 PDT

Comment on attachment 253524 [details]
Patch

Oh, I see why.

Comment 11 Csaba Osztrogonác 2015-06-01 02:11:29 PDT

Comment on attachment 253524 [details]
Patch

Clearing flags on attachment: 253524

Committed r185055: <http://trac.webkit.org/changeset/185055>

Comment 12 Csaba Osztrogonác 2015-06-01 02:11:38 PDT

All reviewed patches have been landed.  Closing bug.