Last Comment Bug 761510 - rm JSFUN_NULL_CLOSURE
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla17
Assigned To: Luke Wagner [:luke]
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2012-06-04 22:44 PDT by Luke Wagner [:luke]
Modified: 2012-07-24 03:03 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

rm JSFUN_NULL_CLOSURE (9.52 KB, patch)
2012-07-12 16:51 PDT, Luke Wagner [:luke]
jimb: review+
Details | Diff | Splinter Review
rm SetFunctionKinds and PNK_UPVARS (14.08 KB, patch)
2012-07-12 18:16 PDT, Luke Wagner [:luke]
jimb: review+
Details | Diff | Splinter Review
rm PN_NAMESET (7.83 KB, patch)
2012-07-17 16:34 PDT, Luke Wagner [:luke]
jimb: review+
Details | Diff | Splinter Review

Description User image Luke Wagner [:luke] 2012-06-04 22:44:32 PDT
I don't think it serves a purpose anymore.  Also, almost all of the rest of SemanticAnalysis.cpp can be removed with this.
Comment 1 User image Luke Wagner [:luke] 2012-06-22 18:42:40 PDT
Note to self: with SetFunctionKinds removed, remove PNK_UPVARS and free lexdeps eagerly.
Comment 2 User image Luke Wagner [:luke] 2012-07-12 16:51:25 PDT
Created attachment 641654 [details] [diff] [review]

Step 1: remove the flag.
Comment 3 User image Luke Wagner [:luke] 2012-07-12 18:16:54 PDT
Created attachment 641681 [details] [diff] [review]
rm SetFunctionKinds and PNK_UPVARS

Mmmm, tasy.
Comment 4 User image Luke Wagner [:luke] 2012-07-12 18:17:09 PDT
*tasty, even.
Comment 5 User image Jim Blandy :jimb 2012-07-16 16:11:20 PDT
Comment on attachment 641654 [details] [diff] [review]

Review of attachment 641654 [details] [diff] [review]:

It is a good day.
Comment 6 User image Luke Wagner [:luke] 2012-07-17 16:34:11 PDT
Created attachment 643184 [details] [diff] [review]

jimb pointed out that patch 2 makes PN_NAMESET dead as well!
Comment 7 User image Jim Blandy :jimb 2012-07-17 19:02:49 PDT
Comment on attachment 641681 [details] [diff] [review]
rm SetFunctionKinds and PNK_UPVARS

Review of attachment 641681 [details] [diff] [review]:

It looks to me as if PNK_UPVARS is the only user of the nameset arity; so that's a bunch more stuff that might as well go too.

::: js/src/frontend/ParseNode.h
@@ +822,5 @@
>      bool isGeneratorExpr() const {
>          if (getKind() == PNK_LP) {
>              ParseNode *callee = this->pn_head;
>              if (callee->getKind() == PNK_FUNCTION) {
> +                ParseNode *body = callee->pn_body;

If you like, I think it would be okay to just write 'callee->pn_body->getKind() == ...' here, and perhaps combine the inner if with the enclosing if. The temporary variable doesn't seem to serve any purpose any more.
Comment 8 User image Luke Wagner [:luke] 2012-07-18 14:43:16 PDT
D'oh, I realized that the existing patch makes enclosing functions heavyweight if any nested function contains a free variable.  For example, in:
  function f() { (function g() { x++ })() }
f will be marked heavyweight.  This is because g creates a placeholder which is marked as 'closed' when it is hoisted into f.  However, it is too early to say whether f contains a definition of 'x' yet.

I think the solution is to mark functions as having closed variables when they have finished parsing.  This would also give us a simple place to compile the list of upvars instead of the scattered noteClosedVar/noteClosedArg stuff.
Comment 9 User image Luke Wagner [:luke] 2012-07-18 15:09:10 PDT
Hrm, the solution to this will fit nicely into bug 775323, so I'll land the JSFUN_NULL_CLOSURE-removal now and roll the SetFunctionKinds-removal into bug 775323.
Comment 10 User image Ed Morley [:emorley] 2012-07-19 07:33:01 PDT
Comment 11 User image :Ms2ger (⌚ UTC+1/+2) 2012-07-23 04:59:12 PDT
`fun` was unused in SetFunctionKinds, so I landed
Comment 12 User image Ed Morley [:emorley] 2012-07-24 03:03:22 PDT

Note You need to log in before you can comment on or make changes to this bug.