Bug 192752 - [Win][Clang] Fix compilation warnings under WebCore/platform/graphics directory
Summary: [Win][Clang] Fix compilation warnings under WebCore/platform/graphics directory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-12-17 02:33 PST by Fujii Hironori
Modified: 2018-12-17 21:51 PST (History)
6 users (show)

See Also:


Attachments
Patch (16.97 KB, patch)
2018-12-17 02:47 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (13.02 KB, patch)
2018-12-17 18:09 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 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]
Comment 1 Fujii Hironori 2018-12-17 02:47:21 PST
Created attachment 357432 [details]
Patch
Comment 2 Don Olmstead 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
Comment 3 Fujii Hironori 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.
Comment 4 Fujii Hironori 2018-12-17 18:09:29 PST
Created attachment 357510 [details]
Patch
Comment 5 Don Olmstead 2018-12-17 21:40:23 PST
r=me
Comment 6 Fujii Hironori 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>
Comment 7 Fujii Hironori 2018-12-17 21:50:32 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-12-17 21:51:28 PST
<rdar://problem/46801224>