Revert "[esnext] load iterator.next only once at beginning of itera…

…tion"

This reverts commit bf4cc9e.

Reason for revert: Breaks windows with msvc and linux with gcc
https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20msvc/builds/841
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20gcc%204.8/builds/17265

Original change's description:
> [esnext] load `iterator.next` only once at beginning of iteration
> 
> tc39/ecma262#988 gained concensus during the
> september 2017 TC39 meetings. This moves the load of the "next" method
> to the very beginning of the iteration protocol, rather than during
> each iteration step.
> 
> This impacts:
> 
> - yield*
> - for-of loops
> - spread arguments
> - array spreads
> 
> In the v8 implementation, this also affects async iteration versions of
> these things (the sole exception being the Async-From-Sync iterator,
> which requires a few more changes to work with this, likely done in a
> followup patch).
> 
> This change introduces a new AST node, ResolvedProperty, which can be used
> as a callee by Call nodes to produce the same bytecode as Property calls,
> without observably re-loading the property. This is used in several
> AST-desugarings involving the iteration protocol.
> 
> BUG=v8:6861, v8:5699
> R=​rmcilroy@chromium.org, neis@chromium.org, adamk@chromium.org
> 
> Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
> Change-Id: Ib81106a0182687fc5efea0bc32302ad06376773b
> Reviewed-on: https://chromium-review.googlesource.com/687997
> Commit-Queue: Caitlin Potter <caitp@igalia.com>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Adam Klein <adamk@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50452}

TBR=rmcilroy@chromium.org,adamk@chromium.org,neis@chromium.org,caitp@igalia.com,caitp@chromium.org

Change-Id: I1797c0d596dfd6850d6f0f505f591a7a990dd1f1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6861, v8:5699
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/857616
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50454}

src/ast/ast-numbering.cc

@@ -195,11 +195,6 @@ void AstNumberingVisitor::VisitProperty(Property* node) {
   Visit(node->obj());
 }
 
-void AstNumberingVisitor::VisitResolvedProperty(ResolvedProperty* node) {
-  Visit(node->object());
-  Visit(node->property());
-}
-
 void AstNumberingVisitor::VisitAssignment(Assignment* node) {
   Visit(node->target());
   Visit(node->value());
@@ -256,7 +251,6 @@ void AstNumberingVisitor::VisitForInStatement(ForInStatement* node) {
 
 void AstNumberingVisitor::VisitForOfStatement(ForOfStatement* node) {
   Visit(node->assign_iterator());  // Not part of loop.
-  Visit(node->assign_next());
   node->set_first_suspend_id(suspend_count_);
   Visit(node->next_result());
   Visit(node->result_done());

src/ast/ast-traversal-visitor.h

@@ -392,14 +392,6 @@ void AstTraversalVisitor<Subclass>::VisitProperty(Property* expr) {
   RECURSE_EXPRESSION(Visit(expr->key()));
 }
 
-template <class Subclass>
-void AstTraversalVisitor<Subclass>::VisitResolvedProperty(
-    ResolvedProperty* expr) {
-  PROCESS_EXPRESSION(expr);
-  RECURSE_EXPRESSION(VisitVariableProxy(expr->object()));
-  RECURSE_EXPRESSION(VisitVariableProxy(expr->property()));
-}
-
 template <class Subclass>
 void AstTraversalVisitor<Subclass>::VisitCall(Call* expr) {
   PROCESS_EXPRESSION(expr);

src/ast/ast.cc

@@ -805,10 +805,6 @@ Call::CallType Call::GetCallType() const {
     }
   }
 
-  if (expression()->IsResolvedProperty()) {
-    return RESOLVED_PROPERTY_CALL;
-  }
-
   return OTHER_CALL;
 }
 

src/ast/ast.h

@@ -94,7 +94,6 @@ namespace internal {
   V(Literal)                    \
   V(NativeFunctionLiteral)      \
   V(Property)                   \
-  V(ResolvedProperty)           \
   V(RewritableExpression)       \
   V(Spread)                     \
   V(SuperCallReference)         \
@@ -591,13 +590,11 @@ class ForInStatement final : public ForEachStatement {
 class ForOfStatement final : public ForEachStatement {
  public:
   void Initialize(Statement* body, Variable* iterator,
-                  Expression* assign_iterator, Expression* assign_next,
-                  Expression* next_result, Expression* result_done,
-                  Expression* assign_each) {
+                  Expression* assign_iterator, Expression* next_result,
+                  Expression* result_done, Expression* assign_each) {
     ForEachStatement::Initialize(body);
     iterator_ = iterator;
     assign_iterator_ = assign_iterator;
-    assign_next_ = assign_next;
     next_result_ = next_result;
     result_done_ = result_done;
     assign_each_ = assign_each;
@@ -612,9 +609,6 @@ class ForOfStatement final : public ForEachStatement {
     return assign_iterator_;
   }
 
-  // iteratorRecord.next = iterator.next
-  Expression* assign_next() const { return assign_next_; }
-
   // result = iterator.next()  // with type check
   Expression* next_result() const {
     return next_result_;
@@ -630,12 +624,6 @@ class ForOfStatement final : public ForEachStatement {
     return assign_each_;
   }
 
-  void set_assign_iterator(Expression* e) { assign_iterator_ = e; }
-  void set_assign_next(Expression* e) { assign_next_ = e; }
-  void set_next_result(Expression* e) { next_result_ = e; }
-  void set_result_done(Expression* e) { result_done_ = e; }
-  void set_assign_each(Expression* e) { assign_each_ = e; }
-
  private:
   friend class AstNodeFactory;
 
@@ -649,7 +637,6 @@ class ForOfStatement final : public ForEachStatement {
 
   Variable* iterator_;
   Expression* assign_iterator_;
-  Expression* assign_next_;
   Expression* next_result_;
   Expression* result_done_;
   Expression* assign_each_;
@@ -1620,25 +1607,6 @@ class Property final : public Expression {
   Expression* key_;
 };
 
-// ResolvedProperty pairs a receiver field with a value field. It allows Call
-// to support arbitrary receivers while still taking advantage of TypeFeedback.
-class ResolvedProperty final : public Expression {
- public:
-  VariableProxy* object() const { return object_; }
-  VariableProxy* property() const { return property_; }
-
-  void set_object(VariableProxy* e) { object_ = e; }
-  void set_property(VariableProxy* e) { property_ = e; }
-
- private:
-  friend class AstNodeFactory;
-
-  ResolvedProperty(VariableProxy* obj, VariableProxy* property, int pos)
-      : Expression(pos, kResolvedProperty), object_(obj), property_(property) {}
-
-  VariableProxy* object_;
-  VariableProxy* property_;
-};
 
 class Call final : public Expression {
  public:
@@ -1665,7 +1633,6 @@ class Call final : public Expression {
     NAMED_SUPER_PROPERTY_CALL,
     KEYED_SUPER_PROPERTY_CALL,
     SUPER_CALL,
-    RESOLVED_PROPERTY_CALL,
     OTHER_CALL
   };
 
@@ -2137,13 +2104,15 @@ class YieldStar final : public Suspend {
   //   - One for awaiting the iterator result yielded by the delegated iterator
   //     (await_delegated_iterator_output_suspend_id)
   int await_iterator_close_suspend_id() const {
+    DCHECK_NE(-1, await_iterator_close_suspend_id_);
     return await_iterator_close_suspend_id_;
   }
   void set_await_iterator_close_suspend_id(int id) {
     await_iterator_close_suspend_id_ = id;
   }
 
   int await_delegated_iterator_output_suspend_id() const {
+    DCHECK_NE(-1, await_delegated_iterator_output_suspend_id_);
     return await_delegated_iterator_output_suspend_id_;
   }
   void set_await_delegated_iterator_output_suspend_id(int id) {
@@ -3028,12 +2997,6 @@ class AstNodeFactory final BASE_EMBEDDED {
     return new (zone_) Property(obj, key, pos);
   }
 
-  ResolvedProperty* NewResolvedProperty(VariableProxy* obj,
-                                        VariableProxy* property,
-                                        int pos = kNoSourcePosition) {
-    return new (zone_) ResolvedProperty(obj, property, pos);
-  }
-
   Call* NewCall(Expression* expression, ZoneList<Expression*>* arguments,
                 int pos, Call::PossiblyEval possibly_eval = Call::NOT_EVAL) {
     return new (zone_) Call(expression, arguments, pos, possibly_eval);

src/ast/prettyprinter.cc

@@ -326,7 +326,6 @@ void CallPrinter::VisitProperty(Property* node) {
   }
 }
 
-void CallPrinter::VisitResolvedProperty(ResolvedProperty* node) {}
 
 void CallPrinter::VisitCall(Call* node) {
   bool was_found = false;
@@ -1250,14 +1249,6 @@ void AstPrinter::VisitProperty(Property* node) {
   }
 }
 
-void AstPrinter::VisitResolvedProperty(ResolvedProperty* node) {
-  EmbeddedVector<char, 128> buf;
-  SNPrintF(buf, "RESOLVED-PROPERTY");
-  IndentedScope indent(this, buf.start(), node->position());
-
-  PrintIndentedVisit("RECEIVER", node->object());
-  PrintIndentedVisit("PROPERTY", node->property());
-}
 
 void AstPrinter::VisitCall(Call* node) {
   EmbeddedVector<char, 128> buf;

src/builtins/builtins-collections-gen.cc

@@ -232,9 +232,10 @@ void BaseCollectionsAssembler::AddConstructorEntriesFromIterable(
 
   TNode<Object> add_func = GetAddFunction(variant, context, collection);
   IteratorBuiltinsAssembler iterator_assembler(this->state());
-  IteratorRecord iterator = iterator_assembler.GetIterator(context, iterable);
+  TNode<Object> iterator =
+      CAST(iterator_assembler.GetIterator(context, iterable));
 
-  CSA_ASSERT(this, Word32BinaryNot(IsUndefined(iterator.object)));
+  CSA_ASSERT(this, Word32BinaryNot(IsUndefined(iterator)));
 
   TNode<Object> fast_iterator_result_map =
       LoadContextElement(native_context, Context::ITERATOR_RESULT_MAP_INDEX);

src/builtins/builtins-iterator-gen.cc

@@ -11,20 +11,19 @@ namespace internal {
 
 using compiler::Node;
 
-IteratorRecord IteratorBuiltinsAssembler::GetIterator(Node* context,
-                                                      Node* object,
-                                                      Label* if_exception,
-                                                      Variable* exception) {
+Node* IteratorBuiltinsAssembler::GetIterator(Node* context, Node* object,
+                                             Label* if_exception,
+                                             Variable* exception) {
   Node* method = GetProperty(context, object, factory()->iterator_symbol());
   GotoIfException(method, if_exception, exception);
 
   Callable callable = CodeFactory::Call(isolate());
   Node* iterator = CallJS(callable, context, method, object);
   GotoIfException(iterator, if_exception, exception);
 
-  Label get_next(this), if_notobject(this, Label::kDeferred);
+  Label done(this), if_notobject(this, Label::kDeferred);
   GotoIf(TaggedIsSmi(iterator), &if_notobject);
-  Branch(IsJSReceiver(iterator), &get_next, &if_notobject);
+  Branch(IsJSReceiver(iterator), &done, &if_notobject);
 
   BIND(&if_notobject);
   {
@@ -35,21 +34,24 @@ IteratorRecord IteratorBuiltinsAssembler::GetIterator(Node* context,
     Unreachable();
   }
 
-  BIND(&get_next);
-  Node* const next = GetProperty(context, iterator, factory()->next_string());
-  GotoIfException(next, if_exception, exception);
-
-  return IteratorRecord{TNode<JSReceiver>::UncheckedCast(iterator),
-                        TNode<Object>::UncheckedCast(next)};
+  BIND(&done);
+  return iterator;
 }
 
-Node* IteratorBuiltinsAssembler::IteratorStep(
-    Node* context, const IteratorRecord& iterator, Label* if_done,
-    Node* fast_iterator_result_map, Label* if_exception, Variable* exception) {
+Node* IteratorBuiltinsAssembler::IteratorStep(Node* context, Node* iterator,
+                                              Label* if_done,
+                                              Node* fast_iterator_result_map,
+                                              Label* if_exception,
+                                              Variable* exception) {
   DCHECK_NOT_NULL(if_done);
+
+  // IteratorNext
+  Node* next_method = GetProperty(context, iterator, factory()->next_string());
+  GotoIfException(next_method, if_exception, exception);
+
   // 1. a. Let result be ? Invoke(iterator, "next", « »).
   Callable callable = CodeFactory::Call(isolate());
-  Node* result = CallJS(callable, context, iterator.next, iterator.object);
+  Node* result = CallJS(callable, context, next_method, iterator);
   GotoIfException(result, if_exception, exception);
 
   // 3. If Type(result) is not Object, throw a TypeError exception.
@@ -127,20 +129,20 @@ Node* IteratorBuiltinsAssembler::IteratorValue(Node* context, Node* result,
   return var_value.value();
 }
 
-void IteratorBuiltinsAssembler::IteratorCloseOnException(
-    Node* context, const IteratorRecord& iterator, Label* if_exception,
-    Variable* exception) {
+void IteratorBuiltinsAssembler::IteratorCloseOnException(Node* context,
+                                                         Node* iterator,
+                                                         Label* if_exception,
+                                                         Variable* exception) {
   // Perform ES #sec-iteratorclose when an exception occurs. This simpler
   // algorithm does not include redundant steps which are never reachable from
   // the spec IteratorClose algorithm.
   DCHECK_NOT_NULL(if_exception);
   DCHECK_NOT_NULL(exception);
   CSA_ASSERT(this, IsNotTheHole(exception->value()));
-  CSA_ASSERT(this, IsJSReceiver(iterator.object));
+  CSA_ASSERT(this, IsJSReceiver(iterator));
 
   // Let return be ? GetMethod(iterator, "return").
-  Node* method =
-      GetProperty(context, iterator.object, factory()->return_string());
+  Node* method = GetProperty(context, iterator, factory()->return_string());
   GotoIfException(method, if_exception, exception);
 
   // If return is undefined, return Completion(completion).
@@ -150,16 +152,17 @@ void IteratorBuiltinsAssembler::IteratorCloseOnException(
     // Let innerResult be Call(return, iterator, « »).
     // If an exception occurs, the original exception remains bound
     Node* inner_result =
-        CallJS(CodeFactory::Call(isolate()), context, method, iterator.object);
+        CallJS(CodeFactory::Call(isolate()), context, method, iterator);
     GotoIfException(inner_result, if_exception, nullptr);
 
     // (If completion.[[Type]] is throw) return Completion(completion).
     Goto(if_exception);
   }
 }
 
-void IteratorBuiltinsAssembler::IteratorCloseOnException(
-    Node* context, const IteratorRecord& iterator, Variable* exception) {
+void IteratorBuiltinsAssembler::IteratorCloseOnException(Node* context,
+                                                         Node* iterator,
+                                                         Variable* exception) {
   Label rethrow(this, Label::kDeferred);
   IteratorCloseOnException(context, iterator, &rethrow, exception);
 

src/builtins/builtins-iterator-gen.h

@@ -19,17 +19,16 @@ class IteratorBuiltinsAssembler : public CodeStubAssembler {
 
   // https://tc39.github.io/ecma262/#sec-getiterator --- never used for
   // @@asyncIterator.
-  IteratorRecord GetIterator(Node* context, Node* object,
-                             Label* if_exception = nullptr,
-                             Variable* exception = nullptr);
+  Node* GetIterator(Node* context, Node* object, Label* if_exception = nullptr,
+                    Variable* exception = nullptr);
 
   // https://tc39.github.io/ecma262/#sec-iteratorstep
   // Returns `false` if the iterator is done, otherwise returns an
   // iterator result.
   // `fast_iterator_result_map` refers to the map for the JSIteratorResult
   // object, loaded from the native context.
-  Node* IteratorStep(Node* context, const IteratorRecord& iterator,
-                     Label* if_done, Node* fast_iterator_result_map = nullptr,
+  Node* IteratorStep(Node* context, Node* iterator, Label* if_done,
+                     Node* fast_iterator_result_map = nullptr,
                      Label* if_exception = nullptr,
                      Variable* exception = nullptr);
 
@@ -43,9 +42,9 @@ class IteratorBuiltinsAssembler : public CodeStubAssembler {
                       Variable* exception = nullptr);
 
   // https://tc39.github.io/ecma262/#sec-iteratorclose
-  void IteratorCloseOnException(Node* context, const IteratorRecord& iterator,
+  void IteratorCloseOnException(Node* context, Node* iterator,
                                 Label* if_exception, Variable* exception);
-  void IteratorCloseOnException(Node* context, const IteratorRecord& iterator,
+  void IteratorCloseOnException(Node* context, Node* iterator,
                                 Variable* exception);
 };
 

src/builtins/builtins-promise-gen.cc

@@ -1807,9 +1807,8 @@ TF_BUILTIN(PerformNativePromiseThen, PromiseBuiltinsAssembler) {
 }
 
 Node* PromiseBuiltinsAssembler::PerformPromiseAll(
-    Node* context, Node* constructor, Node* capability,
-    const IteratorRecord& iterator, Label* if_exception,
-    Variable* var_exception) {
+    Node* context, Node* constructor, Node* capability, Node* iterator,
+    Label* if_exception, Variable* var_exception) {
   IteratorBuiltinsAssembler iter_assembler(state());
   Label close_iterator(this);
 
@@ -2015,7 +2014,7 @@ TF_BUILTIN(PromiseAll, PromiseBuiltinsAssembler) {
   // Let iterator be GetIterator(iterable).
   // IfAbruptRejectPromise(iterator, promiseCapability).
   Node* const iterable = Parameter(Descriptor::kIterable);
-  IteratorRecord iterator = iter_assembler.GetIterator(
+  Node* const iterator = iter_assembler.GetIterator(
       context, iterable, &reject_promise, &var_exception);
 
   // Let result be PerformPromiseAll(iteratorRecord, C, promiseCapability).
@@ -2152,7 +2151,7 @@ TF_BUILTIN(PromiseRace, PromiseBuiltinsAssembler) {
   // Let iterator be GetIterator(iterable).
   // IfAbruptRejectPromise(iterator, promiseCapability).
   Node* const iterable = Parameter(Descriptor::kIterable);
-  IteratorRecord iterator = iter_assembler.GetIterator(
+  Node* const iterator = iter_assembler.GetIterator(
       context, iterable, &reject_promise, &var_exception);
 
   // Let result be PerformPromiseRace(iteratorRecord, C, promiseCapability).

src/builtins/builtins-promise-gen.h

@@ -157,7 +157,7 @@ class PromiseBuiltinsAssembler : public CodeStubAssembler {
   Node* CreateThrowerFunction(Node* reason, Node* native_context);
 
   Node* PerformPromiseAll(Node* context, Node* constructor, Node* capability,
-                          const IteratorRecord& record, Label* if_exception,
+                          Node* iterator, Label* if_exception,
                           Variable* var_exception);
 
   Node* IncrementSmiCell(Node* cell, Label* if_overflow = nullptr);

src/code-stub-assembler.h

@@ -71,17 +71,6 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol };
     promise_default_resolve_handler_symbol,                                   \
     PromiseDefaultResolveHandlerSymbol)
 
-// Returned from IteratorBuiltinsAssembler::GetIterator(). Struct is declared
-// here to simplify use in other generated builtins.
-struct IteratorRecord {
- public:
-  // iteratorRecord.[[Iterator]]
-  compiler::TNode<JSReceiver> object;
-
-  // iteratorRecord.[[NextMethod]]
-  compiler::TNode<Object> next;
-};
-
 // Provides JavaScript-specific "macro-assembler" functionality on top of the
 // CodeAssembler. By factoring the JavaScript-isms out of the CodeAssembler,
 // it's possible to add JavaScript-specific useful CodeAssembler "macros"