Bug 169123

Summary: [WinCairo] '__int128_t': undeclared identifier in MediaTime.cpp
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: Web Template FrameworkAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, Basuke.Suzuki, don.olmstead, jer.noble
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 169098    
Bug Blocks:    
Attachments:
Description Flags
Patch
achristensen: review+
Patch with CMake checks for HAVE achristensen: review-

Description Fujii Hironori 2017-03-03 00:27:20 PST 
[WinCairo] '__int128_t': undeclared identifier in MediaTime.cpp

WinCairo x64 build fails since r213324 (Bug 169098).

> [6/1400] Building CXX object Source\WTF\wtf\CMakeFiles\WTF.dir\MediaTime.cpp.obj
> FAILED: C:\PROGRA~2\MICROS~3.0\VC\bin\amd64\cl.exe   /nologo /TP /DWIN32 /D_WINDOWS /W4 /GR- /EHs- /EHc-  /MT /O2 /Ob2 /D NDEBUG -IDerivedSources\ForwardingHeaders -IDerivedSources -I..\..\WebKitLibraries\win\include -I..\..\Source\bmalloc -I..\..\Source\WTF -I..\..\Source\WTF\wtf -I..\..\Source\WTF\wtf\dtoa -I..\..\Source\WTF\wtf\persistence -I..\..\Source\WTF\wtf\text -I..\..\Source\WTF\wtf\text\icu -I..\..\Source\WTF\wtf\threads -I..\..\Source\WTF\wtf\unicode -I..\..\Source\ThirdParty -I.    /wd4018 /wd4068 /wd4099 /wd4100 /wd4127 /wd4138 /wd4146 /wd4180 /wd4189 /wd4201 /wd4206 /wd4244 /wd4251 /wd4267 /wd4275 /wd4288 /wd4291 /wd4305 /wd4309 /wd4344 /wd4355 /wd4389 /wd4396 /wd4456 /wd4457 /wd4458 /wd4459 /wd4481 /wd4503 /wd4505 /wd4510 /wd4512 /wd4530 /wd4610 /wd4611 /wd4646 /wd4702 /wd4706 /wd4722 /wd4800 /wd4819 /wd4951 /wd4952 /wd4996 /wd6011 /wd6031 /wd6211 /wd6246 /wd6255 /wd6387 /Zi /GS /EHa- /EHc- /EHs- /fp:except- /analyze- /bigobj /Gy- /openmp- /GF- /Oy- /showIncludes -DBUILDING_WITH_CMAKE=1 -DBUILDING_WTF -DHAVE_CONFIG_H=1 -DNOMINMAX -DUNICODE -DWINVER=0x601 -DWTF_EXPORTS -DWTF_PLATFORM_WIN_CAIRO=1 -D_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES=1 -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_UNICODE -D_WINDOWS /FoSource\WTF\wtf\CMakeFiles\WTF.dir\MediaTime.cpp.obj /FdSource\WTF\wtf\CMakeFiles\WTF.dir\ /FS -c ..\..\Source\WTF\wtf\MediaTime.cpp
> ..\..\Source\WTF\wtf\MediaTime.cpp(493): error C2065: '__int128_t': undeclared identifier
> ..\..\Source\WTF\wtf\MediaTime.cpp(493): error C2146: syntax error: missing ';' before identifier 'newValue'
> ..\..\Source\WTF\wtf\MediaTime.cpp(493): error C2065: 'newValue': undeclared identifier
> ..\..\Source\WTF\wtf\MediaTime.cpp(493): error C2061: syntax error: identifier '__int128_t'
> ..\..\Source\WTF\wtf\MediaTime.cpp(494): error C2065: 'newValue': undeclared identifier
> ..\..\Source\WTF\wtf\MediaTime.cpp(495): error C2065: 'newValue': undeclared identifier
> ..\..\Source\WTF\wtf\MediaTime.cpp(497): error C2065: 'newValue': undeclared identifier
> ..\..\Source\WTF\wtf\MediaTime.cpp(502): error C2065: 'newValue': undeclared identifier
> ..\..\Source\WTF\wtf\MediaTime.cpp(518): error C2065: 'newValue': undeclared identifier
> ninja: build stopped: subcommand failed.
Comment 1 Jer Noble 2017-03-03 10:20:30 PST 
Created attachment 303327 [details]
Patch
Comment 2 Jer Noble 2017-03-03 10:21:45 PST 
Would someone with access to a 64-bit Windows builder please try out this patch and verify that it fixes the compiler error?
Comment 3 Don Olmstead 2017-03-03 10:25:17 PST 
Jer Noble would a more effective patch be to actually test for the presence of int128_t? I was looking at this and was thinking of doing a HAVE version of it.
Comment 4 Jer Noble 2017-03-03 10:57:39 PST 
(In reply to comment #3)
> Jer Noble would a more effective patch be to actually test for the presence
> of int128_t? I was looking at this and was thinking of doing a HAVE version
> of it.

I couldn't find a directive I could use for such a test; if you can, more the better!
Comment 5 Don Olmstead 2017-03-03 11:18:35 PST 
Created attachment 303329 [details]
Patch with CMake checks for HAVE

Here's a version that checks for the presence of __int128_t through CMake.
Comment 6 Alex Christensen 2017-03-03 14:17:10 PST 
Comment on attachment 303329 [details]
Patch with CMake checks for HAVE

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

> Source/WTF/wtf/Platform.h:553
>  #if CPU(X86_64)
>  #define HAVE_NETWORK_EXTENSION 1
>  #define USE_PLUGIN_HOST_PROCESS 1
> +#define HAVE_INT128_T 1

I don't think this properly reflects the fact that Visual Studio doesn't support the __int128_t type.
Comment 7 Don Olmstead 2017-03-03 14:18:27 PST 
All the changes to Platform.h are for the Mac side since its not using CMake.
Comment 8 Alex Christensen 2017-03-04 00:21:24 PST 
Sorry, Don.  Jer's fix passed EWS and was simpler.  If we feel it's worth it to optimize this for Windows, we can do so later, but we need to fix the build.
Comment 9 Fujii Hironori 2017-03-06 16:58:08 PST 
Landed manually.

https://trac.webkit.org/changeset/213419