Bug 136725 - Move JSScope out of JSFunction into separate JSCallee class
Summary: Move JSScope out of JSFunction into separate JSCallee class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks: 136724
  Show dependency treegraph
 
Reported: 2014-09-10 16:40 PDT by Michael Saboff
Modified: 2014-09-11 19:58 PDT (History)
6 users (show)

See Also:


Attachments
Patch (25.44 KB, patch)
2014-09-10 23:22 PDT, Michael Saboff
oliver: review+
Details | Formatted Diff | Diff
Added speculative fix for EWS build failures (25.46 KB, patch)
2014-09-11 10:21 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2014-09-10 16:40:43 PDT
Split out the JSScope from JSFunction so that we can easily have a JSScope for Program and Eval CallFrames.
Comment 1 Michael Saboff 2014-09-10 23:22:51 PDT
Created attachment 237934 [details]
Patch
Comment 2 Oliver Hunt 2014-09-11 09:21:29 PDT
Comment on attachment 237934 [details]
Patch

You need to update the cake lists, but otherwise yay! r=me
Comment 3 Michael Saboff 2014-09-11 10:21:49 PDT
Created attachment 237963 [details]
Added speculative fix for EWS build failures
Comment 4 Michael Saboff 2014-09-11 14:52:47 PDT
Committed r173541: <http://trac.webkit.org/changeset/173541>
Comment 5 Geoffrey Garen 2014-09-11 16:05:33 PDT
Probably best for JSCallee not to have a create function, since it doesn't make sense to create an object of type JSCallee.
Comment 6 Michael Saboff 2014-09-11 18:22:03 PDT
(In reply to comment #5)
> Probably best for JSCallee not to have a create function, since it doesn't make sense to create an object of type JSCallee.

I thought we agreed that we'd create a JSCallee for program and eval frames and put that object in the JSCallee slot.
Comment 7 Geoffrey Garen 2014-09-11 18:48:28 PDT
> I thought we agreed that we'd create a JSCallee for program and eval frames and put that object in the JSCallee slot.

Do you plan to use the JSCallee type for eval and program? I assumed you would create a subclass for each, like we have for JSFunction.

Side note: These functions probably belong in JSFunction, since they are for functions only:

 51    JS_EXPORT_PRIVATE String name(ExecState*);
 52    JS_EXPORT_PRIVATE String displayName(ExecState*);
 53    const String calculatedDisplayName(ExecState*);
Comment 8 Michael Saboff 2014-09-11 19:36:42 PDT
(In reply to comment #7)
> > I thought we agreed that we'd create a JSCallee for program and eval frames and put that object in the JSCallee slot.
> 
> Do you plan to use the JSCallee type for eval and program? I assumed you would create a subclass for each, like we have for JSFunction.

We can create subclasses, but what would be in the subclass for program, eval (and global exec)?  If you want subclasses just to distinguish use I can do that, but I think it might be more confusing to have the subclasses.

> Side note: These functions probably belong in JSFunction, since they are for functions only:
> 
>  51    JS_EXPORT_PRIVATE String name(ExecState*);
>  52    JS_EXPORT_PRIVATE String displayName(ExecState*);
>  53    const String calculatedDisplayName(ExecState*);

I'll remove these.
Comment 9 Geoffrey Garen 2014-09-11 19:58:35 PDT
Actually, I guess if we're only going to access scope, and not executable, it might be fine not to have subclasses.