diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc index f7ecf4984b..c1dcf49532 100644 --- a/src/ast/scopes.cc +++ b/src/ast/scopes.cc @@ -1204,51 +1204,37 @@ Variable* Scope::NewTemporary(const AstRawString* name, Declaration* Scope::CheckConflictingVarDeclarations() { for (Declaration* decl : decls_) { - VariableMode mode = decl->proxy()->var()->mode(); - // Lexical vs lexical conflicts within the same scope have already been // captured in Parser::Declare. The only conflicts we still need to check - // are lexical vs nested var, or any declarations within a declaration - // block scope vs lexical declarations in its surrounding (function) scope. - Scope* current = this; + // are lexical vs nested var. if (decl->IsVariableDeclaration() && decl->AsVariableDeclaration()->AsNested() != nullptr) { - DCHECK_EQ(mode, VariableMode::kVar); - current = decl->AsVariableDeclaration()->AsNested()->scope(); - } else if (IsLexicalVariableMode(mode)) { - if (!is_block_scope()) continue; - DCHECK(is_declaration_scope()); - DCHECK_EQ(outer_scope()->scope_type(), FUNCTION_SCOPE); - current = outer_scope(); - } - - // Iterate through all scopes until and including the declaration scope. - while (true) { - // There is a conflict if there exists a non-VAR binding. - Variable* other_var = - current->variables_.Lookup(decl->proxy()->raw_name()); - if (other_var != nullptr && IsLexicalVariableMode(other_var->mode())) { - return decl; + DCHECK_EQ(decl->proxy()->var()->mode(), VariableMode::kVar); + Scope* current = decl->AsVariableDeclaration()->AsNested()->scope(); + // Iterate through all scopes until and including the declaration scope. + while (true) { + // There is a conflict if there exists a non-VAR binding. + Variable* other_var = + current->variables_.Lookup(decl->proxy()->raw_name()); + if (other_var != nullptr && IsLexicalVariableMode(other_var->mode())) { + return decl; + } + if (current->is_declaration_scope()) break; + current = current->outer_scope(); } - if (current->is_declaration_scope()) break; - current = current->outer_scope(); } } return nullptr; } -const AstRawString* Scope::FindLexVariableDeclaredIn(Scope* scope) { - DCHECK(is_block_scope()); +const AstRawString* Scope::FindVariableDeclaredIn(Scope* scope, + VariableMode mode_limit) { const VariableMap& variables = scope->variables_; for (ZoneHashMap::Entry* p = variables.Start(); p != nullptr; p = variables.Next(p)) { const AstRawString* name = static_cast(p->key); Variable* var = LookupLocal(name); - if (var != nullptr) { - // Conflict; find and return its declaration. - DCHECK(IsLexicalVariableMode(var->mode())); - return name; - } + if (var != nullptr && var->mode() <= mode_limit) return name; } return nullptr; } diff --git a/src/ast/scopes.h b/src/ast/scopes.h index 082c934469..602da5872f 100644 --- a/src/ast/scopes.h +++ b/src/ast/scopes.h @@ -312,11 +312,13 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) { // scope over a let binding of the same name. Declaration* CheckConflictingVarDeclarations(); - // Find lexical variable that has a name that was declared in |scope|. This is - // used to catch patterns like `try{}catch(e){let e;}`, which is an error even - // though the two 'e's are declared in different scopes. Returns the first - // duplicate variable name if there is one, nullptr otherwise. - const AstRawString* FindLexVariableDeclaredIn(Scope* scope); + // Find variable with (variable->mode() <= |mode_limit|) that was declared in + // |scope|. This is used to catch patterns like `try{}catch(e){let e;}` and + // function([e]) { let e }, which are errors even though the two 'e's are each + // time declared in different scopes. Returns the first duplicate variable + // name if there is one, nullptr otherwise. + const AstRawString* FindVariableDeclaredIn(Scope* scope, + VariableMode mode_limit); // Find the declaration that introduced |name|. Declaration* DeclarationFor(const AstRawString* name); diff --git a/src/globals.h b/src/globals.h index 700fc39252..f0c45424f6 100644 --- a/src/globals.h +++ b/src/globals.h @@ -1056,10 +1056,12 @@ enum class VariableMode : uint8_t { // variable is global unless it has been shadowed // by an eval-introduced variable - kDynamicLocal // requires dynamic lookup, but we know that the - // variable is local and where it is unless it - // has been shadowed by an eval-introduced - // variable + kDynamicLocal, // requires dynamic lookup, but we know that the + // variable is local and where it is unless it + // has been shadowed by an eval-introduced + // variable + + kLastLexicalVariableMode = kConst, }; // Printing support @@ -1104,7 +1106,7 @@ inline bool IsDeclaredVariableMode(VariableMode mode) { inline bool IsLexicalVariableMode(VariableMode mode) { STATIC_ASSERT(static_cast(VariableMode::kLet) == 0); // Implies that mode >= VariableMode::kLet. - return mode <= VariableMode::kConst; + return mode <= VariableMode::kLastLexicalVariableMode; } enum VariableLocation : uint8_t { diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h index 55578e6c15..8a1f03fd07 100644 --- a/src/parsing/parser-base.h +++ b/src/parsing/parser-base.h @@ -3905,6 +3905,11 @@ void ParserBase::ParseFunctionBody( inner_scope->set_end_position(end_position()); if (inner_scope->FinalizeBlockScope() != nullptr) { + const AstRawString* conflict = inner_scope->FindVariableDeclaredIn( + function_scope, VariableMode::kLastLexicalVariableMode); + if (conflict != nullptr) { + impl()->ReportVarRedeclarationIn(conflict, inner_scope); + } impl()->CheckConflictingVarDeclarations(inner_scope); impl()->InsertShadowingVarBindingInitializers(inner_block); } else { @@ -4042,6 +4047,13 @@ ParserBase::ParseArrowFunctionLiteral( // body is preparsed; move relevant parts of parameter handling to // simulate consistent parameter handling. + // Building the parameter initialization block declares the parameters. + // TODO(verwaest): Rely on ArrowHeadParsingScope instead. + if (!formal_parameters.is_simple) { + impl()->BuildParameterInitializationBlock(formal_parameters); + if (has_error()) return impl()->FailureExpression(); + } + // For arrow functions, we don't need to retrieve data about function // parameters. int dummy_num_parameters = -1; @@ -5290,11 +5302,11 @@ typename ParserBase::StatementT ParserBase::ParseTryStatement() { const AstRawString* name = catch_info.variable->raw_name(); if (inner_scope->LookupLocal(name)) conflict = name; } else { - conflict = inner_scope->FindLexVariableDeclaredIn(scope()); + conflict = inner_scope->FindVariableDeclaredIn( + scope(), VariableMode::kVar); } if (conflict != nullptr) { - impl()->ReportConflictingDeclarationInCatch(conflict, - inner_scope); + impl()->ReportVarRedeclarationIn(conflict, inner_scope); } } diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc index 89b1b09450..eadb961ac3 100644 --- a/src/parsing/parser.cc +++ b/src/parsing/parser.cc @@ -1589,8 +1589,7 @@ Block* Parser::RewriteCatchPattern(CatchInfo* catch_info) { return init_block; } -void Parser::ReportConflictingDeclarationInCatch(const AstRawString* name, - Scope* scope) { +void Parser::ReportVarRedeclarationIn(const AstRawString* name, Scope* scope) { for (Declaration* decl : *scope->declarations()) { if (decl->proxy()->raw_name() == name) { int position = decl->proxy()->position(); @@ -2869,9 +2868,6 @@ Block* Parser::BuildParameterInitializationBlock( if (param_block != init_block) { param_scope = param_scope->FinalizeBlockScope(); - if (param_scope != nullptr) { - CheckConflictingVarDeclarations(param_scope); - } init_block->statements()->Add(param_block, zone()); } ++index; diff --git a/src/parsing/parser.h b/src/parsing/parser.h index b4b59587d3..10643ed236 100644 --- a/src/parsing/parser.h +++ b/src/parsing/parser.h @@ -318,8 +318,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase) { Statement* RewriteSwitchStatement(SwitchStatement* switch_statement, Scope* scope); Block* RewriteCatchPattern(CatchInfo* catch_info); - void ReportConflictingDeclarationInCatch(const AstRawString* name, - Scope* scope); + void ReportVarRedeclarationIn(const AstRawString* name, Scope* scope); Statement* RewriteTryStatement(Block* try_block, Block* catch_block, const SourceRange& catch_range, Block* finally_block, diff --git a/src/parsing/preparser.cc b/src/parsing/preparser.cc index 66e5a910f2..47c954d4fe 100644 --- a/src/parsing/preparser.cc +++ b/src/parsing/preparser.cc @@ -116,13 +116,7 @@ PreParser::PreParseResult PreParser::PreParseFunction( function_scope->set_is_being_lazily_parsed(true); #endif - // Start collecting data for a new function which might contain skippable - // functions. - PreParsedScopeDataBuilder::DataGatheringScope - preparsed_scope_data_builder_scope(this); - if (!IsArrowFunction(kind)) { - preparsed_scope_data_builder_scope.Start(function_scope); - } + PreParserFormalParameters formals(function_scope); // In the preparser, we use the function literal ids to count how many // FunctionLiterals were encountered. The PreParser doesn't actually persist @@ -136,11 +130,18 @@ PreParser::PreParseResult PreParser::PreParseFunction( DCHECK_NULL(scope_); FunctionState function_state(&function_state_, &scope_, function_scope); - PreParserFormalParameters formals(function_scope); + // Start collecting data for a new function which might contain skippable + // functions. + PreParsedScopeDataBuilder::DataGatheringScope + preparsed_scope_data_builder_scope(this); - // Parse non-arrow function parameters. For arrow functions, the parameters - // have already been parsed. - if (!IsArrowFunction(kind)) { + if (IsArrowFunction(kind)) { + formals.is_simple = function_scope->has_simple_parameters(); + } else { + preparsed_scope_data_builder_scope.Start(function_scope); + + // Parse non-arrow function parameters. For arrow functions, the parameters + // have already been parsed. DeclarationParsingScope formals_scope( this, ExpressionScope::kParameterDeclaration); // We return kPreParseSuccess in failure cases too - errors are retrieved @@ -187,7 +188,11 @@ PreParser::PreParseResult PreParser::PreParseFunction( SetLanguageMode(function_scope, inner_scope->language_mode()); inner_scope->set_end_position(scanner()->peek_location().end_pos); - inner_scope->FinalizeBlockScope(); + if (inner_scope->FinalizeBlockScope() != nullptr) { + const AstRawString* conflict = inner_scope->FindVariableDeclaredIn( + function_scope, VariableMode::kLastLexicalVariableMode); + if (conflict != nullptr) ReportVarRedeclarationIn(conflict, inner_scope); + } } use_counts_ = nullptr; diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h index 25a403b791..ce84177602 100644 --- a/src/parsing/preparser.h +++ b/src/parsing/preparser.h @@ -1163,8 +1163,8 @@ class PreParser : public ParserBase { return PreParserBlock::Default(); } - V8_INLINE void ReportConflictingDeclarationInCatch(const AstRawString* name, - Scope* scope) { + V8_INLINE void ReportVarRedeclarationIn(const AstRawString* name, + Scope* scope) { ReportUnidentifiableError(); } diff --git a/test/message/fail/param-arrow-redeclaration-as-let.js b/test/message/fail/param-arrow-redeclaration-as-let.js new file mode 100644 index 0000000000..7713849014 --- /dev/null +++ b/test/message/fail/param-arrow-redeclaration-as-let.js @@ -0,0 +1,7 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +({x}) => { + let x +} diff --git a/test/message/fail/param-arrow-redeclaration-as-let.out b/test/message/fail/param-arrow-redeclaration-as-let.out new file mode 100644 index 0000000000..36eaba3def --- /dev/null +++ b/test/message/fail/param-arrow-redeclaration-as-let.out @@ -0,0 +1,5 @@ +*%(basename)s:6: SyntaxError: Identifier 'x' has already been declared + let x + ^ +SyntaxError: Identifier 'x' has already been declared + diff --git a/test/message/fail/param-async-arrow-redeclaration-as-let.js b/test/message/fail/param-async-arrow-redeclaration-as-let.js new file mode 100644 index 0000000000..efec34ce68 --- /dev/null +++ b/test/message/fail/param-async-arrow-redeclaration-as-let.js @@ -0,0 +1,7 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +async ({x}) => { + let x +} diff --git a/test/message/fail/param-async-arrow-redeclaration-as-let.out b/test/message/fail/param-async-arrow-redeclaration-as-let.out new file mode 100644 index 0000000000..36eaba3def --- /dev/null +++ b/test/message/fail/param-async-arrow-redeclaration-as-let.out @@ -0,0 +1,5 @@ +*%(basename)s:6: SyntaxError: Identifier 'x' has already been declared + let x + ^ +SyntaxError: Identifier 'x' has already been declared + diff --git a/test/message/fail/param-async-function-redeclaration-as-let.js b/test/message/fail/param-async-function-redeclaration-as-let.js new file mode 100644 index 0000000000..fe5b605b65 --- /dev/null +++ b/test/message/fail/param-async-function-redeclaration-as-let.js @@ -0,0 +1,7 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +async function f({x}) { + let x +} diff --git a/test/message/fail/param-async-function-redeclaration-as-let.out b/test/message/fail/param-async-function-redeclaration-as-let.out new file mode 100644 index 0000000000..36eaba3def --- /dev/null +++ b/test/message/fail/param-async-function-redeclaration-as-let.out @@ -0,0 +1,5 @@ +*%(basename)s:6: SyntaxError: Identifier 'x' has already been declared + let x + ^ +SyntaxError: Identifier 'x' has already been declared + diff --git a/test/message/fail/param-function-redeclaration-as-let.js b/test/message/fail/param-function-redeclaration-as-let.js new file mode 100644 index 0000000000..11ae7cf6f9 --- /dev/null +++ b/test/message/fail/param-function-redeclaration-as-let.js @@ -0,0 +1,7 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +function f({x}) { + let x +} diff --git a/test/message/fail/param-function-redeclaration-as-let.out b/test/message/fail/param-function-redeclaration-as-let.out new file mode 100644 index 0000000000..36eaba3def --- /dev/null +++ b/test/message/fail/param-function-redeclaration-as-let.out @@ -0,0 +1,5 @@ +*%(basename)s:6: SyntaxError: Identifier 'x' has already been declared + let x + ^ +SyntaxError: Identifier 'x' has already been declared + diff --git a/test/test262/test262.status b/test/test262/test262.status index 0c7da3d131..954d12e1a7 100644 --- a/test/test262/test262.status +++ b/test/test262/test262.status @@ -471,10 +471,6 @@ 'language/statements/try/tco-finally': [SKIP], 'language/statements/while/tco-body': [SKIP], - # https://bugs.chromium.org/p/v8/issues/detail?id=5064 - 'language/expressions/arrow-function/dflt-params-duplicates': [FAIL], - 'language/expressions/async-arrow-function/dflt-params-duplicates': [FAIL], - # https://bugs.chromium.org/p/v8/issues/detail?id=5327 'built-ins/TypedArrayConstructors/internals/Set/key-is-minus-zero': [FAIL], 'built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-minus-zero': [FAIL],