Skip to content

Commit

Permalink
Revert of Revert of [strong] checking of this & super in constructors…
Browse files Browse the repository at this point in the history 
… (patchset #1 id:1 of https://codereview.chromium.org/1105453002/)

Reason for revert:
Was an infrastructure problem.

Original issue's description:
> Revert of [strong] checking of this & super in constructors (patchset #7 id:110001 of https://codereview.chromium.org/1024063002/)
>
> Reason for revert:
> [Sheriff] Breaks mac gc stress:
> http://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/1024
>
> Original issue's description:
> > [strong] checking of this & super in constructors
> >
> > R=dslomov@chromium.org, marja@chromium.org
> > BUG=v8:3956
> > LOG=N
> >
> > Enforces for constructors that
> > - the only use of 'super' is the super constructor call
> > - the only use of 'this' is a property assignment
> > - both of these must happen at the top-level of the body
> > - 'this' may only be assigned after the 'super' call
> > - 'return' may only be used after the last assignment to 'this'
> >
> > Not yet working for arrow functions (there might be deeper bugs with those).
> >
> > Committed: https://crrev.com/580d66bcda66220d2f3062ac58daf925436df74c
> > Cr-Commit-Position: refs/heads/master@{#27977}
>
> TBR=dslomov@chromium.org,marja@chromium.org,conradw@chromium.org,rossberg@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=v8:3956

TBR=dslomov@chromium.org,marja@chromium.org,conradw@chromium.org,rossberg@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:3956

Review URL: https://codereview.chromium.org/1073103004

Cr-Commit-Position: refs/heads/master@{#28001}
  • Loading branch information
@mi-ac
mi-ac authored and Commit bot committed Apr 22, 2015
1 parent 2631c9f commit 1ea118d
Show file tree
Hide file tree
Showing 7 changed files with 469 additions and 91 deletions.
6 changes: 4 additions & 2 deletions src/messages.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,11 @@ var kMessages = {
strong_unbound_global: ["In strong mode, using an undeclared global variable '", "%0", "' is not allowed"],
strong_super_call_missing: ["In strong mode, invoking the super constructor in a subclass is required"],
strong_super_call_duplicate: ["In strong mode, invoking the super constructor multiple times is deprecated"],
strong_super_call_nested: ["In strong mode, invoking the super constructor nested inside another statement or expression is deprecated"],
strong_super_call_misplaced: ["In strong mode, the super constructor must be invoked before any assignment to 'this'"],
strong_constructor_super: ["In strong mode, 'super' can only be used to invoke the super constructor, and cannot be nested inside another statement or expression"],
strong_constructor_this: ["In strong mode, 'this' can only be used to initialize properties, and cannot be nested inside another statement or expression"],
strong_constructor_return_value: ["In strong mode, returning a value from a constructor is deprecated"],
strong_constructor_return_misplaced: ["In strong mode, returning from a constructor before its super constructor invocation is deprecated"],
strong_constructor_return_misplaced: ["In strong mode, returning from a constructor before its super constructor invocation or all assignments to 'this' is deprecated"],
sloppy_lexical: ["Block-scoped declarations (let, const, function, class) not yet supported outside strict mode"],
malformed_arrow_function_parameter_list: ["Malformed arrow function parameter list"],
cant_prevent_ext_external_array_elements: ["Cannot prevent extension of an object with external array elements"],
Expand Down
91 changes: 62 additions & 29 deletions src/parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1210,25 +1210,28 @@ void* Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
directive_prologue = false;
}

Token::Value token = peek();
Scanner::Location token_loc = scanner()->peek_location();
Scanner::Location old_super_loc = function_state_->super_call_location();
Scanner::Location old_this_loc = function_state_->this_location();
Scanner::Location old_super_loc = function_state_->super_location();
Statement* stat = ParseStatementListItem(CHECK_OK);
Scanner::Location super_loc = function_state_->super_call_location();

if (is_strong(language_mode()) &&
i::IsConstructor(function_state_->kind()) &&
!old_super_loc.IsValid() && super_loc.IsValid() &&
token != Token::SUPER) {
// TODO(rossberg): This is more permissive than spec'ed, it allows e.g.
// super(), 1;
// super() + "";
// super() = 0;
// That should still be safe, though, thanks to left-to-right evaluation.
// The proper check would be difficult to implement in the preparser.
ReportMessageAt(super_loc, "strong_super_call_nested");
*ok = false;
return NULL;
scope_->is_function_scope() &&
i::IsConstructor(function_state_->kind())) {
Scanner::Location this_loc = function_state_->this_location();
Scanner::Location super_loc = function_state_->super_location();
if (this_loc.beg_pos != old_this_loc.beg_pos &&
this_loc.beg_pos != token_loc.beg_pos) {
ReportMessageAt(this_loc, "strong_constructor_this");
*ok = false;
return nullptr;
}
if (super_loc.beg_pos != old_super_loc.beg_pos &&
super_loc.beg_pos != token_loc.beg_pos) {
ReportMessageAt(super_loc, "strong_constructor_super");
*ok = false;
return nullptr;
}
}

if (stat == NULL || stat->IsEmpty()) {
Expand Down Expand Up @@ -2593,6 +2596,8 @@ Statement* Parser::ParseExpressionOrLabelledStatement(
// ExpressionStatement[Yield] :
// [lookahead ∉ {{, function, class, let [}] Expression[In, ?Yield] ;

int pos = peek_position();

switch (peek()) {
case Token::FUNCTION:
case Token::LBRACE:
Expand All @@ -2602,14 +2607,44 @@ Statement* Parser::ParseExpressionOrLabelledStatement(
*ok = false;
return nullptr;

case Token::THIS:
case Token::SUPER:
if (is_strong(language_mode()) &&
i::IsConstructor(function_state_->kind())) {
bool is_this = peek() == Token::THIS;
Expression* expr;
if (is_this) {
expr = ParseStrongInitializationExpression(CHECK_OK);
} else {
expr = ParseStrongSuperCallExpression(CHECK_OK);
}
switch (peek()) {
case Token::SEMICOLON:
Consume(Token::SEMICOLON);
break;
case Token::RBRACE:
case Token::EOS:
break;
default:
if (!scanner()->HasAnyLineTerminatorBeforeNext()) {
ReportMessageAt(function_state_->this_location(),
is_this ? "strong_constructor_this"
: "strong_constructor_super");
*ok = false;
return nullptr;
}
}
return factory()->NewExpressionStatement(expr, pos);
}
break;

// TODO(arv): Handle `let [`
// https://code.google.com/p/v8/issues/detail?id=3847

default:
break;
}

int pos = peek_position();
bool starts_with_idenfifier = peek_any_identifier();
Expression* expr = ParseExpression(true, CHECK_OK);
if (peek() == Token::COLON && starts_with_idenfifier && expr != NULL &&
Expand Down Expand Up @@ -4003,14 +4038,23 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
parenthesized_function_ = false; // The bit was set for this function only.

if (is_lazily_parsed) {
SkipLazyFunctionBody(function_name, &materialized_literal_count,
SkipLazyFunctionBody(&materialized_literal_count,
&expected_property_count, CHECK_OK);
} else {
body = ParseEagerFunctionBody(function_name, pos, fvar, fvar_init_op,
kind, CHECK_OK);
materialized_literal_count = function_state.materialized_literal_count();
expected_property_count = function_state.expected_property_count();
handler_count = function_state.handler_count();

if (is_strong(language_mode()) && IsSubclassConstructor(kind)) {
if (!function_state.super_location().IsValid()) {
ReportMessageAt(function_name_location,
"strong_super_call_missing", kReferenceError);
*ok = false;
return nullptr;
}
}
}

// Validate name and parameter names. We can do this only after parsing the
Expand All @@ -4025,18 +4069,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
if (is_strict(language_mode())) {
CheckStrictOctalLiteral(scope->start_position(), scope->end_position(),
CHECK_OK);
}
if (is_strict(language_mode())) {
CheckConflictingVarDeclarations(scope, CHECK_OK);
}
if (is_strong(language_mode()) && IsSubclassConstructor(kind)) {
if (!function_state.super_call_location().IsValid()) {
ReportMessageAt(function_name_location, "strong_super_call_missing",
kReferenceError);
*ok = false;
return nullptr;
}
}
}

FunctionLiteral::ParameterFlag duplicate_parameters =
Expand All @@ -4060,8 +4094,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
}


void Parser::SkipLazyFunctionBody(const AstRawString* function_name,
int* materialized_literal_count,
void Parser::SkipLazyFunctionBody(int* materialized_literal_count,
int* expected_property_count,
bool* ok) {
if (produce_cached_parse_data()) CHECK(log_);
Expand Down
11 changes: 4 additions & 7 deletions src/parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,8 +773,7 @@ class ParserTraits {
bool name_is_strict_reserved, FunctionKind kind,
int function_token_position, FunctionLiteral::FunctionType type,
FunctionLiteral::ArityRestriction arity_restriction, bool* ok);
V8_INLINE void SkipLazyFunctionBody(const AstRawString* name,
int* materialized_literal_count,
V8_INLINE void SkipLazyFunctionBody(int* materialized_literal_count,
int* expected_property_count, bool* ok);
V8_INLINE ZoneList<Statement*>* ParseEagerFunctionBody(
const AstRawString* name, int pos, Variable* fvar,
Expand Down Expand Up @@ -1025,8 +1024,7 @@ class Parser : public ParserBase<ParserTraits> {

// Skip over a lazy function, either using cached data if we have it, or
// by parsing the function with PreParser. Consumes the ending }.
void SkipLazyFunctionBody(const AstRawString* function_name,
int* materialized_literal_count,
void SkipLazyFunctionBody(int* materialized_literal_count,
int* expected_property_count,
bool* ok);

Expand Down Expand Up @@ -1091,12 +1089,11 @@ const AstRawString* ParserTraits::EmptyIdentifierString() {
}


void ParserTraits::SkipLazyFunctionBody(const AstRawString* function_name,
int* materialized_literal_count,
void ParserTraits::SkipLazyFunctionBody(int* materialized_literal_count,
int* expected_property_count,
bool* ok) {
return parser_->SkipLazyFunctionBody(
function_name, materialized_literal_count, expected_property_count, ok);
materialized_literal_count, expected_property_count, ok);
}


Expand Down
72 changes: 62 additions & 10 deletions src/preparser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,15 @@ PreParser::PreParseResult PreParser::PreParseLazyFunction(
if (is_strict(scope_->language_mode())) {
int end_pos = scanner()->location().end_pos;
CheckStrictOctalLiteral(start_position, end_pos, &ok);
if (!ok) return kPreParseSuccess;

if (is_strong(scope_->language_mode()) && IsSubclassConstructor(kind)) {
if (!function_state.super_location().IsValid()) {
ReportMessageAt(Scanner::Location(start_position, start_position + 1),
"strong_super_call_missing", kReferenceError);
return kPreParseSuccess;
}
}
}
}
return kPreParseSuccess;
Expand Down Expand Up @@ -196,19 +205,31 @@ void PreParser::ParseStatementList(int end_token, bool* ok) {
if (directive_prologue && peek() != Token::STRING) {
directive_prologue = false;
}
Token::Value token = peek();
Scanner::Location old_super_loc = function_state_->super_call_location();
Scanner::Location token_loc = scanner()->peek_location();
Scanner::Location old_this_loc = function_state_->this_location();
Scanner::Location old_super_loc = function_state_->super_location();
Statement statement = ParseStatementListItem(ok);
if (!*ok) return;
Scanner::Location super_loc = function_state_->super_call_location();

if (is_strong(language_mode()) &&
i::IsConstructor(function_state_->kind()) &&
!old_super_loc.IsValid() && super_loc.IsValid() &&
token != Token::SUPER) {
ReportMessageAt(super_loc, "strong_super_call_nested");
*ok = false;
return;
scope_->is_function_scope() &&
i::IsConstructor(function_state_->kind())) {
Scanner::Location this_loc = function_state_->this_location();
Scanner::Location super_loc = function_state_->super_location();
if (this_loc.beg_pos != old_this_loc.beg_pos &&
this_loc.beg_pos != token_loc.beg_pos) {
ReportMessageAt(this_loc, "strong_constructor_this");
*ok = false;
return;
}
if (super_loc.beg_pos != old_super_loc.beg_pos &&
super_loc.beg_pos != token_loc.beg_pos) {
ReportMessageAt(super_loc, "strong_constructor_super");
*ok = false;
return;
}
}

if (directive_prologue) {
if (statement.IsUseStrictLiteral()) {
scope_->SetLanguageMode(
Expand Down Expand Up @@ -532,6 +553,37 @@ PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(bool* ok) {
*ok = false;
return Statement::Default();

case Token::THIS:
case Token::SUPER:
if (is_strong(language_mode()) &&
i::IsConstructor(function_state_->kind())) {
bool is_this = peek() == Token::THIS;
Expression expr = Expression::Default();
if (is_this) {
expr = ParseStrongInitializationExpression(CHECK_OK);
} else {
expr = ParseStrongSuperCallExpression(CHECK_OK);
}
switch (peek()) {
case Token::SEMICOLON:
Consume(Token::SEMICOLON);
break;
case Token::RBRACE:
case Token::EOS:
break;
default:
if (!scanner()->HasAnyLineTerminatorBeforeNext()) {
ReportMessageAt(function_state_->this_location(),
is_this ? "strong_constructor_this"
: "strong_constructor_super");
*ok = false;
return Statement::Default();
}
}
return Statement::ExpressionStatement(expr);
}
break;

// TODO(arv): Handle `let [`
// https://code.google.com/p/v8/issues/detail?id=3847

Expand Down Expand Up @@ -972,7 +1024,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
}

if (is_strong(language_mode()) && IsSubclassConstructor(kind)) {
if (!function_state.super_call_location().IsValid()) {
if (!function_state.super_location().IsValid()) {
ReportMessageAt(function_name_location, "strong_super_call_missing",
kReferenceError);
*ok = false;
Expand Down
Loading

0 comments on commit 1ea118d

Please sign in to comment.