Bug 157213

Summary: ToThis should be able to be eliminated in Constant Folding
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Keith Miller 2016-04-29 16:16:37 PDT
ToThis should be able to be eliminated in Constant Folding
Comment 1 Keith Miller 2016-04-29 20:44:51 PDT
Created attachment 277790 [details]
Patch
Comment 2 Saam Barati 2016-05-01 16:11:25 PDT
Comment on attachment 277790 [details]
Patch

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

r=me

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:146
> +            ASSERT(!isStrictMode ? (type.type() != StringType && type.type() != SymbolType) : true);

I think this is easier to read as:
if (!strictMode)
    ASSERT(type.type() != StringType && type.type() != SymbolType)
Comment 3 Keith Miller 2016-05-02 09:38:44 PDT
Created attachment 277910 [details]
Patch for landing
Comment 4 Keith Miller 2016-05-02 09:40:03 PDT
Comment on attachment 277790 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:146
>> +            ASSERT(!isStrictMode ? (type.type() != StringType && type.type() != SymbolType) : true);
> 
> I think this is easier to read as:
> if (!strictMode)
>     ASSERT(type.type() != StringType && type.type() != SymbolType)

I agree, although I actually changed it to:
if (!isStrictMode)
    ASSERT(type.isObjet());
Comment 5 WebKit Commit Bot 2016-05-02 10:37:42 PDT
Comment on attachment 277910 [details]
Patch for landing

Clearing flags on attachment: 277910

Committed r200325: <http://trac.webkit.org/changeset/200325>
Comment 6 WebKit Commit Bot 2016-05-02 10:37:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Geoffrey Garen 2016-05-02 11:20:06 PDT
Comment on attachment 277910 [details]
Patch for landing

Seems like we want a performance regression test for this improvement too.