Reviewers: wingo, rossberg,
Message:
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.
Description:
[es6] Make ParseSuperExpression uses 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
[email protected], [email protected]
Please review this at https://codereview.chromium.org/1092503003/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+34, -36 lines):
M src/ast.cc
M src/parser.cc
M src/preparser.h
M src/preparser.cc
M src/scopes.cc
M test/cctest/test-parsing.cc
Index: src/ast.cc
diff --git a/src/ast.cc b/src/ast.cc
index
5038716003e80125c1ff1fd1b3709443a0a3ea63..1d564f3849794f64378d828d5fe91ee064d21cdc
100644
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -179,7 +179,7 @@ LanguageMode FunctionLiteral::language_mode() const {
bool FunctionLiteral::uses_super_property() const {
DCHECK_NOT_NULL(scope());
- return scope()->uses_super_property() ||
scope()->inner_uses_super_property();
+ return scope()->uses_super_property();
}
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
9507b69df6f0d661c4c5f5933882dc897233b2ad..4301f4648326fc5009698bf707d9a7827841e31d
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -1128,7 +1128,8 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate,
ParseInfo* info,
bool ok = true;
if (shared_info->is_arrow()) {
- Scope* scope = NewScope(scope_, ARROW_SCOPE);
+ Scope* scope =
+ NewScope(scope_, ARROW_SCOPE, FunctionKind::kArrowFunction);
scope->set_start_position(shared_info->start_position());
FormalParameterErrorLocations error_locs;
bool has_rest = false;
Index: src/preparser.cc
diff --git a/src/preparser.cc b/src/preparser.cc
index
950f7b50ec35ca6a255788dac4b1aecccdef9086..024c916488951b5f85d573ab928e98742ed0d4f9
100644
--- a/src/preparser.cc
+++ b/src/preparser.cc
@@ -107,7 +107,7 @@ PreParser::PreParseResult
PreParser::PreParseLazyFunction(
FunctionState top_state(&function_state_, &scope_, top_scope,
kNormalFunction,
&top_factory);
scope_->SetLanguageMode(language_mode);
- Scope* function_scope = NewScope(scope_, FUNCTION_SCOPE);
+ Scope* function_scope = NewScope(scope_, FUNCTION_SCOPE, kind);
PreParserFactory function_factory(NULL);
FunctionState function_state(&function_state_, &scope_, function_scope,
kind,
&function_factory);
@@ -982,7 +982,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
// Parse function body.
bool outer_is_script_scope = scope_->is_script_scope();
- Scope* function_scope = NewScope(scope_, FUNCTION_SCOPE);
+ Scope* function_scope = NewScope(scope_, FUNCTION_SCOPE, kind);
PreParserFactory factory(NULL);
FunctionState function_state(&function_state_, &scope_, function_scope,
kind,
&factory);
Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index
36320513adb872f5ef4f51d450551af9ab1499d8..057a730bfec81d024440cb2866f92cc0a6296d17
100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -345,12 +345,17 @@ class ParserBase : public Traits {
Mode old_mode_;
};
- Scope* NewScope(Scope* parent, ScopeType scope_type,
- FunctionKind kind = kNormalFunction) {
+ Scope* NewScope(Scope* parent, ScopeType scope_type) {
+ DCHECK(scope_type != FUNCTION_SCOPE && scope_type != ARROW_SCOPE);
+ return NewScope(parent, scope_type, kNormalFunction);
+ }
+
+ Scope* NewScope(Scope* parent, ScopeType scope_type, FunctionKind kind) {
DCHECK(ast_value_factory());
DCHECK(scope_type != MODULE_SCOPE || allow_harmony_modules());
- DCHECK((scope_type == FUNCTION_SCOPE && IsValidFunctionKind(kind)) ||
- kind == kNormalFunction);
+ // DCHECK((scope_type == FUNCTION_SCOPE && IsValidFunctionKind(kind)) |
|
+ // kind == kNormalFunction);
+ DCHECK(scope_type == ARROW_SCOPE ? IsArrowFunction(kind) : true);
Scope* result = new (zone())
Scope(zone(), parent, scope_type, ast_value_factory(), kind);
result->Initialize();
@@ -2173,7 +2178,8 @@
ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
Consume(Token::LPAREN);
if (allow_harmony_arrow_functions() && Check(Token::RPAREN)) {
// As a primary expression, the only thing that can follow "()"
is "=>".
- Scope* scope = this->NewScope(scope_, ARROW_SCOPE);
+ Scope* scope =
+ this->NewScope(scope_, ARROW_SCOPE,
FunctionKind::kArrowFunction);
scope->set_start_position(beg_pos);
FormalParameterErrorLocations error_locs;
bool has_rest = false;
@@ -2654,7 +2660,8 @@ ParserBase<Traits>::ParseAssignmentExpression(bool
accept_IN,
FormalParameterErrorLocations error_locs;
Scanner::Location loc(lhs_location.beg_pos,
scanner()->location().end_pos);
bool has_rest = false;
- Scope* scope = this->NewScope(scope_, ARROW_SCOPE);
+ Scope* scope =
+ this->NewScope(scope_, ARROW_SCOPE, FunctionKind::kArrowFunction);
scope->set_start_position(lhs_location.beg_pos);
this->ParseArrowFunctionFormalParameters(scope, expression, loc,
&error_locs, &has_rest,
CHECK_OK);
@@ -3251,19 +3258,20 @@ ParserBase<Traits>::ParseSuperExpression(bool
is_new,
bool* ok) {
Expect(Token::SUPER, CHECK_OK);
- // TODO(wingo): Does this actually work with lazily compiled arrows?
- FunctionState* function_state = function_state_;
- while (IsArrowFunction(function_state->kind())) {
- function_state = function_state->outer();
+ Scope* scope = scope_->DeclarationScope();
+
+ while (scope->is_eval_scope() || scope->is_arrow_scope()) {
+ scope = scope->outer_scope();
+ DCHECK_NOT_NULL(scope);
+ scope = scope->DeclarationScope();
}
- // TODO(arv): Handle eval scopes similarly.
- FunctionKind kind = function_state->kind();
+ FunctionKind kind = scope->function_kind();
if (IsConciseMethod(kind) || IsAccessorFunction(kind) ||
i::IsConstructor(kind)) {
if (peek() == Token::PERIOD || peek() == Token::LBRACK) {
- scope_->RecordSuperPropertyUsage();
- return this->SuperReference(scope_, factory());
+ scope->RecordSuperPropertyUsage();
+ return this->SuperReference(scope, factory());
}
// new super() is never allowed.
// super() is only allowed in derived constructor
@@ -3274,8 +3282,9 @@ ParserBase<Traits>::ParseSuperExpression(bool is_new,
*ok = false;
return this->EmptyExpression();
}
- function_state->set_super_location(scanner()->location());
- return this->SuperReference(scope_, factory());
+ // TODO(rossberg): We might not have a FunctionState for the method
here.
+ // function_state->set_super_location(scanner()->location());
+ return this->SuperReference(scope, factory());
}
}
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index
d34d2e934c89d383995291a83e25e1f92f620430..cdf4b87aebc4900ce2d576c7277cd61246bec7bc
100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -174,7 +174,6 @@ void Scope::SetDefaults(ScopeType scope_type, Scope*
outer_scope,
inner_scope_calls_eval_ = false;
inner_scope_uses_arguments_ = false;
inner_scope_uses_this_ = false;
- inner_scope_uses_super_property_ = false;
force_eager_compilation_ = false;
force_context_allocation_ = (outer_scope != NULL && !is_function_scope())
? outer_scope->has_forced_context_allocation() : false;
@@ -934,8 +933,6 @@ void Scope::Print(int n) {
if (inner_scope_uses_arguments_) {
Indent(n1, "// inner scope uses 'arguments'\n");
}
- if (inner_scope_uses_super_property_)
- Indent(n1, "// inner scope uses 'super' property\n");
if (inner_scope_uses_this_) Indent(n1, "// inner scope uses 'this'\n");
if (outer_scope_calls_sloppy_eval_) {
Indent(n1, "// outer scope calls 'eval' in sloppy context\n");
@@ -1296,10 +1293,6 @@ void Scope::PropagateScopeInfo(bool
outer_scope_calls_sloppy_eval ) {
if (inner->scope_uses_arguments_ ||
inner->inner_scope_uses_arguments_) {
inner_scope_uses_arguments_ = true;
}
- if (inner->scope_uses_super_property_ ||
- inner->inner_scope_uses_super_property_) {
- inner_scope_uses_super_property_ = true;
- }
if (inner->scope_uses_this_ || inner->inner_scope_uses_this_) {
inner_scope_uses_this_ = true;
}
Index: test/cctest/test-parsing.cc
diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index
ba842587708dfaaa8053883460b2e2e41f8bdf7f..6b8af3fea2e3a8779f8facd9e296de574d1ada15
100644
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -961,8 +961,7 @@ TEST(ScopeUsesArgumentsSuperThis) {
SUPER_PROPERTY = 1 << 1,
THIS = 1 << 2,
INNER_ARGUMENTS = 1 << 3,
- INNER_SUPER_PROPERTY = 1 << 4,
- INNER_THIS = 1 << 5
+ INNER_THIS = 1 << 4
};
static const struct {
@@ -978,7 +977,7 @@ TEST(ScopeUsesArgumentsSuperThis) {
{"return this + arguments[0] + super.x",
ARGUMENTS | SUPER_PROPERTY | THIS},
{"return x => this + x", INNER_THIS},
- {"return x => super.f() + x", INNER_SUPER_PROPERTY},
+ {"return x => super.f() + x", SUPER_PROPERTY},
{"this.foo = 42;", THIS},
{"this.foo();", THIS},
{"if (foo()) { this.f() }", THIS},
@@ -1006,10 +1005,9 @@ TEST(ScopeUsesArgumentsSuperThis) {
{"\"use strict\"; while (true) { let x; this, arguments; }",
INNER_ARGUMENTS | INNER_THIS},
{"\"use strict\"; while (true) { let x; this, super.f(), arguments; }",
- INNER_ARGUMENTS | INNER_SUPER_PROPERTY | INNER_THIS},
+ INNER_ARGUMENTS | SUPER_PROPERTY | INNER_THIS},
{"\"use strict\"; if (foo()) { let x; this.f() }", INNER_THIS},
- {"\"use strict\"; if (foo()) { let x; super.f() }",
- INNER_SUPER_PROPERTY},
+ {"\"use strict\"; if (foo()) { let x; super.f() }", SUPER_PROPERTY},
{"\"use strict\"; if (1) {"
" let x; return { m() { return this + super.m() + arguments } }"
"}",
@@ -1030,7 +1028,6 @@ TEST(ScopeUsesArgumentsSuperThis) {
for (unsigned i = 0; i < arraysize(source_data); ++i) {
// Super property is only allowed in constructor and method.
if (((source_data[i].expected & SUPER_PROPERTY) ||
- (source_data[i].expected & INNER_SUPER_PROPERTY) ||
(source_data[i].expected == NONE)) && j != 2) {
continue;
}
@@ -1074,8 +1071,6 @@ TEST(ScopeUsesArgumentsSuperThis) {
CHECK_EQ((source_data[i].expected & THIS) != 0, scope->uses_this());
CHECK_EQ((source_data[i].expected & INNER_ARGUMENTS) != 0,
scope->inner_uses_arguments());
- CHECK_EQ((source_data[i].expected & INNER_SUPER_PROPERTY) != 0,
- scope->inner_uses_super_property());
CHECK_EQ((source_data[i].expected & INNER_THIS) != 0,
scope->inner_uses_this());
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
