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

…tion"

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
TBR=neis@chromium.org, adamk@chromium.org

Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: I9685db6e85315ba8a2df87a4537c2bf491e1e35b
Reviewed-on: https://chromium-review.googlesource.com/857593
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50518}

src/ast/ast-numbering.cc

@@ -195,6 +195,11 @@ 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());
@@ -251,6 +256,7 @@ 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,6 +392,14 @@ 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,6 +805,10 @@ Call::CallType Call::GetCallType() const {
     }
   }
 
+  if (expression()->IsResolvedProperty()) {
+    return RESOLVED_PROPERTY_CALL;
+  }
+
   return OTHER_CALL;
 }
 

src/ast/ast.h

@@ -94,6 +94,7 @@ namespace internal {
   V(Literal)                    \
   V(NativeFunctionLiteral)      \
   V(Property)                   \
+  V(ResolvedProperty)           \
   V(RewritableExpression)       \
   V(Spread)                     \
   V(SuperCallReference)         \
@@ -590,11 +591,13 @@ class ForInStatement final : public ForEachStatement {
 class ForOfStatement final : public ForEachStatement {
  public:
   void Initialize(Statement* body, Variable* iterator,
-                  Expression* assign_iterator, Expression* next_result,
-                  Expression* result_done, Expression* assign_each) {
+                  Expression* assign_iterator, Expression* assign_next,
+                  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;
@@ -609,6 +612,9 @@ 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_;
@@ -624,6 +630,12 @@ 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;
 
@@ -637,6 +649,7 @@ class ForOfStatement final : public ForEachStatement {
 
   Variable* iterator_;
   Expression* assign_iterator_;
+  Expression* assign_next_;
   Expression* next_result_;
   Expression* result_done_;
   Expression* assign_each_;
@@ -1607,6 +1620,25 @@ 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:
@@ -1633,6 +1665,7 @@ class Call final : public Expression {
     NAMED_SUPER_PROPERTY_CALL,
     KEYED_SUPER_PROPERTY_CALL,
     SUPER_CALL,
+    RESOLVED_PROPERTY_CALL,
     OTHER_CALL
   };
 
@@ -2104,15 +2137,13 @@ 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) {
@@ -2997,6 +3028,12 @@ 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,6 +326,7 @@ void CallPrinter::VisitProperty(Property* node) {
   }
 }
 
+void CallPrinter::VisitResolvedProperty(ResolvedProperty* node) {}
 
 void CallPrinter::VisitCall(Call* node) {
   bool was_found = false;
@@ -1249,6 +1250,14 @@ 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,10 +232,9 @@ void BaseCollectionsAssembler::AddConstructorEntriesFromIterable(
 
   TNode<Object> add_func = GetAddFunction(variant, context, collection);
   IteratorBuiltinsAssembler iterator_assembler(this->state());
-  TNode<Object> iterator =
-      CAST(iterator_assembler.GetIterator(context, iterable));
+  IteratorRecord iterator = iterator_assembler.GetIterator(context, iterable);
 
-  CSA_ASSERT(this, Word32BinaryNot(IsUndefined(iterator)));
+  CSA_ASSERT(this, Word32BinaryNot(IsUndefined(iterator.object)));
 
   TNode<Object> fast_iterator_result_map =
       LoadContextElement(native_context, Context::ITERATOR_RESULT_MAP_INDEX);

src/builtins/builtins-iterator-gen.cc

@@ -11,19 +11,20 @@ namespace internal {
 
 using compiler::Node;
 
-Node* IteratorBuiltinsAssembler::GetIterator(Node* context, Node* object,
-                                             Label* if_exception,
-                                             Variable* exception) {
+IteratorRecord 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 done(this), if_notobject(this, Label::kDeferred);
+  Label get_next(this), if_notobject(this, Label::kDeferred);
   GotoIf(TaggedIsSmi(iterator), &if_notobject);
-  Branch(IsJSReceiver(iterator), &done, &if_notobject);
+  Branch(IsJSReceiver(iterator), &get_next, &if_notobject);
 
   BIND(&if_notobject);
   {
@@ -34,24 +35,21 @@ Node* IteratorBuiltinsAssembler::GetIterator(Node* context, Node* object,
     Unreachable();
   }
 
-  BIND(&done);
-  return iterator;
+  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)};
 }
 
-Node* IteratorBuiltinsAssembler::IteratorStep(Node* context, Node* iterator,
-                                              Label* if_done,
-                                              Node* fast_iterator_result_map,
-                                              Label* if_exception,
-                                              Variable* exception) {
+Node* IteratorBuiltinsAssembler::IteratorStep(
+    Node* context, const IteratorRecord& 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, next_method, iterator);
+  Node* result = CallJS(callable, context, iterator.next, iterator.object);
   GotoIfException(result, if_exception, exception);
 
   // 3. If Type(result) is not Object, throw a TypeError exception.
@@ -129,20 +127,20 @@ Node* IteratorBuiltinsAssembler::IteratorValue(Node* context, Node* result,
   return var_value.value();
 }
 
-void IteratorBuiltinsAssembler::IteratorCloseOnException(Node* context,
-                                                         Node* iterator,
-                                                         Label* if_exception,
-                                                         Variable* exception) {
+void IteratorBuiltinsAssembler::IteratorCloseOnException(
+    Node* context, const IteratorRecord& 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));
+  CSA_ASSERT(this, IsJSReceiver(iterator.object));
 
   // Let return be ? GetMethod(iterator, "return").
-  Node* method = GetProperty(context, iterator, factory()->return_string());
+  Node* method =
+      GetProperty(context, iterator.object, factory()->return_string());
   GotoIfException(method, if_exception, exception);
 
   // If return is undefined, return Completion(completion).
@@ -152,17 +150,16 @@ void IteratorBuiltinsAssembler::IteratorCloseOnException(Node* context,
     // 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);
+        CallJS(CodeFactory::Call(isolate()), context, method, iterator.object);
     GotoIfException(inner_result, if_exception, nullptr);
 
     // (If completion.[[Type]] is throw) return Completion(completion).
     Goto(if_exception);
   }
 }
 
-void IteratorBuiltinsAssembler::IteratorCloseOnException(Node* context,
-                                                         Node* iterator,
-                                                         Variable* exception) {
+void IteratorBuiltinsAssembler::IteratorCloseOnException(
+    Node* context, const IteratorRecord& iterator, Variable* exception) {
   Label rethrow(this, Label::kDeferred);
   IteratorCloseOnException(context, iterator, &rethrow, exception);
 

src/builtins/builtins-iterator-gen.h

@@ -19,16 +19,17 @@ class IteratorBuiltinsAssembler : public CodeStubAssembler {
 
   // https://tc39.github.io/ecma262/#sec-getiterator --- never used for
   // @@asyncIterator.
-  Node* GetIterator(Node* context, Node* object, Label* if_exception = nullptr,
-                    Variable* exception = nullptr);
+  IteratorRecord 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, Node* iterator, Label* if_done,
-                     Node* fast_iterator_result_map = nullptr,
+  Node* IteratorStep(Node* context, const IteratorRecord& iterator,
+                     Label* if_done, Node* fast_iterator_result_map = nullptr,
                      Label* if_exception = nullptr,
                      Variable* exception = nullptr);
 
@@ -42,9 +43,9 @@ class IteratorBuiltinsAssembler : public CodeStubAssembler {
                       Variable* exception = nullptr);
 
   // https://tc39.github.io/ecma262/#sec-iteratorclose
-  void IteratorCloseOnException(Node* context, Node* iterator,
+  void IteratorCloseOnException(Node* context, const IteratorRecord& iterator,
                                 Label* if_exception, Variable* exception);
-  void IteratorCloseOnException(Node* context, Node* iterator,
+  void IteratorCloseOnException(Node* context, const IteratorRecord& iterator,
                                 Variable* exception);
 };
 

src/builtins/builtins-promise-gen.cc

@@ -1744,8 +1744,9 @@ TF_BUILTIN(PerformNativePromiseThen, PromiseBuiltinsAssembler) {
 }
 
 Node* PromiseBuiltinsAssembler::PerformPromiseAll(
-    Node* context, Node* constructor, Node* capability, Node* iterator,
-    Label* if_exception, Variable* var_exception) {
+    Node* context, Node* constructor, Node* capability,
+    const IteratorRecord& iterator, Label* if_exception,
+    Variable* var_exception) {
   IteratorBuiltinsAssembler iter_assembler(state());
   Label close_iterator(this);
 
@@ -1951,7 +1952,7 @@ TF_BUILTIN(PromiseAll, PromiseBuiltinsAssembler) {
   // Let iterator be GetIterator(iterable).
   // IfAbruptRejectPromise(iterator, promiseCapability).
   Node* const iterable = Parameter(Descriptor::kIterable);
-  Node* const iterator = iter_assembler.GetIterator(
+  IteratorRecord iterator = iter_assembler.GetIterator(
       context, iterable, &reject_promise, &var_exception);
 
   // Let result be PerformPromiseAll(iteratorRecord, C, promiseCapability).
@@ -2088,7 +2089,7 @@ TF_BUILTIN(PromiseRace, PromiseBuiltinsAssembler) {
   // Let iterator be GetIterator(iterable).
   // IfAbruptRejectPromise(iterator, promiseCapability).
   Node* const iterable = Parameter(Descriptor::kIterable);
-  Node* const iterator = iter_assembler.GetIterator(
+  IteratorRecord 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,
-                          Node* iterator, Label* if_exception,
+                          const IteratorRecord& record, Label* if_exception,
                           Variable* var_exception);
 
   Node* IncrementSmiCell(Node* cell, Label* if_overflow = nullptr);

src/code-stub-assembler.h

@@ -71,6 +71,17 @@ 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"