752793 - Remove Parser-only flags from TreeContextFlags

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

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.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: 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}.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: 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().

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: 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.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: patch 3, v2: remove TCF_DECL_DESTRUCTURING

The previous patch was missing a piece.

Comment 5

[Luke Wagner [:luke]]

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.

Comment 6

[Nicholas Nethercote [inactive]]

> 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.

Comment 7

[Nicholas Nethercote [inactive]]

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

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/57a6e886c44b
https://hg.mozilla.org/mozilla-central/rev/97eac5652174
https://hg.mozilla.org/mozilla-central/rev/080eca3e7be9