Bug 192752

Summary: [Win][Clang] Fix compilation warnings under WebCore/platform/graphics directory
Product: WebKit Reporter: Fujii Hironori <fujii.hironori>
Component: PlatformAssignee: Fujii Hironori <fujii.hironori>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, don.olmstead, pvollan, ross.kirsling, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Fujii Hironori
Reported 2018-12-17 02:33:21 PST
[Win][Clang] Fix compilation warnings WebCore/platform/graphics directory clang-cl reports the following compilation warnings: > ..\..\Source\WebCore\platform\graphics\win\DIBPixelData.cpp(31,19): warning: unused variable 'bitmapType' [-Wunused-const-variable] > ..\..\Source\WebCore\platform\graphics\win\DIBPixelData.cpp(32,19): warning: unused variable 'bitmapPixelsPerMeter' [-Wunused-const-variable] > ..\..\Source\WebCore\platform\graphics\win\FontPlatformDataWin.cpp(82,15): warning: unused variable 'result' [-Wunused-variable] > ..\..\Source\WebCore\platform\graphics\win\FontCacheWin.cpp(362,24): warning: using the result of an assignment as a condition without parentheses [-Wparentheses] > ..\..\Source\WebCore\platform\graphics\win\FontCacheWin.cpp(372,24): warning: using the result of an assignment as a condition without parentheses [-Wparentheses] > ..\..\Source\WebCore\platform\graphics\win\FontCacheWin.cpp(380,24): warning: using the result of an assignment as a condition without parentheses [-Wparentheses] > ..\..\Source\WebCore\platform\graphics\win\FontCacheWin.cpp(382,24): warning: using the result of an assignment as a condition without parentheses [-Wparentheses] > ..\..\Source\WebCore\platform\graphics\win\FontCacheWin.cpp(384,24): warning: using the result of an assignment as a condition without parentheses [-Wparentheses] > ..\..\Source\WebCore\platform\graphics\win\FontCacheWin.cpp(386,24): warning: using the result of an assignment as a condition without parentheses [-Wparentheses] > ..\..\Source\WebCore\platform\graphics\win\FontCacheWin.cpp(388,24): warning: using the result of an assignment as a condition without parentheses [-Wparentheses] > ..\..\Source\WebCore\platform\graphics\win\GraphicsContextWin.cpp(189,21): warning: unused variable 'deg2rad' [-Wunused-const-variable] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(113,36): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(113,43): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(113,50): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(113,57): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(180,36): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(180,43): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(180,50): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(180,57): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(196,36): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(196,43): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(196,50): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(196,57): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataWin.cpp(46,13): warning: unused variable 'cSmallCapsFontSizeMultiplier' [-Wunused-const-variable] > ..\..\Source\WebCore\platform\graphics\win\UniscribeController.cpp(53,7): warning: field 'm_end' will be initialized after field 'm_currentCharacter' [-Wreorder] > ..\..\Source\WebCore\platform\graphics\win\UniscribeController.cpp(90,42): warning: '&&' within '||' [-Wlogical-op-parentheses] > ..\..\Source\WebCore\platform\graphics\win\UniscribeController.cpp(90,74): warning: '&&' within '||' [-Wlogical-op-parentheses] > ..\..\Source\WebCore\platform\graphics\win\UniscribeController.cpp(201,15): warning: using the result of an assignment as a condition without parentheses [-Wparentheses] > ..\..\Source\WebCore\platform\graphics\win\UniscribeController.cpp(263,9): warning: unused variable 'glyphCount' [-Wunused-variable] > ..\..\Source\WebCore\platform\graphics\texmap\TextureMapperGL.cpp(153,10): warning: lambda capture 'this' is not used [-Wunused-lambda-capture] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataCairoWin.cpp(119,36): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataCairoWin.cpp(119,43): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataCairoWin.cpp(119,50): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\SimpleFontDataCairoWin.cpp(119,57): warning: suggest braces around initialization of subobject [-Wmissing-braces] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(98,7): warning: field 'm_hwndVideo' will be initialized after field 'm_volume' [-Wreorder] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(255,13): warning: unused variable 'hr' [-Wunused-variable] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(316,13): warning: unused variable 'hr' [-Wunused-variable] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(428,13): warning: unused variable 'hr' [-Wunused-variable] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(461,13): warning: unused variable 'hr' [-Wunused-variable] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(907,13): warning: unused variable 'hr' [-Wunused-variable] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(2959,17): warning: 64 enumeration values not handled in switch: 'D3DFMT_UNKNOWN', 'D3DFMT_R8G8B8', 'D3DFMT_R5G6B5'... [-Wswitch] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(57,1): warning: unused function 'MFCreateSampleGrabberSinkActivatePtr' [-Wunused-function] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(63,1): warning: unused function 'MFCreateMemoryBufferPtr' [-Wunused-function] > ..\..\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp(64,1): warning: unused function 'MFCreateSamplePtr' [-Wunused-function]
Attachments
Patch (16.97 KB, patch)
2018-12-17 02:47 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Patch (13.02 KB, patch)
2018-12-17 18:09 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Fujii Hironori
Comment 1 2018-12-17 02:47:21 PST
Created attachment 357432 [details] Patch
Don Olmstead
Comment 2 2018-12-17 11:40:12 PST
Comment on attachment 357432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357432&action=review You aren't being consistent in your fixing of "using the result of an assignment as a condition without parentheses". I think you should probably pick the added parenthesis but not sure how this would affect the style guide here. > Source/WebCore/platform/graphics/win/FontCacheWin.cpp:386 > + simpleFont = fontFromDescriptionAndLogFont(fontDescription, nonClientMetrics.lfMenuFont, fallbackFontName); > + if (simpleFont) Not adding parenthesis > Source/WebCore/platform/graphics/win/UniscribeController.cpp:201 > + while ((rc = ::ScriptItemize(cp, length, m_items.size() - 1, &m_control, &m_state, m_items.data(), &numItems)) == E_OUTOFMEMORY) { Added parenthesis here
Fujii Hironori
Comment 3 2018-12-17 17:59:40 PST
Thank you for the review. (In reply to Don Olmstead from comment #2) > Comment on attachment 357432 [details] > You aren't being consistent in your fixing of "using the result of an > assignment as a condition without parentheses". I think you should probably > pick the added parenthesis but not sure how this would affect the style > guide here. Grepping through WebKit code, there are a lot of these code. I think I should give -Wno-parentheses to suppress the warning. Will revert the part of the patch.
Fujii Hironori
Comment 4 2018-12-17 18:09:29 PST
Created attachment 357510 [details] Patch
Don Olmstead
Comment 5 2018-12-17 21:40:23 PST
r=me
Fujii Hironori
Comment 6 2018-12-17 21:50:30 PST
Comment on attachment 357510 [details] Patch Clearing flags on attachment: 357510 Committed r239320: <https://trac.webkit.org/changeset/239320>
Fujii Hironori
Comment 7 2018-12-17 21:50:32 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2018-12-17 21:51:28 PST
<rdar://problem/46801224>
Note You need to log in before you can comment on or make changes to this bug.