Closed
Bug 761510
Opened 12 years ago
Closed 12 years ago
rm JSFUN_NULL_CLOSURE
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: luke, Assigned: luke)
Details
(Whiteboard: [js:t])
Attachments
(2 files, 1 obsolete file)
9.52 KB,
patch
|
jimb
:
review+
|
Details | Diff | Splinter Review |
7.83 KB,
patch
|
jimb
:
review+
|
Details | Diff | Splinter Review |
I don't think it serves a purpose anymore. Also, almost all of the rest of SemanticAnalysis.cpp can be removed with this.
Updated•12 years ago
|
Whiteboard: [js:t]
Assignee | ||
Comment 1•12 years ago
|
||
Note to self: with SetFunctionKinds removed, remove PNK_UPVARS and free lexdeps eagerly.
Assignee | ||
Comment 2•12 years ago
|
||
Step 1: remove the flag.
Attachment #641654 -
Flags: review?(jimb)
Assignee | ||
Comment 4•12 years ago
|
||
*tasty, even.
Comment 5•12 years ago
|
||
Comment on attachment 641654 [details] [diff] [review] rm JSFUN_NULL_CLOSURE Review of attachment 641654 [details] [diff] [review]: ----------------------------------------------------------------- It is a good day.
Attachment #641654 -
Flags: review?(jimb) → review+
Assignee | ||
Comment 6•12 years ago
|
||
jimb pointed out that patch 2 makes PN_NAMESET dead as well!
Attachment #643184 -
Flags: review?(jimb)
Comment 7•12 years ago
|
||
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.
Attachment #641681 -
Flags: review?(jimb) → review+
Updated•12 years ago
|
Attachment #643184 -
Flags: review?(jimb) → review+
Assignee | ||
Comment 8•12 years ago
|
||
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.
Assignee | ||
Comment 9•12 years ago
|
||
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. https://hg.mozilla.org/integration/mozilla-inbound/rev/6a96719b7acf
Target Milestone: --- → mozilla17
Assignee | ||
Updated•12 years ago
|
Attachment #641681 -
Attachment is obsolete: true
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6a96719b7acf
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 11•12 years ago
|
||
`fun` was unused in SetFunctionKinds, so I landed https://hg.mozilla.org/integration/mozilla-inbound/rev/4c75b59971a6
You need to log in
before you can comment on or make changes to this bug.
Description
•