Bug 179706 – Add a compile-time-checked string literal initializer for FourCC.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Add a compile-time-checked string literal initializer for FourCC.

[Jer Noble]

Created attachment 326941 [details]
Patch

[Michael Catanzaro]

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]

(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]

Created attachment 326986 [details]
Patch for landing

[Jer Noble]

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 on attachment 326986 [details]
Patch for landing

Clearing flags on attachment: 326986

Committed r224887: <https://trac.webkit.org/changeset/224887>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35566955>

[Michael Catanzaro]

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]

(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]

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)