Bug 179706

Summary: Add a compile-time-checked string literal initializer for FourCC.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, commit-queue, don.olmstead, mcatanzaro, pvollan, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=179750
Bug Depends on: 179499    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch for landing none

Jer Noble
Reported 2017-11-14 16:34:14 PST
Add a compile-time-checked string literal initializer for FourCC.
Attachments
Patch (7.18 KB, patch)
2017-11-14 16:37 PST, Jer Noble 		no flags
DetailsFormatted DiffDiff
Patch for landing (8.23 KB, patch)
2017-11-15 08:47 PST, Jer Noble 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jer Noble
Comment 1 2017-11-14 16:37:16 PST
Created attachment 326941 [details] Patch
Michael Catanzaro
Comment 2 2017-11-14 19:13:30 PST
Comment on attachment 326941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326941&action=review > Source/WebCore/platform/graphics/FourCC.h:36 > + template<std::size_t N> > + constexpr FourCC(const char (&data)[N]) Oh wow, clever. EWS is green, but I'll test it with GCC soon anyway. Thanks!
Michael Catanzaro
Comment 3 2017-11-14 20:12:15 PST
(In reply to Michael Catanzaro from comment #2) > EWS is green, but I'll test it with GCC soon anyway. Thanks! You missed ISOVTTCue.cpp.
Jer Noble
Comment 4 2017-11-15 08:47:38 PST
Created attachment 326986 [details] Patch for landing
Jer Noble
Comment 5 2017-11-15 09:02:39 PST
Just for reference, I verified with godbolt.org that the following (made up) function: bool isSettingsBox(uint32_t name) { return FourCC(name) == vttSettingsBoxType(); } Compiles down to to a single assembly integer equality test with -O3: isSettingsBox(unsigned int): cmp edi, 1937011815 sete al ret
WebKit Commit Bot
Comment 6 2017-11-15 11:06:56 PST
Comment on attachment 326986 [details] Patch for landing Clearing flags on attachment: 326986 Committed r224887: <https://trac.webkit.org/changeset/224887>
WebKit Commit Bot
Comment 7 2017-11-15 11:06:58 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2017-11-15 12:06:42 PST
<rdar://problem/35566955>
Michael Catanzaro
Comment 9 2017-11-15 13:23:19 PST
It broke the build with GCC 5. I think the best option is to add a fallback non-constexpr constructor just for GCC 5, with a note to remove it when we require GCC 6 (which is planned for May). I'm testing a patch now.
Don Olmstead
Comment 10 2017-11-15 18:13:03 PST
(In reply to Michael Catanzaro from comment #9) > It broke the build with GCC 5. I think the best option is to add a fallback > non-constexpr constructor just for GCC 5, with a note to remove it when we > require GCC 6 (which is planned for May). I'm testing a patch now. Also seems like WinCairo with VS2015 does not like this either. Complains that not all values are initialized. Bots seem fine. Not sure why AppleWin isn't having an issue. Really need to move to VS2017 across the board
Don Olmstead
Comment 11 2017-11-15 18:16:58 PST
Seems like AppleWin hit this as well https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/6072/steps/compile-webkit/logs/stdio C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\FourCC.h(36): error C2476: 'constexpr' constructor does not initialize all members (compiling source file C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\avfoundation\cf\InbandTextTrackPrivateAVCF.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\FourCC.h(48): note: 'WebCore::FourCC::value' was not initialized by the constructor (compiling source file C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\avfoundation\cf\InbandTextTrackPrivateAVCF.cpp) C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\iso\ISOVTTCue.h(45): note: see reference to function template instantiation 'WebCore::FourCC::FourCC<5>(const char (&)[5])' being compiled (compiling source file C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\avfoundation\cf\InbandTextTrackPrivateAVCF.cpp)
Note You need to log in before you can comment on or make changes to this bug.