[webkit-reviews] review denied: [Bug 171800] [Win] StaticStringImpl in HTMLNames.cpp aren't constructed : [Attachment 309480] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <utatane.tea at gmail.com> has denied Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 171800: [Win] StaticStringImpl in HTMLNames.cpp aren't constructed
https://bugs.webkit.org/show_bug.cgi?id=171800

Attachment 309480: Patch

https://bugs.webkit.org/attachment.cgi?id=309480&action=review




--- Comment #3 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 309480
  --> https://bugs.webkit.org/attachment.cgi?id=309480
Patch

Hmm, I think we should fix this issue by ensuring StaticStringImpl is correctly
initialized at compile time in Windows even in it is a global variable.
Current fix is moving it to static variables in functions. It does not work
well for StringImpl::empty()'s static string.
And we don't want to make it static string inside function because we cannot
make StringImpl::empty() inline-function.

Since assertHashIsCorrect() exists before Bug 171586, I guess Bug 171586 change
makes these strings non-compile-time-initialized in Windows.
Can you ensure that this `assertHashIsCorrect()` does not fire before Bug
171586?
We can guarantee that they are compile-time-initialized in OSX since we fail to
compile if the code includes global constructor invocations.

If we can ensure that this `assertHashIsCorrect()` does not fire before Bug
171586, we can assume that Bug 171586 changes these strings
non-compile-time-initialized thing in Windows. And I think the right fix is
changing them compile-time-string again (as is in OSX).
I guess non-constexpr StringImplShape is called from StaticStringImpl
constructor.