Closed Bug 761510 Opened 10 years ago Closed 10 years ago

rm JSFUN_NULL_CLOSURE

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: luke, Assigned: luke)

Details

(Whiteboard: [js:t])

Attachments

(2 files, 1 obsolete file)

I don't think it serves a purpose anymore.  Also, almost all of the rest of SemanticAnalysis.cpp can be removed with this.
Whiteboard: [js:t]
Note to self: with SetFunctionKinds removed, remove PNK_UPVARS and free lexdeps eagerly.
Step 1: remove the flag.
Attachment #641654 - Flags: review?(jimb)
Mmmm, tasy.
Attachment #641681 - Flags: review?(jimb)
*tasty, even.
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+
Attached patch rm PN_NAMESETSplinter Review
jimb pointed out that patch 2 makes PN_NAMESET dead as well!
Attachment #643184 - Flags: review?(jimb)
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+
Attachment #643184 - Flags: review?(jimb) → review+
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.
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
Attachment #641681 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/6a96719b7acf
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.