Bug 169123 - [WinCairo] '__int128_t': undeclared identifier in MediaTime.cpp
Summary: [WinCairo] '__int128_t': undeclared identifier in MediaTime.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on: 169098
Blocks:
  Show dependency treegraph
 
Reported: 2017-03-03 00:27 PST by Fujii Hironori
Modified: 2017-03-06 16:58 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.42 KB, patch)
2017-03-03 10:20 PST, Jer Noble
achristensen: review+
Details | Formatted Diff | Diff
Patch with CMake checks for HAVE (3.03 KB, patch)
2017-03-03 11:18 PST, Don Olmstead
achristensen: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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