Bug 136725

Summary: Move JSScope out of JSFunction into separate JSCallee class
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren, gyuyoung.kim, rakuco, ryuan.choi, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 136724    
Attachments:
Description Flags
Patch
oliver: review+
Added speculative fix for EWS build failures none

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.