The default bug view has changed. See this FAQ.

Remove Parser-only flags from TreeContextFlags

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
In this bug I will remove the flags in TreeContextFlags that only relate to parsing, and find nicer ways to deal with them, i.e. so they're not exposed to every corner of the front-end.
(Assignee)

Comment 1

5 years ago
Created attachment 621865 [details] [diff] [review]
patch 1: remove TCF_RETURN_{EXPR,VOID}

TCF_RETURN_{EXPR,VOID} are only used in the parser, for issuing
warnings/errors.  This patch removes them from TreeContextFlags (where they
can be seen by BytecodeEmitter!) and puts them in TreeContext as single-bit
fields.

Notes:

- They're initialized to false when a new TreeContext is created for a
  function.

- They're set when a return statement of the appropriate kind is seen.

- They're never cleared.  Therefore the presence of TCF_RETURN_FLAGS in the
  assignment in Parser::statement() isn't necessary.

- They're checked in a couple of places.

- In LeaveFunction(), |funbox->tcflags| *doesn't* need to be assigned them
  -- it's only used for communicating state to the BytecodeEmitter, and
  BytecodeEmitter doesn't use TCF_RETURN_{EXPR,VOID}.
Attachment #621865 - Flags: review?(luke)
(Assignee)

Comment 2

5 years ago
Created attachment 621866 [details] [diff] [review]
patch 2: remove TCF_HAS_FUNCTION_STMT

TCF_HAS_FUNCTION_STMT is also only used in the parser, and its use is so
local that it can be replaced with an outparam in statements().  This is
much easier to understand than the flag;  the old code had to worry about
setting and clearing the flag for nested invocations of statements().
Attachment #621866 - Flags: review?(luke)
(Assignee)

Comment 3

5 years ago
Created attachment 621867 [details] [diff] [review]
patch 3: remove TCF_DECL_DESTRUCTURING

TCF_DECL_DESTRUCTURING is easy -- it's set and then cleared for certain
contexts.  It's easily moved into a bit-field in TreeContext.
Attachment #621867 - Flags: review?(luke)
(Assignee)

Updated

5 years ago
Attachment #621866 - Attachment description: patch 2: → patch 2: remove TCF_DECL_DESTRUCTURING
(Assignee)

Comment 4

5 years ago
Created attachment 621870 [details] [diff] [review]
patch 3, v2: remove TCF_DECL_DESTRUCTURING

The previous patch was missing a piece.
Attachment #621867 - Attachment is obsolete: true
Attachment #621867 - Flags: review?(luke)
Attachment #621870 - Flags: review?(luke)
(Assignee)

Updated

5 years ago
Attachment #621866 - Attachment description: patch 2: remove TCF_DECL_DESTRUCTURING → patch 2: remove TCF_HAS_FUNCTION_STMT
(Assignee)

Updated

5 years ago
Blocks: 752816

Comment 5

5 years ago
Comment on attachment 621865 [details] [diff] [review]
patch 1: remove TCF_RETURN_{EXPR,VOID}

Review of attachment 621865 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/Parser.cpp
@@ +2692,5 @@
>  #endif
> +            tc->hasReturnVoid = true;
> +    }
> +
> +    if (tc->hasReturnExpr && (tc->sc->flags & TCF_FUN_IS_GENERATOR)) {

Aw, but it was so readable before.
Attachment #621865 - Flags: review?(luke) → review+

Updated

5 years ago
Attachment #621866 - Flags: review?(luke) → review+

Updated

5 years ago
Attachment #621870 - Flags: review?(luke) → review+
(Assignee)

Comment 6

5 years ago
> Aw, but it was so readable before.

Tell me about it -- I had to draw a truth table to check that I was understanding the old version correctly.
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/57a6e886c44b
https://hg.mozilla.org/integration/mozilla-inbound/rev/97eac5652174
https://hg.mozilla.org/integration/mozilla-inbound/rev/080eca3e7be9
https://hg.mozilla.org/mozilla-central/rev/57a6e886c44b
https://hg.mozilla.org/mozilla-central/rev/97eac5652174
https://hg.mozilla.org/mozilla-central/rev/080eca3e7be9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.