Bug 127161 – Get rid of OpaqueJSString::deprecatedCharacters()

• Component: New Bugs
• Status: RESOLVED FIXED

[Anders Carlsson]

Get rid of OpaqueJSString::deprecatedCharacters()

[Anders Carlsson]

Created attachment 221440 [details]
Patch

[Sam Weinig]

Comment on attachment 221440 [details]
Patch

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

> Source/JavaScriptCore/API/OpaqueJSString.cpp:45
> +    if (UChar* characters = m_characters)
> +        fastFree(static_cast<void*>(characters));

You should add a comment about why you are putting this in a local.

> Source/JavaScriptCore/API/OpaqueJSString.cpp:78
> +    UChar* characters = m_characters;
> +    if (characters)
> +        return characters;

Same here.

[Anders Carlsson]

Committed r162185: <http://trac.webkit.org/changeset/162185>

[Alexey Proskuryakov]

This broke Windows build:

     1>c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\include\private\javascriptcore\X86Assembler.h(2348): warning C4309: 'argument' : truncation of constant value
     1>c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(97): error C2039: 'atomic' : is not a member of 'std'
     1>c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(97): error C2143: syntax error : missing ';' before '<'
     1>c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(97): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
     1>c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(97): error C2238: unexpected token(s) preceding ';'
     1>c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(79): error C2614: 'OpaqueJSString' : illegal member initialization: 'm_characters' is not a base or member
     1>c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(85): error C2614: 'OpaqueJSString' : illegal member initialization: 'm_characters' is not a base or member
     1>c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(91): error C2614: 'OpaqueJSString' : illegal member initialization: 'm_characters' is not a base or member

[Alexey Proskuryakov]

And Mac too:


Undefined symbols for architecture x86_64:
  "__ZN14OpaqueJSStringD1Ev", referenced from:
      __ZN3WTF20ThreadSafeRefCountedI14OpaqueJSStringE5derefEv in ScriptCachedFrameData.o
      __ZN3WTF20ThreadSafeRefCountedI14OpaqueJSStringE5derefEv in ScriptController.o

Undefined symbols for architecture i386:
  "__ZN14OpaqueJSStringD1Ev", referenced from:
      __ZN24OpaqueJSClassContextDataD2Ev in ScriptCachedFrameData.o

[Alexey Proskuryakov]

...and it also fails JSC API test:

2014-01-16 23:47:39.670 testapi[96811:507] myObj2.currentThis: [object Object]
ASSERTION FAILED: JSStringGetCharactersPtr(constantStringRef) == constantString
/Volumes/Data/slave/mavericks-release/build/Source/JavaScriptCore/API/tests/testapi.c(1263) : int main(int, char **)
1   0x7fff8e72a5fd start
2   0x1

http://build.webkit.org/builders/Apple%20Mavericks%20Release%20WK1%20(Tests)/builds/2483/steps/jscore-test/logs/stdio

[WebKit Commit Bot]

Re-opened since this is blocked by bug 127164

[Alexey Proskuryakov]

Looks like this really breaks semantics of JSStringCreateWithCharactersNoCopy, rolling out :(

[Anders Carlsson]

Created attachment 221471 [details]
Patch

[Anders Carlsson]

Created attachment 221472 [details]
Patch

[Anders Carlsson]

Committed r162205: <http://trac.webkit.org/changeset/162205>

[Alexey Proskuryakov]

Comment on attachment 221472 [details]
Patch

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

> Source/JavaScriptCore/API/OpaqueJSString.h:98
> +    // This will be initialized on demand when characters() is called.

This comment seems off now.

[Alexey Proskuryakov]

And this broke build on Windows (EWS also said so):

     1>C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\xxatomic(229): error C2664: 'std::_Atomic_address &std::_Atomic_address::operator =(const std::_Atomic_address &)' : cannot convert argument 1 from 'const UChar *' to 'void *'
                 Conversion loses qualifiers
                 C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\xxatomic(227) : while compiling class template member function 'std::atomic<const UChar *>::atomic(_Ty *) throw()'
                 with
                 [
                     _Ty=const UChar
                 ]
                 c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(75) : see reference to function template instantiation 'std::atomic<const UChar *>::atomic(_Ty *) throw()' being compiled
                 with
                 [
                     _Ty=const UChar
                 ]
                 c:\cygwin\home\buildbot\slave\win-release\build\source\javascriptcore\api\OpaqueJSString.h(99) : see reference to class template instantiation 'std::atomic<const UChar *>' being compiled