Bug 189496 - [Win][Clang][ImageDiff] Fix compilation error and warning of PlatformImageCairo.cpp
Summary: [Win][Clang][ImageDiff] Fix compilation error and warning of PlatformImageCai...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-09-11 02:22 PDT by Fujii Hironori
Modified: 2018-09-12 19:05 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.73 KB, patch)
2018-09-11 02:38 PDT, 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-09-11 02:22:17 PDT
[Win][Clang][ImageDiff] Fix compilation error and warning of PlatformImageCairo.cpp

While trying to build WebKit WinCairo port with the latest Clang (Bug 171618),
the following compilation error and warning is reported.

> [2/1360] Building CXX object Tools\ImageDiff\CMakeFiles\ImageDiffLib.dir\cairo\PlatformImageCairo.cpp.obj
> FAILED: Tools/ImageDiff/CMakeFiles/ImageDiffLib.dir/cairo/PlatformImageCairo.cpp.obj 
> C:\tools\llvm\bin\clang-cl.exe  (...) /FdTools\ImageDiff\CMakeFiles\ImageDiffLib.dir\ -c ..\..\Tools\ImageDiff\cairo\PlatformImageCairo.cpp
> ..\..\Tools\ImageDiff\cairo\PlatformImageCairo.cpp(46,22):  error: non-constant-expression cannot be narrowed from type 'size_t' (aka 'unsigned long long') to 'unsigned long' in initializer list [-Wc++11-narrowing]
>     } context { { }, imageSize, 0 };
>                      ^~~~~~~~~
> ..\..\Tools\ImageDiff\cairo\PlatformImageCairo.cpp(46,22):  note: insert an explicit cast to silence this issue
>     } context { { }, imageSize, 0 };
>                      ^~~~~~~~~
>                      static_cast<unsigned long>( )
> ..\..\Tools\ImageDiff\cairo\PlatformImageCairo.cpp(125,61):  warning: format specifies type 'unsigned __int64' (aka 'unsigned long long') but the argument has type 'unsigned long' [-Wformat]
>     fprintf(stdout, "Content-Length: %" FORMAT_SIZE_T "\n", context.writtenBytes);
>                                      ~~~~~~~~~~~~~~~~       ^~~~~~~~~~~~~~~~~~~~
> 1 warning and 1 error generated.
Comment 1 Fujii Hironori 2018-09-11 02:38:47 PDT
Created attachment 349389 [details]
Patch
Comment 2 Don Olmstead 2018-09-11 09:47:57 PDT
Informal r+ from me. Will look for a formal review.
Comment 3 Per Arne Vollan 2018-09-11 13:14:10 PDT
Comment on attachment 349389 [details]
Patch

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

> Tools/ImageDiff/cairo/PlatformImageCairo.cpp:40
> -        }, &context);
> +        }, nullptr);

Can this parameter be nullptr? I think this is passed to the read function in the first parameter.
Comment 4 Fujii Hironori 2018-09-11 19:18:09 PDT
Comment on attachment 349389 [details]
Patch

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

>> Tools/ImageDiff/cairo/PlatformImageCairo.cpp:40
>> +        }, nullptr);
> 
> Can this parameter be nullptr? I think this is passed to the read function in the first parameter.

I don't understand. I removed the unnecessary first argument.

This is the spec.
https://www.cairographics.org/manual/cairo-PNG-Support.html#cairo-read-func-t
Comment 5 Fujii Hironori 2018-09-11 19:30:12 PDT
Just for the record. MSVC also reports following warnings in this source file:

> 10>c:\webkit\ga\tools\imagediff\cairo\platformimagecairo.cpp(46): warning C4838: conversion from 'size_t' to 'unsigned long' requires a narrowing conversion
> 10>c:\webkit\ga\tools\imagediff\cairo\platformimagecairo.cpp(125): warning C4477: 'fprintf' : format string '%Iu' requires an argument of type 'unsigned __int64', but variadic argument 1 has type 'unsigned long'
> 10>c:\webkit\ga\tools\imagediff\cairo\platformimagecairo.cpp(125): note: consider using '%lu' in the format string
Comment 6 Fujii Hironori 2018-09-11 21:50:54 PDT
Comment on attachment 349389 [details]
Patch

Clearing flags on attachment: 349389

Committed r235929: <https://trac.webkit.org/changeset/235929>
Comment 7 Fujii Hironori 2018-09-11 21:50:57 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-09-11 21:51:28 PDT
<rdar://problem/44370981>
Comment 9 Frédéric Wang (:fredw) 2018-09-12 09:01:19 PDT
Comment on attachment 349389 [details]
Patch

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

>>> Tools/ImageDiff/cairo/PlatformImageCairo.cpp:40
>>> +        }, nullptr);
>> 
>> Can this parameter be nullptr? I think this is passed to the read function in the first parameter.
> 
> I don't understand. I removed the unnecessary first argument.
> 
> This is the spec.
> https://www.cairographics.org/manual/cairo-PNG-Support.html#cairo-read-func-t

Note that this is causing compilation warning now that imageSize is unused. I think you can just write "PlatformImage::createFromStdin(size_t)" now.
Comment 10 Fujii Hironori 2018-09-12 13:43:14 PDT
Thank you for letting me know. I will fix.
Comment 11 Fujii Hironori 2018-09-12 19:05:22 PDT
Committed r235965: <https://trac.webkit.org/changeset/235965>