Bug 238918 – [JSC] DFG / FTL should be aware of JSString's String replacement

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

[Yusuke Suzuki]

[JSC] JSString::value should return String instead of reference

[Yusuke Suzuki]

Created attachment 456881 [details]
Patch

[Yusuke Suzuki]

Created attachment 456955 [details]
Patch

[Zolkarnain bin hashim]

Slick

[Yusuke Suzuki]

Created attachment 457004 [details]
Patch

[Saam Barati]

Comment on attachment 457004 [details]
Patch

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

Nice. r=me

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3795
> +        ASSERT(node->arrayMode().type() != Array::String);

Can we put this in DFGValidate? That is the more canonical place to put such things.

> Source/JavaScriptCore/dfg/DFGClobberize.h:1341
> +        ASSERT(node->arrayMode().type() != Array::String);

ditto. I think just putting it in Validate is the right way

> Source/JavaScriptCore/dfg/DFGOperations.cpp:2524
> +    string->value(globalObject);

why not call JSRopeString->resolveRope here?

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4775
> +        ASSERT(m_node->arrayMode().type() != Array::String);

ditto about validation

[Yusuke Suzuki]

Comment on attachment 457004 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3795
>> +        ASSERT(node->arrayMode().type() != Array::String);
> 
> Can we put this in DFGValidate? That is the more canonical place to put such things.

Done. Nice

>> Source/JavaScriptCore/dfg/DFGClobberize.h:1341
>> +        ASSERT(node->arrayMode().type() != Array::String);
> 
> ditto. I think just putting it in Validate is the right way

Ditto.

>> Source/JavaScriptCore/dfg/DFGOperations.cpp:2524
>> +    string->value(globalObject);
> 
> why not call JSRopeString->resolveRope here?

Sounds good. Changed.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4775
>> +        ASSERT(m_node->arrayMode().type() != Array::String);
> 
> ditto about validation

Nice, done.

[Yusuke Suzuki]

Committed r292697 (249489@trunk): <https://commits.webkit.org/249489@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/91547350>