From 029b8a23790d4f7ec0012fe40057775b5abdfce0 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Thu, 24 Jul 2014 13:40:08 +0000 Subject: [PATCH] For-of on null or undefined is an error The latest ES6 draft changed the behavior of for-of on null / undefined, which for once is a simplification. R=rossberg@chromium.org BUG= Review URL: https://codereview.chromium.org/416033002 Patch from Andy Wingo . git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22602 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 9 --------- src/arm64/full-codegen-arm64.cc | 10 ---------- src/ast-value-factory.h | 1 - src/ast.h | 10 +--------- src/ia32/full-codegen-ia32.cc | 9 --------- src/mips/full-codegen-mips.cc | 10 ---------- src/mips64/full-codegen-mips64.cc | 10 ---------- src/parser.cc | 18 +++--------------- src/x64/full-codegen-x64.cc | 9 --------- src/x87/full-codegen-x87.cc | 9 --------- test/mjsunit/harmony/iteration-semantics.js | 6 +++--- 11 files changed, 7 insertions(+), 94 deletions(-) diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 303f830a7e..9e42c4109d 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -1276,15 +1276,6 @@ void FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { Iteration loop_statement(this, stmt); increment_loop_depth(); - // var iterable = subject - VisitForAccumulatorValue(stmt->assign_iterable()); - - // As with for-in, skip the loop if the iterator is null or undefined. - __ CompareRoot(r0, Heap::kUndefinedValueRootIndex); - __ b(eq, loop_statement.break_label()); - __ CompareRoot(r0, Heap::kNullValueRootIndex); - __ b(eq, loop_statement.break_label()); - // var iterator = iterable[Symbol.iterator](); VisitForEffect(stmt->assign_iterator()); diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc index 5e2b129c5a..5fbf283f00 100644 --- a/src/arm64/full-codegen-arm64.cc +++ b/src/arm64/full-codegen-arm64.cc @@ -1271,16 +1271,6 @@ void FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { Iteration loop_statement(this, stmt); increment_loop_depth(); - // var iterable = subject - VisitForAccumulatorValue(stmt->assign_iterable()); - - // As with for-in, skip the loop if the iterator is null or undefined. - Register iterator = x0; - __ JumpIfRoot(iterator, Heap::kUndefinedValueRootIndex, - loop_statement.break_label()); - __ JumpIfRoot(iterator, Heap::kNullValueRootIndex, - loop_statement.break_label()); - // var iterator = iterable[Symbol.iterator](); VisitForEffect(stmt->assign_iterator()); diff --git a/src/ast-value-factory.h b/src/ast-value-factory.h index 4711bb4a22..bb93d10cd2 100644 --- a/src/ast-value-factory.h +++ b/src/ast-value-factory.h @@ -243,7 +243,6 @@ class AstValue : public ZoneObject { F(dot_for, ".for") \ F(dot_generator, ".generator") \ F(dot_generator_object, ".generator_object") \ - F(dot_iterable, ".iterable") \ F(dot_iterator, ".iterator") \ F(dot_module, ".module") \ F(dot_result, ".result") \ diff --git a/src/ast.h b/src/ast.h index ecb42c936c..b48a1f164a 100644 --- a/src/ast.h +++ b/src/ast.h @@ -961,13 +961,11 @@ class ForOfStatement V8_FINAL : public ForEachStatement { void Initialize(Expression* each, Expression* subject, Statement* body, - Expression* assign_iterable, Expression* assign_iterator, Expression* next_result, Expression* result_done, Expression* assign_each) { ForEachStatement::Initialize(each, subject, body); - assign_iterable_ = assign_iterable; assign_iterator_ = assign_iterator; next_result_ = next_result; result_done_ = result_done; @@ -978,12 +976,7 @@ class ForOfStatement V8_FINAL : public ForEachStatement { return subject(); } - // var iterable = subject; - Expression* assign_iterable() const { - return assign_iterable_; - } - - // var iterator = iterable[Symbol.iterator](); + // var iterator = subject[Symbol.iterator](); Expression* assign_iterator() const { return assign_iterator_; } @@ -1018,7 +1011,6 @@ class ForOfStatement V8_FINAL : public ForEachStatement { back_edge_id_(GetNextId(zone)) { } - Expression* assign_iterable_; Expression* assign_iterator_; Expression* next_result_; Expression* result_done_; diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 03ceab9a87..6aa2772853 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -1207,15 +1207,6 @@ void FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { Iteration loop_statement(this, stmt); increment_loop_depth(); - // var iterable = subject - VisitForAccumulatorValue(stmt->assign_iterable()); - - // As with for-in, skip the loop if the iterator is null or undefined. - __ CompareRoot(eax, Heap::kUndefinedValueRootIndex); - __ j(equal, loop_statement.break_label()); - __ CompareRoot(eax, Heap::kNullValueRootIndex); - __ j(equal, loop_statement.break_label()); - // var iterator = iterable[Symbol.iterator](); VisitForEffect(stmt->assign_iterator()); diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 7174a22b30..1d8c7cc9dc 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -1272,16 +1272,6 @@ void FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { Iteration loop_statement(this, stmt); increment_loop_depth(); - // var iterable = subject - VisitForAccumulatorValue(stmt->assign_iterable()); - __ mov(a0, v0); - - // As with for-in, skip the loop if the iterator is null or undefined. - __ LoadRoot(at, Heap::kUndefinedValueRootIndex); - __ Branch(loop_statement.break_label(), eq, a0, Operand(at)); - __ LoadRoot(at, Heap::kNullValueRootIndex); - __ Branch(loop_statement.break_label(), eq, a0, Operand(at)); - // var iterator = iterable[Symbol.iterator](); VisitForEffect(stmt->assign_iterator()); diff --git a/src/mips64/full-codegen-mips64.cc b/src/mips64/full-codegen-mips64.cc index 6ac77e2684..212d917e39 100644 --- a/src/mips64/full-codegen-mips64.cc +++ b/src/mips64/full-codegen-mips64.cc @@ -1267,16 +1267,6 @@ void FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { Iteration loop_statement(this, stmt); increment_loop_depth(); - // var iterable = subject - VisitForAccumulatorValue(stmt->assign_iterable()); - __ mov(a0, v0); - - // As with for-in, skip the loop if the iterator is null or undefined. - __ LoadRoot(at, Heap::kUndefinedValueRootIndex); - __ Branch(loop_statement.break_label(), eq, a0, Operand(at)); - __ LoadRoot(at, Heap::kNullValueRootIndex); - __ Branch(loop_statement.break_label(), eq, a0, Operand(at)); - // var iterator = iterable[Symbol.iterator](); VisitForEffect(stmt->assign_iterator()); diff --git a/src/parser.cc b/src/parser.cc index 43196c49e9..170b1f5a35 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2749,37 +2749,26 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt, ForOfStatement* for_of = stmt->AsForOfStatement(); if (for_of != NULL) { - Variable* iterable = scope_->DeclarationScope()->NewTemporary( - ast_value_factory_->dot_iterable_string()); Variable* iterator = scope_->DeclarationScope()->NewTemporary( ast_value_factory_->dot_iterator_string()); Variable* result = scope_->DeclarationScope()->NewTemporary( ast_value_factory_->dot_result_string()); - Expression* assign_iterable; Expression* assign_iterator; Expression* next_result; Expression* result_done; Expression* assign_each; - // var iterable = subject; + // var iterator = subject[Symbol.iterator](); { - Expression* iterable_proxy = factory()->NewVariableProxy(iterable); - assign_iterable = factory()->NewAssignment( - Token::ASSIGN, iterable_proxy, subject, subject->position()); - } - - // var iterator = iterable[Symbol.iterator](); - { - Expression* iterable_proxy = factory()->NewVariableProxy(iterable); Expression* iterator_symbol_literal = factory()->NewSymbolLiteral("symbolIterator", RelocInfo::kNoPosition); // FIXME(wingo): Unhappily, it will be a common error that the RHS of a // for-of doesn't have a Symbol.iterator property. We should do better // than informing the user that "undefined is not a function". int pos = subject->position(); - Expression* iterator_property = factory()->NewProperty( - iterable_proxy, iterator_symbol_literal, pos); + Expression* iterator_property = + factory()->NewProperty(subject, iterator_symbol_literal, pos); ZoneList* iterator_arguments = new(zone()) ZoneList(0, zone()); Expression* iterator_call = factory()->NewCall( @@ -2826,7 +2815,6 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt, } for_of->Initialize(each, subject, body, - assign_iterable, assign_iterator, next_result, result_done, diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index f79e23017e..daacbd6026 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -1241,15 +1241,6 @@ void FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { Iteration loop_statement(this, stmt); increment_loop_depth(); - // var iterable = subject - VisitForAccumulatorValue(stmt->assign_iterable()); - - // As with for-in, skip the loop if the iterable is null or undefined. - __ CompareRoot(rax, Heap::kUndefinedValueRootIndex); - __ j(equal, loop_statement.break_label()); - __ CompareRoot(rax, Heap::kNullValueRootIndex); - __ j(equal, loop_statement.break_label()); - // var iterator = iterable[Symbol.iterator](); VisitForEffect(stmt->assign_iterator()); diff --git a/src/x87/full-codegen-x87.cc b/src/x87/full-codegen-x87.cc index 8194db658f..d5baad813c 100644 --- a/src/x87/full-codegen-x87.cc +++ b/src/x87/full-codegen-x87.cc @@ -1204,15 +1204,6 @@ void FullCodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { Iteration loop_statement(this, stmt); increment_loop_depth(); - // var iterable = subject - VisitForAccumulatorValue(stmt->assign_iterable()); - - // As with for-in, skip the loop if the iterator is null or undefined. - __ CompareRoot(eax, Heap::kUndefinedValueRootIndex); - __ j(equal, loop_statement.break_label()); - __ CompareRoot(eax, Heap::kNullValueRootIndex); - __ j(equal, loop_statement.break_label()); - // var iterator = iterable[Symbol.iterator](); VisitForEffect(stmt->assign_iterator()); diff --git a/test/mjsunit/harmony/iteration-semantics.js b/test/mjsunit/harmony/iteration-semantics.js index 803f12faa9..cc3cea8092 100644 --- a/test/mjsunit/harmony/iteration-semantics.js +++ b/test/mjsunit/harmony/iteration-semantics.js @@ -213,9 +213,9 @@ assertEquals([1, 2], { value: 37, done: true }, never_getter(never_getter({}, 'done'), 'value')]))); -// Null and undefined do not cause an error. -assertEquals(0, fold(sum, 0, unreachable(null))); -assertEquals(0, fold(sum, 0, unreachable(undefined))); +// Unlike the case with for-in, null and undefined cause an error. +assertThrows('fold(sum, 0, unreachable(null))', TypeError); +assertThrows('fold(sum, 0, unreachable(undefined))', TypeError); // Other non-iterators do cause an error. assertThrows('fold(sum, 0, unreachable({}))', TypeError);