[parser] Improve error message for destructuring non iterable

Print the object that is being destructured and update the error message. Previously, d8> var [a] = {} (d8):1: TypeError: [Symbol.iterator] is not a function Now, d8> var [a] = {} (d8):1: TypeError: {} is not iterable Bug: v8:6513, v8:5532 Change-Id: I5cbfe7c7e20632bce1a48bd38a1b0c98d0ff0660 Reviewed-on: https://chromium-review.googlesource.com/557370 Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Reviewed-by: Adam Klein <adamk@chromium.org> Reviewed-by: Caitlin Potter <caitp@igalia.com> Cr-Commit-Position: refs/heads/master@{#46457}

[parser] Improve error message for destructuring non iterable
Print the object that is being destructured and update the error message. Previously, d8> var [a] = {} (d8):1: TypeError: [Symbol.iterator] is not a function Now, d8> var [a] = {} (d8):1: TypeError: {} is not iterable Bug: v8:6513, v8:5532 Change-Id: I5cbfe7c7e20632bce1a48bd38a1b0c98d0ff0660 Reviewed-on: https://chromium-review.googlesource.com/557370 Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org> Reviewed-by: Adam Klein <adamk@chromium.org> Reviewed-by: Caitlin Potter <caitp@igalia.com> Cr-Commit-Position: refs/heads/master@{#46457}
d8e1c484 · Sathya Gunasekaran · Commit Bot · aeb8f332 · d8e1c484 · d8e1c484
Commit d8e1c484 authored Jul 06, 2017 by Sathya Gunasekaran Committed by Commit Bot Jul 06, 2017
22 changed files
--- a/src/ast/ast.h
+++ b/src/ast/ast.h
@@ -2914,14 +2914,35 @@ class GetIterator final : public Expression {
    return async_iterator_call_feedback_slot_;
  }

+  Expression* iterable_for_call_printer() const {
+    return destructured_iterable_ != nullptr ? destructured_iterable_
+                                             : iterable_;
+  }
+
 private:
  friend class AstNodeFactory;

-  explicit GetIterator(Expression* iterable, IteratorType hint, int pos)
-      : Expression(pos, kGetIterator), hint_(hint), iterable_(iterable) {}
+  GetIterator(Expression* iterable, Expression* destructured_iterable,
+              IteratorType hint, int pos)
+      : Expression(pos, kGetIterator),
+        hint_(hint),
+        iterable_(iterable),
+        destructured_iterable_(destructured_iterable) {}
+
+  GetIterator(Expression* iterable, IteratorType hint, int pos)
+      : Expression(pos, kGetIterator),
+        hint_(hint),
+        iterable_(iterable),
+        destructured_iterable_(nullptr) {}

  IteratorType hint_;
  Expression* iterable_;
+
+  // iterable_ is the variable proxy, while destructured_iterable_ points to
+  // the raw value stored in the variable proxy. This is only used for
+  // pretty printing error messages.
+  Expression* destructured_iterable_;
+
  FeedbackSlot iterator_property_feedback_slot_;
  FeedbackSlot iterator_call_feedback_slot_;
  FeedbackSlot async_iterator_property_feedback_slot_;
@@ -3522,6 +3543,12 @@ class AstNodeFactory final BASE_EMBEDDED {
    return new (zone_) EmptyParentheses(pos);
  }

+  GetIterator* NewGetIterator(Expression* iterable,
+                              Expression* destructured_iterable,
+                              IteratorType hint, int pos) {
+    return new (zone_) GetIterator(iterable, destructured_iterable, hint, pos);
+  }
+
  GetIterator* NewGetIterator(Expression* iterable, IteratorType hint,
                              int pos) {
    return new (zone_) GetIterator(iterable, hint, pos);

--- a/src/ast/prettyprinter.cc
+++ b/src/ast/prettyprinter.cc
@@ -22,10 +22,15 @@ CallPrinter::CallPrinter(Isolate* isolate, bool is_user_js)
  num_prints_ = 0;
  found_ = false;
  done_ = false;
+  iterator_hint_ = IteratorHint::kNone;
  is_user_js_ = is_user_js;
  InitializeAstVisitor(isolate);
 }

+CallPrinter::IteratorHint CallPrinter::GetIteratorHint() const {
+  return iterator_hint_;
+}
+
 Handle<String> CallPrinter::Print(FunctionLiteral* program, int position) {
  num_prints_ = 0;
  position_ = position;
@@ -223,9 +228,11 @@ void CallPrinter::VisitRegExpLiteral(RegExpLiteral* node) {


 void CallPrinter::VisitObjectLiteral(ObjectLiteral* node) {
+  Print("{");
  for (int i = 0; i < node->properties()->length(); i++) {
    Find(node->properties()->at(i)->value());
  }
+  Print("}");
 }


@@ -372,17 +379,14 @@ void CallPrinter::VisitEmptyParentheses(EmptyParentheses* node) {
 }

 void CallPrinter::VisitGetIterator(GetIterator* node) {
-  // Because CallPrinter is used by RenderCallSite() in runtime-internal.cc,
-  // and the GetIterator node results in a Call, either to a [@@iterator] or
-  // [@@asyncIterator]. It's unknown which call this error refers to, so we
-  // assume it's the first call.
  bool was_found = !found_ && node->position() == position_;
  if (was_found) {
    found_ = true;
+    iterator_hint_ = node->hint() == IteratorType::kNormal
+                         ? IteratorHint::kNormal
+                         : IteratorHint::kAsync;
  }
-  Find(node->iterable(), true);
-  Print(node->hint() == IteratorType::kNormal ? "[Symbol.iterator]"
-                                              : "[Symbol.asyncIterator]");
+  Find(node->iterable_for_call_printer(), true);
  if (was_found) done_ = true;
 }


--- a/src/ast/prettyprinter.h
+++ b/src/ast/prettyprinter.h
@@ -20,6 +20,8 @@ class CallPrinter final : public AstVisitor<CallPrinter> {
  // The following routine prints the node with position |position| into a
  // string.
  Handle<String> Print(FunctionLiteral* program, int position);
+  enum IteratorHint { kNone, kNormal, kAsync };
+  IteratorHint GetIteratorHint() const;

 // Individual nodes
 #define DECLARE_VISIT(type) void Visit##type(type* node);
@@ -39,6 +41,7 @@ class CallPrinter final : public AstVisitor<CallPrinter> {
  bool found_;
  bool done_;
  bool is_user_js_;
+  IteratorHint iterator_hint_;

  DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();


--- a/src/messages.h
+++ b/src/messages.h
@@ -345,6 +345,7 @@ class ErrorUtils : public AllStatic {
  T(NotDateObject, "this is not a Date object.")                               \
  T(NotGeneric, "% requires that 'this' be a %")                               \
  T(NotIterable, "% is not iterable")                                          \
+  T(NotAsyncIterable, "% is not async iterable")                               \
  T(NotPropertyName, "% is not a valid property name")                         \
  T(NotTypedArray, "this is not a typed array.")                               \
  T(NotSuperConstructor, "Super constructor % of % is not a constructor")      \

--- a/src/parsing/pattern-rewriter.cc
+++ b/src/parsing/pattern-rewriter.cc
@@ -399,7 +399,7 @@ void Parser::PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,

  auto temp = *temp_var = CreateTempVar(current_value_);
  auto iterator = CreateTempVar(factory()->NewGetIterator(
-      factory()->NewVariableProxy(temp), IteratorType::kNormal,
+      factory()->NewVariableProxy(temp), current_value_, IteratorType::kNormal,
      current_value_->position()));
  auto done =
      CreateTempVar(factory()->NewBooleanLiteral(false, kNoSourcePosition));

--- a/src/runtime/runtime-internal.cc
+++ b/src/runtime/runtime-internal.cc
@@ -392,13 +392,20 @@ bool ComputeLocation(Isolate* isolate, MessageLocation* target) {
  return false;
 }

-Handle<String> RenderCallSite(Isolate* isolate, Handle<Object> object) {
+Handle<String> RenderCallSite(Isolate* isolate, Handle<Object> object,
+                              MessageTemplate::Template* id) {
  MessageLocation location;
  if (ComputeLocation(isolate, &location)) {
    ParseInfo info(location.shared());
    if (parsing::ParseAny(&info, isolate)) {
      CallPrinter printer(isolate, location.shared()->IsUserJavaScript());
      Handle<String> str = printer.Print(info.literal(), location.start_pos());
+      CallPrinter::IteratorHint type = printer.GetIteratorHint();
+      if (type == CallPrinter::IteratorHint::kNormal) {
+        *id = MessageTemplate::kNotIterable;
+      } else if (type == CallPrinter::IteratorHint::kAsync) {
+        *id = MessageTemplate::kNotAsyncIterable;
+      }
      if (str->length() > 0) return str;
    } else {
      isolate->clear_pending_exception();
@@ -413,9 +420,9 @@ RUNTIME_FUNCTION(Runtime_ThrowCalledNonCallable) {
  HandleScope scope(isolate);
  DCHECK_EQ(1, args.length());
  CONVERT_ARG_HANDLE_CHECKED(Object, object, 0);
-  Handle<String> callsite = RenderCallSite(isolate, object);
-  THROW_NEW_ERROR_RETURN_FAILURE(
-      isolate, NewTypeError(MessageTemplate::kCalledNonCallable, callsite));
+  MessageTemplate::Template id = MessageTemplate::kCalledNonCallable;
+  Handle<String> callsite = RenderCallSite(isolate, object, &id);
+  THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewTypeError(id, callsite));
 }

 RUNTIME_FUNCTION(Runtime_ThrowCalledOnNullOrUndefined) {
@@ -430,9 +437,9 @@ RUNTIME_FUNCTION(Runtime_ThrowConstructedNonConstructable) {
  HandleScope scope(isolate);
  DCHECK_EQ(1, args.length());
  CONVERT_ARG_HANDLE_CHECKED(Object, object, 0);
-  Handle<String> callsite = RenderCallSite(isolate, object);
-  THROW_NEW_ERROR_RETURN_FAILURE(
-      isolate, NewTypeError(MessageTemplate::kNotConstructor, callsite));
+  MessageTemplate::Template id = MessageTemplate::kNotConstructor;
+  Handle<String> callsite = RenderCallSite(isolate, object, &id);
+  THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewTypeError(id, callsite));
 }

 RUNTIME_FUNCTION(Runtime_ThrowConstructorReturnedNonObject) {

--- a/test/cctest/interpreter/bytecode_expectations/AsyncGenerators.golden
+++ b/test/cctest/interpreter/bytecode_expectations/AsyncGenerators.golden
@@ -427,7 +427,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(19),
                B(LdaConstant), U8(15),
                B(Star), R(20),
@@ -742,7 +742,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(17),
                B(LdaConstant), U8(25),
                B(Star), R(18),
@@ -818,7 +818,7 @@ bytecodes: [
                B(JumpIfToBooleanFalse), U8(4),
                B(Jump), U8(7),
                B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(8), U8(1),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                B(Star), R(17),
                B(LdaConstant), U8(25),
                B(Star), R(18),

--- a/test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden
+++ b/test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden
@@ -156,7 +156,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(20),
                B(LdaConstant), U8(15),
                B(Star), R(21),
@@ -488,7 +488,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(20),
                B(LdaConstant), U8(15),
                B(Star), R(21),
@@ -843,7 +843,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(20),
                B(LdaConstant), U8(15),
                B(Star), R(21),
@@ -1123,7 +1123,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(16),
                B(LdaConstant), U8(10),
                B(Star), R(17),

--- a/test/cctest/interpreter/bytecode_expectations/ForOf.golden
+++ b/test/cctest/interpreter/bytecode_expectations/ForOf.golden
@@ -85,7 +85,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(11),
                B(LdaConstant), U8(8),
                B(Star), R(12),
@@ -225,7 +225,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(12),
                B(LdaConstant), U8(8),
                B(Star), R(13),
@@ -377,7 +377,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(11),
                B(LdaConstant), U8(8),
                B(Star), R(12),
@@ -520,7 +520,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(10),
                B(LdaConstant), U8(10),
                B(Star), R(11),

--- a/test/cctest/interpreter/bytecode_expectations/ForOfLoop.golden
+++ b/test/cctest/interpreter/bytecode_expectations/ForOfLoop.golden
@@ -89,7 +89,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(13),
                B(LdaConstant), U8(7),
                B(Star), R(14),
@@ -266,7 +266,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(12),
                B(LdaConstant), U8(11),
                B(Star), R(13),
@@ -419,7 +419,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(11),
                B(LdaConstant), U8(9),
                B(Star), R(12),
@@ -576,7 +576,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(16),
                B(LdaConstant), U8(9),
                B(Star), R(17),
@@ -756,7 +756,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(17),
                B(LdaConstant), U8(10),
                B(Star), R(18),
@@ -965,7 +965,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(16),
                B(LdaConstant), U8(14),
                B(Star), R(17),
@@ -1129,7 +1129,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(20),
                B(LdaConstant), U8(7),
                B(Star), R(21),
@@ -1390,7 +1390,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(21),
                B(LdaConstant), U8(11),
                B(Star), R(22),

--- a/test/cctest/interpreter/bytecode_expectations/Generators.golden
+++ b/test/cctest/interpreter/bytecode_expectations/Generators.golden
@@ -283,7 +283,7 @@ bytecodes: [
                B(TestTypeOf), U8(5),
                B(JumpIfFalse), U8(4),
                B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(131),
+                B(Wide), B(LdaSmi), I16(132),
                B(Star), R(15),
                B(LdaConstant), U8(15),
                B(Star), R(16),

--- a/test/message/destructuring-array-non-iterable-number.out
+++ b/test/message/destructuring-array-non-iterable-number.out
-*%(basename)s:5: TypeError: [Symbol.iterator] is not a function
+*%(basename)s:5: TypeError: 1 is not iterable
 var [a] = 1;
          ^
-TypeError: [Symbol.iterator] is not a function
+TypeError: 1 is not iterable
    at *%(basename)s:5:11
--- a/test/message/destructuring-array-non-iterable-object-literal-complex.js
+++ b/test/message/destructuring-array-non-iterable-object-literal-complex.js
+// Copyright 2017 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.
+
+var [a] = {['a']: {b: 1}};
--- a/test/message/destructuring-array-non-iterable-object-literal-complex.out
+++ b/test/message/destructuring-array-non-iterable-object-literal-complex.out
+*%(basename)s:5: TypeError: {(intermediate value)} is not iterable
+var [a] = {['a']: {b: 1}};
+          ^
+TypeError: {(intermediate value)} is not iterable
+    at *%(basename)s:5:11
--- a/test/message/destructuring-array-non-iterable-object-literal.js
+++ b/test/message/destructuring-array-non-iterable-object-literal.js
+// Copyright 2017 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.
+
+var [a] = {a: 1};
--- a/test/message/destructuring-array-non-iterable-object-literal.out
+++ b/test/message/destructuring-array-non-iterable-object-literal.out
+*%(basename)s:5: TypeError: {(intermediate value)} is not iterable
+var [a] = {a: 1};
+          ^
+TypeError: {(intermediate value)} is not iterable
+    at *%(basename)s:5:11
--- a/test/message/destructuring-array-non-iterable-object.out
+++ b/test/message/destructuring-array-non-iterable-object.out
-*%(basename)s:6: TypeError: [Symbol.iterator] is not a function
+*%(basename)s:6: TypeError: x is not iterable
 var [a] = x;
          ^
-TypeError: [Symbol.iterator] is not a function
+TypeError: x is not iterable
    at *%(basename)s:6:11
--- a/test/message/for-of-non-iterable.js
+++ b/test/message/for-of-non-iterable.js
+// Copyright 2017 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.
+
+for ([a] of {}) {}
--- a/test/message/for-of-non-iterable.out
+++ b/test/message/for-of-non-iterable.out
+*%(basename)s:5: TypeError: {} is not iterable
+for ([a] of {}) {}
+            ^
+TypeError: {} is not iterable
+    at *%(basename)s:5:13
--- a/test/message/get-iterator1.out
+++ b/test/message/get-iterator1.out
-*%(basename)s:7: TypeError: nonIterable(...)[Symbol.iterator] is not a function
+*%(basename)s:7: TypeError: nonIterable(...) is not iterable
 [...nonIterable()];
    ^
-TypeError: nonIterable(...)[Symbol.iterator] is not a function
+TypeError: nonIterable(...) is not iterable
    at *%(basename)s:7:5
--- a/test/mjsunit/harmony/async-for-of-non-iterable.js
+++ b/test/mjsunit/harmony/async-for-of-non-iterable.js
+// Copyright 2017 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.
+
+// Flags: --harmony-async-iteration
+var done = false;
+
+async function f() {
+    try {
+        for await ([a] of {}) {
+            UNREACHABLE();
+        }
+        UNREACHABLE();
+    } catch (e) {
+        assertEquals(e.message, "{} is not async iterable");
+        done = true;
+    }
+}
+
+f();
+assertTrue(done);
--- a/test/webkit/exception-for-nonobject-expected.txt
+++ b/test/webkit/exception-for-nonobject-expected.txt
@@ -26,7 +26,7 @@ Test for correct handling of exceptions from instanceof and 'new' expressions
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


-PASS new {}.undefined threw exception TypeError: (intermediate value).undefined is not a constructor.
+PASS new {}.undefined threw exception TypeError: {}.undefined is not a constructor.
 PASS 1 instanceof {}.undefined threw exception TypeError: Right-hand side of 'instanceof' is not an object.
 PASS successfullyParsed is true