1164777 - super.prop erroneously disallowed inside eval inside arrow functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Eric Faust [:efaust]]

The specese reads:

It is a Syntax Error if StatementList Contains super unless the source code containing super is eval code that is being processed by a direct eval that is contained in function code that is not the function code of an ArrowFunction.

To my read, this "is contained in" is not a formal function call, but rather means "it is within a non-arrow function". This means that eval inside an arrow function should work.

Comment 1

[Eric Faust [:efaust]]

Jason, does this interpretation seem correct to you?

Comment 2

[Eric Faust [:efaust]]

Attachment: Part 1: Move evalStaticScope from BCE to GlobalSharedContext

Comment 3

[Eric Faust [:efaust]]

Attachment: Part 2: Allow super.prop inside eval inside arrow functons.

Comment 4

[Eric Faust [:efaust]]

(In reply to Eric Faust [:efaust] from comment #0)
> The specese reads:
> 
> It is a Syntax Error if StatementList Contains super unless the source code
> containing super is eval code that is being processed by a direct eval that
> is contained in function code that is not the function code of an
> ArrowFunction.
> 
> To my read, this "is contained in" is not a formal function call, but rather
> means "it is within a non-arrow function". This means that eval inside an
> arrow function should work.

Rather, this should work as long as the immediately enclosing arrow function is eventually enclosed by a non-arrow function. So eval inside an arrow function inside a method should work, for example.

Comment 5

[Shu-yu Guo [:shu]]

Comment on attachment 8605702 [details] [diff] [review]
Part 1: Move evalStaticScope from BCE to GlobalSharedContext

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

Nice refactor.

::: js/src/frontend/Parser.cpp
@@ +699,5 @@
>       */
>      Directives directives(options().strictOption);
> +    GlobalSharedContext globalsc(context, directives,
> +                                 /* evalScope = */ js::NullPtr(),
> +                                 options().extraWarningsOption,

Side question: is this function only used by Reflect.parse?

::: js/src/frontend/SharedContext.h
@@ +235,5 @@
>  
>  class GlobalSharedContext : public SharedContext
>  {
>    private:
> +    Handle<StaticEvalObject*> evalScope_;

Nit: I'd prefer this to be named staticEvalScope_.

@@ +249,5 @@
>      {}
>  
>      ObjectBox* toObjectBox() { return nullptr; }
>      bool allowSuperProperty() const { return allowSuperProperty_; }
> +    HandleObject evalStaticScope() const { return evalScope_; }

Ditto, I'd prefer this to be named staticEvalScope()

Comment 6

[Jason Orendorff [:jorendorff]]

(In reply to Eric Faust [:efaust] from comment #1)
> Jason, does this interpretation seem correct to you?

Hmm. The specese looks pretty dubious, but your interpretation is the only thing that makes any sense to me. +1.

super.method() works inside eval inside arrow functions inside methods in anba's es6draft, FWIW.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/63914ec87ec5
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc84232705fe

Comment 8

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/03e1398dea10

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/18b28733ec14
https://hg.mozilla.org/integration/mozilla-inbound/rev/893528aa073c

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/18b28733ec14
https://hg.mozilla.org/mozilla-central/rev/893528aa073c