Bug 120781 - REGRESSION(149636, merged in 153145): ToThis conversion doesn't work in the DFG
Summary: REGRESSION(149636, merged in 153145): ToThis conversion doesn't work in the DFG
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 115542
  Show dependency tree / graph
 
Reported: 2013-09-05 11:08 PDT by Filip Pizlo
Modified: 2013-09-06 12:00 PDT (History)
7 users (show)

See Also:

Attachments
the patch (10.47 KB, patch)
2013-09-05 11:11 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (11.10 KB, patch)
2013-09-05 11:36 PDT, Filip Pizlo 		mhahnenberg: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-09-05 11:08:03 PDT 
Patch forthcoming.
Comment 1 Filip Pizlo 2013-09-05 11:11:10 PDT 
Created attachment 210638 [details]
the patch
Comment 2 Filip Pizlo 2013-09-05 11:36:14 PDT 
Created attachment 210640 [details]
the patch
Comment 3 Oliver Hunt 2013-09-05 12:38:58 PDT 
Comment on attachment 210640 [details]
the patch

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

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1089
> -        destination.merge(SpecObjectOther);
> +        destination.merge(SpecObject);

Do we end up with ToThis nodes in strict mode?  If we do ToThis can be a primitive as it won't be boxed (this conversion is only for the global object -> global object proxy).
Comment 4 Filip Pizlo 2013-09-05 13:14:19 PDT 
(In reply to comment #3)
> (From update of attachment 210640 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210640&action=review
> 
> > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1089
> > -        destination.merge(SpecObjectOther);
> > +        destination.merge(SpecObject);
> 
> Do we end up with ToThis nodes in strict mode?  If we do ToThis can be a primitive as it won't be boxed (this conversion is only for the global object -> global object proxy).

Hence the merge.  If the value was a primitive before, this will say "primitive or object".  If it was TOP before, then this will still say TOP.
Comment 5 Mark Hahnenberg 2013-09-05 13:24:22 PDT 
Comment on attachment 210640 [details]
the patch

r=me
Comment 6 Filip Pizlo 2013-09-05 14:18:23 PDT 
Landed in http://trac.webkit.org/changeset/155149
Comment 7 Mark Rowe (bdash) 2013-09-05 18:08:16 PDT 
This broke the build so I rolled it out in r155166.
Comment 8 Filip Pizlo 2013-09-06 12:00:39 PDT 
Relanded in http://trac.webkit.org/changeset/155201