rm JSFUN_NULL_CLOSURE

RESOLVED FIXED in mozilla17

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:t])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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]
(Assignee)

Comment 1

5 years ago
Note to self: with SetFunctionKinds removed, remove PNK_UPVARS and free lexdeps eagerly.
(Assignee)

Comment 2

5 years ago
Created attachment 641654 [details] [diff] [review]
rm JSFUN_NULL_CLOSURE

Step 1: remove the flag.
Attachment #641654 - Flags: review?(jimb)
(Assignee)

Comment 3

5 years ago
Created attachment 641681 [details] [diff] [review]
rm SetFunctionKinds and PNK_UPVARS

Mmmm, tasy.
Attachment #641681 - Flags: review?(jimb)
(Assignee)

Comment 4

5 years ago
*tasty, even.

Comment 5

5 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

5 years ago
Created attachment 643184 [details] [diff] [review]
rm PN_NAMESET

jimb pointed out that patch 2 makes PN_NAMESET dead as well!
Attachment #643184 - Flags: review?(jimb)

Comment 7

5 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

5 years ago
Attachment #643184 - Flags: review?(jimb) → review+
(Assignee)

Comment 8

5 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

5 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

5 years ago
Attachment #641681 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/6a96719b7acf
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
`fun` was unused in SetFunctionKinds, so I landed

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c75b59971a6
https://hg.mozilla.org/mozilla-central/rev/4c75b59971a6
You need to log in before you can comment on or make changes to this bug.