[es6] Make ParseSuperExpression uses scopes instead

[es6] Make ParseSuperExpression use scopes instead

Previously it was using the FunctionState which does not work
correctly for lazy arrow functions.

To get this to work I had to fix some issues with some Scopes not
setting the function kind correctly.

Also, this removes innner_uses_super since we can track that on the
correct scope directly.

BUG=v8:4031
LOG=N
R=wingo@igalia.com, rossberg@chromium.org

[arv (Not doing code reviews), 2015-04-23 16:28:11]

arv@chromium.org changed reviewers:
+ rossberg@chromium.org, wingo@igalia.com

[arv (Not doing code reviews), 2015-04-23 16:29:49]

Andreas, it is not clear how to fix the strong mode todo I added in
ParseSuperExpression.

Andy, did the lexical this CL land? If so I think super.prop in arrows should
work now. I'll add tests if that is the case.

[arv (Not doing code reviews), 2015-04-23 16:38:33]

cleanup

[wingo, 2015-04-24 06:31:45]

On 2015/04/23 16:29:49, arv wrote:
> Andy, did the lexical this CL land? If so I think super.prop in arrows should
> work now. I'll add tests if that is the case.

Lexical this hasn't landed yet, no, and a precursor just got rolled out so it
will probably be next week.  Good news though :)

[rossberg, 2015-04-24 11:49:51]

On 2015/04/23 16:29:49, arv wrote:
> Andreas, it is not clear how to fix the strong mode todo I added in
> ParseSuperExpression.

I think it's actually correct as is, since the location is propagated up in
ParseArrowFunctionLiteral.

[arv (Not doing code reviews), 2015-04-24 15:05:44]

On 2015/04/24 06:31:45, wingo wrote:
> On 2015/04/23 16:29:49, arv wrote:
> > Andy, did the lexical this CL land? If so I think super.prop in arrows
should
> > work now. I'll add tests if that is the case.
> 
> Lexical this hasn't landed yet, no, and a precursor just got rolled out so it
> will probably be next week.  Good news though :)

Without lexical this super in eval does not work. I can either wait for your CL
to land or I can submit this as is without a test.

I'm tempted to submit this as is since it does fixes the invalid function kind
in scopes.

[arv (Not doing code reviews), 2015-04-24 15:07:46]

add commented out js test

[arv (Not doing code reviews), 2015-04-24 15:07:52]

PTAL

[rossberg, 2015-04-24 15:10:55]

lgtm

[arv (Not doing code reviews), 2015-04-24 15:15:41]

https://codereview.chromium.org/1092503003/diff/40001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1092503003/diff/40001/src/preparser.h#newcode...
src/preparser.h:3284: // TODO(rossberg): This might not be the correct
FunctionState for the
Andreas, I'll remove this todo since you said you think this is correct as is.

One thing I noticed was that commenting out this line does not break any tests.
At this point we do not test `super()` in arrow functions so the problematic
case is never tested.

[rossberg, 2015-04-24 18:27:28]

On 2015/04/24 15:15:41, arv wrote:
> https://codereview.chromium.org/1092503003/diff/40001/src/preparser.h
> File src/preparser.h (right):
> 
>
https://codereview.chromium.org/1092503003/diff/40001/src/preparser.h#newcode...
> src/preparser.h:3284: // TODO(rossberg): This might not be the correct
> FunctionState for the
> Andreas, I'll remove this todo since you said you think this is correct as is.
> 
> One thing I noticed was that commenting out this line does not break any
tests.
> At this point we do not test `super()` in arrow functions so the problematic
> case is never tested.

Yeah, there are actually tests in strong/classes.js, but they are commented out
because super in arrows didn't work yet.

[wingo, 2015-05-06 14:47:38]

I don't see the point of the Scope::scope_uses_super_property_ flag.  For user
feedback you want a location, for syntax you need to know that you're in a
constructor, and for the runtime, the use of the lexical "this" in the various
SuperExpression AST nodes should be sufficient.  Am I missing something?

https://codereview.chromium.org/1092503003/diff/40001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1092503003/diff/40001/src/preparser.h#newcode...
src/preparser.h:3266: }
Sounds like we could use a this_declaration_scope() here.

[arv (Not doing code reviews), 2015-05-06 14:57:20]

https://codereview.chromium.org/1092503003/diff/40001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1092503003/diff/40001/src/preparser.h#newcode...
src/preparser.h:3266: }
On 2015/05/06 14:47:38, wingo wrote:
> Sounds like we could use a this_declaration_scope() here.

Yes. That would be perfect for both super and new.target.

[arv (Not doing code reviews), 2015-05-06 14:58:13]

On 2015/05/06 14:47:38, wingo wrote:
> I don't see the point of the Scope::scope_uses_super_property_ flag.  For user
> feedback you want a location, for syntax you need to know that you're in a
> constructor, and for the runtime, the use of the lexical "this" in the various
> SuperExpression AST nodes should be sufficient.  Am I missing something?

The tracking is needed for [[HomeObject]].

Also, see https://code.google.com/p/v8/issues/detail?id=4081

[wingo, 2015-05-06 15:13:17]

On 2015/05/06 14:58:13, arv wrote:
> On 2015/05/06 14:47:38, wingo wrote:
> > I don't see the point of the Scope::scope_uses_super_property_ flag.  For
user
> > feedback you want a location, for syntax you need to know that you're in a
> > constructor, and for the runtime, the use of the lexical "this" in the
various
> > SuperExpression AST nodes should be sufficient.  Am I missing something?
> 
> The tracking is needed for [[HomeObject]].
> 
> Also, see https://code.google.com/p/v8/issues/detail?id=4081

Ah, I am behind regarding these things.  Would it not be possible to declare a
variable for [[HomeObject]] then, and make the appropriate super expressions use
it?  Similar to what we do with "arguments".  Just an ignorant idea.

[arv (Not doing code reviews), 2015-05-06 15:18:43]

On 2015/05/06 15:13:17, wingo wrote:
> On 2015/05/06 14:58:13, arv wrote:
> > On 2015/05/06 14:47:38, wingo wrote:
> > > I don't see the point of the Scope::scope_uses_super_property_ flag.  For
> user
> > > feedback you want a location, for syntax you need to know that you're in a
> > > constructor, and for the runtime, the use of the lexical "this" in the
> various
> > > SuperExpression AST nodes should be sufficient.  Am I missing something?
> > 
> > The tracking is needed for [[HomeObject]].
> > 
> > Also, see https://code.google.com/p/v8/issues/detail?id=4081
> 
> Ah, I am behind regarding these things.  Would it not be possible to declare a
> variable for [[HomeObject]] then, and make the appropriate super expressions
use
> it?  Similar to what we do with "arguments".  Just an ignorant idea.

Yes. See the discussion on the bug.

The risk is that toMethod comes back.

[arv (Not doing code reviews), 2015-05-06 15:39:36]

Andy, it looks like your CL is sticking around... I'll update this later today.

[arv (Not doing code reviews), 2015-05-06 15:39:36]

Andy, it looks like your CL is sticking around... I'll update this later today.

[arv (Not doing code reviews), 2015-05-06 15:39:37]

Andy, it looks like your CL is sticking around... I'll update this later today.

[arv (Not doing code reviews), 2015-05-12 15:34:31]

On 2015/05/06 15:39:37, arv wrote:
> Andy, it looks like your CL is sticking around... I'll update this later
today.

I'm going to close this CL. I have a new patch that builds on this and it
correctly handles super.prop in arrows and eval.