Bug 145126

Summary: [cmake] Suppress parentheses-equality warnings
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: New BugsAssignee: Csaba Osztrogonác <ossy>
Status: RESOLVED FIXED    
Severity: Normal CC: ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 145121    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Csaba Osztrogonác
Reported 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 (==).
Attachments
Patch (2.54 KB, patch)
2015-05-18 03:41 PDT, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
Patch (3.53 KB, patch)
2015-05-18 03:45 PDT, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
Patch (1.42 KB, patch)
2015-05-21 07:15 PDT, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 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.
Csaba Osztrogonác
Comment 2 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
Csaba Osztrogonác
Comment 3 2015-05-18 03:45:11 PDT
Created attachment 253323 [details] Patch
Alexey Proskuryakov
Comment 4 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?
Csaba Osztrogonác
Comment 5 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?
Csaba Osztrogonác
Comment 6 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.
Csaba Osztrogonác
Comment 7 2015-05-21 07:15:15 PDT
Created attachment 253524 [details] Patch
Csaba Osztrogonác
Comment 8 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.
Darin Adler
Comment 9 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?
Darin Adler
Comment 10 2015-05-26 13:09:38 PDT
Comment on attachment 253524 [details] Patch Oh, I see why.
Csaba Osztrogonác
Comment 11 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>
Csaba Osztrogonác
Comment 12 2015-06-01 02:11:38 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.