Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(297)

Unified Diff: src/parsing/parser.cc

Issue 2109733003: Add errors for declarations which conflict with catch parameters. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/ast/scopes.h ('k') | test/mjsunit/es6/block-conflicts.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index b14cdaf15e16e325ea4b3f83d7f12ee09a480353..94461ee0bc7e365e8d4f29f3122b869b967b5e82 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -3063,6 +3063,23 @@ TryStatement* Parser::ParseTryStatement(bool* ok) {
Consume(Token::RBRACE);
}
block_scope->set_end_position(scanner()->location().end_pos);
+ if (is_simple) {
+ // Check for `catch(e) { let e; }` and similar errors.
+ // The BindingPattern case is handled elsewhere, because
+ // the names created by destructuring being lexical.
+ Variable* var = block_scope->LookupLocal(name);
+ if (var != nullptr && IsLexicalVariableMode(var->mode())) {
+ auto position = pattern->position();
+ Scanner::Location location =
+ position == RelocInfo::kNoPosition
+ ? Scanner::Location::invalid()
+ : Scanner::Location(position, position + 1);
+ ParserTraits::ReportMessageAt(
+ location, MessageTemplate::kVarRedeclaration, name);
+ *ok = false;
+ return NULL;
+ }
+ }
block_scope = block_scope->FinalizeBlockScope();
catch_block->set_scope(block_scope);
}
@@ -3568,7 +3585,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
bool* ok) {
int stmt_pos = peek_position();
Statement* init = NULL;
- ZoneList<const AstRawString*> lexical_bindings(1, zone());
+ ZoneList<const AstRawString*> bound_names(1, zone());
+ bool bound_names_are_lexical = false;
// Create an in-between scope for let-bound iteration variables.
Scope* for_scope = NewScope(scope_, BLOCK_SCOPE);
@@ -3619,10 +3637,12 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
}
Block* init_block = nullptr;
+ bound_names_are_lexical =
+ IsLexicalVariableMode(parsing_result.descriptor.mode);
// special case for legacy for (var/const x =.... in)
Dan Ehrenberg 2016/06/29 20:24:32 It's only var at this point, right? Could you chan
bakkot 2016/06/29 21:10:13 Done, and in a few other places.
- if (!IsLexicalVariableMode(parsing_result.descriptor.mode) &&
- decl.pattern->IsVariableProxy() && decl.initializer != nullptr) {
+ if (!bound_names_are_lexical && decl.pattern->IsVariableProxy() &&
+ decl.initializer != nullptr) {
DCHECK(!allow_harmony_for_in());
++use_counts_[v8::Isolate::kForInInitializer];
const AstRawString* name =
@@ -3698,9 +3718,42 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
PatternRewriter::DeclareAndInitializeVariables(
each_initialization_block, &descriptor, &decl,
- IsLexicalVariableMode(descriptor.mode) ? &lexical_bindings
- : nullptr,
+ bound_names_are_lexical ||
+ (mode == ForEachStatement::ITERATE &&
+ parsing_result.descriptor.mode == VariableMode::VAR)
Dan Ehrenberg 2016/06/29 20:24:32 Minor cleanup suggestion: since it occurs multiple
bakkot 2016/06/29 21:10:13 Done.
+ ? &bound_names
+ : nullptr,
CHECK_OK);
+
+ // Annex B.3.5 prohibits the form
+ // `try {} catch(e) { for (var e of {}); }`
+ // So if we are parsing a statement like `for (var ... of ...)`
+ // we need to walk up the scope chain and look for catch scopes
+ // which have a simple binding, then compare their binding against
+ // all of the names declared in the init of the for-of we're
+ // parsing.
+ if (mode == ForEachStatement::ITERATE &&
+ parsing_result.descriptor.mode == VariableMode::VAR) {
+ Scope* catch_scope = scope_;
+ while (catch_scope != nullptr &&
+ !catch_scope->is_declaration_scope()) {
+ if (catch_scope->is_catch_scope()) {
+ auto name = catch_scope->catch_variable_name();
+ if (name !=
+ ast_value_factory()
+ ->dot_catch_string()) { // i.e. is a simple binding
+ if (bound_names.Contains(name)) {
+ ParserTraits::ReportMessageAt(
+ parsing_result.bindings_loc,
+ MessageTemplate::kVarRedeclaration, name);
+ *ok = false;
+ return nullptr;
+ }
+ }
+ }
+ catch_scope = catch_scope->outer_scope();
+ }
+ }
}
body_block->statements()->Add(each_initialization_block, zone());
@@ -3715,18 +3768,17 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
body_block->set_scope(body_scope);
// Create a TDZ for any lexically-bound names.
- if (IsLexicalVariableMode(parsing_result.descriptor.mode)) {
+ if (bound_names_are_lexical) {
DCHECK_NULL(init_block);
init_block =
factory()->NewBlock(nullptr, 1, false, RelocInfo::kNoPosition);
- for (int i = 0; i < lexical_bindings.length(); ++i) {
+ for (int i = 0; i < bound_names.length(); ++i) {
// TODO(adamk): This needs to be some sort of special
// INTERNAL variable that's invisible to the debugger
// but visible to everything else.
- VariableProxy* tdz_proxy =
- NewUnresolved(lexical_bindings[i], LET);
+ VariableProxy* tdz_proxy = NewUnresolved(bound_names[i], LET);
Declaration* tdz_decl = factory()->NewVariableDeclaration(
tdz_proxy, LET, scope_, RelocInfo::kNoPosition);
Variable* tdz_var = Declare(
@@ -3752,11 +3804,10 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
return final_loop;
}
} else {
+ bound_names_are_lexical =
+ IsLexicalVariableMode(parsing_result.descriptor.mode);
init = parsing_result.BuildInitializationBlock(
- IsLexicalVariableMode(parsing_result.descriptor.mode)
- ? &lexical_bindings
- : nullptr,
- CHECK_OK);
+ bound_names_are_lexical ? &bound_names : nullptr, CHECK_OK);
}
} else {
int lhs_beg_pos = peek_position();
@@ -3836,7 +3887,7 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
// If there are let bindings, then condition and the next statement of the
// for loop must be parsed in a new scope.
Scope* inner_scope = scope_;
- if (lexical_bindings.length() > 0) {
+ if (bound_names_are_lexical && bound_names.length() > 0) {
inner_scope = NewScope(for_scope, BLOCK_SCOPE);
inner_scope->set_start_position(scanner()->location().beg_pos);
}
@@ -3858,11 +3909,11 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
}
Statement* result = NULL;
- if (lexical_bindings.length() > 0) {
+ if (bound_names_are_lexical && bound_names.length() > 0) {
BlockState block_state(&scope_, for_scope);
result = DesugarLexicalBindingsInForStatement(
- inner_scope, parsing_result.descriptor.mode, &lexical_bindings, loop,
- init, cond, next, body, CHECK_OK);
+ inner_scope, parsing_result.descriptor.mode, &bound_names, loop, init,
+ cond, next, body, CHECK_OK);
for_scope->set_end_position(scanner()->location().end_pos);
} else {
for_scope->set_end_position(scanner()->location().end_pos);
« no previous file with comments | « src/ast/scopes.h ('k') | test/mjsunit/es6/block-conflicts.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698