Bug 205147 – DFG and FTL expects String.prototype to not qualify for StringObjectUse.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Currently, String.prototype's JSType is StringObjectType.

However, in the compiler, there are a few places that expect that the String.prototype value to not qualify as StringObjectUse.  These places are:
1. SpeculatedType.cpp's speculationFromClassInfo() will speculate SpecObjectOther for the StringPrototype object.
2. DFGFixupPhase.cpp's addCheckStructureForOriginalStringObjectUse() only does a CheckStructure against globalObject->stringObjectStructure().  It does not check against String.prototype's structure.

To resolve this discrepancy, we can either do:
a. change String.prototype's JSType to something else.
b. fix the places in the compiler to accept String.prototype as StringObjectUse.

(a) is trivial and cheap to do.  (b) is doable but will result in less optimal compiled code.  Since passing String.prototype as a StringObject is expected to be a rare thing in JS code, it's not worth incurring the cost for (b).  We'll apply (a).

<rdar://problem/57748888>

[Mark Lam]

Created attachment 385465 [details]
proposed patch.

[Saam Barati]

Comment on attachment 385465 [details]
proposed patch.

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

> Source/JavaScriptCore/ChangeLog:3
> +        FTL expects String.prototype to not qualify for StringObjectUse.

it's not just the FTL. It's DFG+FTL

> Source/JavaScriptCore/runtime/JSType.h:120
> +    // Start StringObjectType types.

this seems unnecessary

> Source/JavaScriptCore/runtime/JSType.h:123
> +    // End StringObjectType types.

ditto

No need to call something out we don't rely on IMO

[Saam Barati]

Comment on attachment 385465 [details]
proposed patch.

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

>> Source/JavaScriptCore/runtime/JSType.h:123
>> +    // End StringObjectType types.
> 
> ditto
> 
> No need to call something out we don't rely on IMO

ignore me

[Mark Lam]

Created attachment 385466 [details]
patch for landing.

Thanks for the review.

[Mark Lam]

The Win EWS bot failures are just due to flakiness.  A re-run of the EWS bot produced different failures, and the failure cannot be due to this change.

Landed in r253432: <http://trac.webkit.org/r253432>.