Add a pretty printer to improve the error message non-function calls

BUG=259443
LOG=y

Review URL: https://codereview.chromium.org/861623002

Cr-Commit-Position: refs/heads/master@{#26189}

src/compiler.h

@@ -93,6 +93,7 @@ class CompilationInfo {
   };
 
   CompilationInfo(Handle<JSFunction> closure, Zone* zone);
+  CompilationInfo(Handle<Script> script, Zone* zone);
   CompilationInfo(Isolate* isolate, Zone* zone);
   virtual ~CompilationInfo();
 
@@ -398,8 +399,6 @@ class CompilationInfo {
   }
 
  protected:
-  CompilationInfo(Handle<Script> script,
-                  Zone* zone);
   CompilationInfo(Handle<SharedFunctionInfo> shared_info,
                   Zone* zone);
   CompilationInfo(HydrogenCodeStub* stub,

src/isolate.cc

@@ -1063,7 +1063,7 @@ void Isolate::ComputeLocation(MessageLocation* target) {
       int pos = frame->LookupCode()->SourcePosition(frame->pc());
       // Compute the location from the function and the reloc info.
       Handle<Script> casted_script(Script::cast(script));
-      *target = MessageLocation(casted_script, pos, pos + 1);
+      *target = MessageLocation(casted_script, pos, pos + 1, handle(fun));
     }
   }
 }

src/messages.h

@@ -43,22 +43,24 @@ class SourceInfo;
 
 class MessageLocation {
  public:
-  MessageLocation(Handle<Script> script,
-                  int start_pos,
-                  int end_pos)
+  MessageLocation(Handle<Script> script, int start_pos, int end_pos,
+                  Handle<JSFunction> function = Handle<JSFunction>())
       : script_(script),
         start_pos_(start_pos),
-        end_pos_(end_pos) { }
+        end_pos_(end_pos),
+        function_(function) {}
   MessageLocation() : start_pos_(-1), end_pos_(-1) { }
 
   Handle<Script> script() const { return script_; }
   int start_pos() const { return start_pos_; }
   int end_pos() const { return end_pos_; }
+  Handle<JSFunction> function() const { return function_; }
 
  private:
   Handle<Script> script_;
   int start_pos_;
   int end_pos_;
+  Handle<JSFunction> function_;
 };
 
 

src/prettyprinter.cc

@@ -14,6 +14,426 @@
 namespace v8 {
 namespace internal {
 
+CallPrinter::CallPrinter(Zone* zone) {
+  output_ = NULL;
+  size_ = 0;
+  pos_ = 0;
+  position_ = 0;
+  found_ = false;
+  done_ = false;
+  InitializeAstVisitor(zone);
+}
+
+
+CallPrinter::~CallPrinter() { DeleteArray(output_); }
+
+
+const char* CallPrinter::Print(FunctionLiteral* program, int position) {
+  Init();
+  position_ = position;
+  Find(program);
+  return output_;
+}
+
+
+void CallPrinter::Find(AstNode* node, bool print) {
+  if (done_) return;
+  if (found_) {
+    if (print) {
+      int start = pos_;
+      Visit(node);
+      if (start != pos_) return;
+    }
+    Print("(intermediate value)");
+  } else {
+    Visit(node);
+  }
+}
+
+
+void CallPrinter::Init() {
+  if (size_ == 0) {
+    DCHECK(output_ == NULL);
+    const int initial_size = 256;
+    output_ = NewArray<char>(initial_size);
+    size_ = initial_size;
+  }
+  output_[0] = '\0';
+  pos_ = 0;
+}
+
+
+void CallPrinter::Print(const char* format, ...) {
+  if (!found_ || done_) return;
+  for (;;) {
+    va_list arguments;
+    va_start(arguments, format);
+    int n = VSNPrintF(Vector<char>(output_, size_) + pos_, format, arguments);
+    va_end(arguments);
+
+    if (n >= 0) {
+      // there was enough space - we are done
+      pos_ += n;
+      return;
+    } else {
+      // there was not enough space - allocate more and try again
+      const int slack = 32;
+      int new_size = size_ + (size_ >> 1) + slack;
+      char* new_output = NewArray<char>(new_size);
+      MemCopy(new_output, output_, pos_);
+      DeleteArray(output_);
+      output_ = new_output;
+      size_ = new_size;
+    }
+  }
+}
+
+
+void CallPrinter::VisitBlock(Block* node) {
+  FindStatements(node->statements());
+}
+
+
+void CallPrinter::VisitVariableDeclaration(VariableDeclaration* node) {}
+
+
+void CallPrinter::VisitFunctionDeclaration(FunctionDeclaration* node) {}
+
+
+void CallPrinter::VisitModuleDeclaration(ModuleDeclaration* node) {
+  Find(node->module());
+}
+
+
+void CallPrinter::VisitImportDeclaration(ImportDeclaration* node) {
+  Find(node->module());
+}
+
+
+void CallPrinter::VisitExportDeclaration(ExportDeclaration* node) {}
+
+
+void CallPrinter::VisitModuleLiteral(ModuleLiteral* node) {
+  VisitBlock(node->body());
+}
+
+
+void CallPrinter::VisitModuleVariable(ModuleVariable* node) {
+  Find(node->proxy());
+}
+
+
+void CallPrinter::VisitModulePath(ModulePath* node) { Find(node->module()); }
+
+
+void CallPrinter::VisitModuleUrl(ModuleUrl* node) {}
+
+
+void CallPrinter::VisitModuleStatement(ModuleStatement* node) {
+  Find(node->body());
+}
+
+
+void CallPrinter::VisitExpressionStatement(ExpressionStatement* node) {
+  Find(node->expression());
+}
+
+
+void CallPrinter::VisitEmptyStatement(EmptyStatement* node) {}
+
+
+void CallPrinter::VisitIfStatement(IfStatement* node) {
+  Find(node->condition());
+  Find(node->then_statement());
+  if (node->HasElseStatement()) {
+    Find(node->else_statement());
+  }
+}
+
+
+void CallPrinter::VisitContinueStatement(ContinueStatement* node) {}
+
+
+void CallPrinter::VisitBreakStatement(BreakStatement* node) {}
+
+
+void CallPrinter::VisitReturnStatement(ReturnStatement* node) {
+  Find(node->expression());
+}
+
+
+void CallPrinter::VisitWithStatement(WithStatement* node) {
+  Find(node->expression());
+  Find(node->statement());
+}
+
+
+void CallPrinter::VisitSwitchStatement(SwitchStatement* node) {
+  Find(node->tag());
+  ZoneList<CaseClause*>* cases = node->cases();
+  for (int i = 0; i < cases->length(); i++) Find(cases->at(i));
+}
+
+
+void CallPrinter::VisitCaseClause(CaseClause* clause) {
+  if (!clause->is_default()) {
+    Find(clause->label());
+  }
+  FindStatements(clause->statements());
+}
+
+
+void CallPrinter::VisitDoWhileStatement(DoWhileStatement* node) {
+  Find(node->body());
+  Find(node->cond());
+}
+
+
+void CallPrinter::VisitWhileStatement(WhileStatement* node) {
+  Find(node->cond());
+  Find(node->body());
+}
+
+
+void CallPrinter::VisitForStatement(ForStatement* node) {
+  if (node->init() != NULL) {
+    Find(node->init());
+  }
+  if (node->cond() != NULL) Find(node->cond());
+  if (node->next() != NULL) Find(node->next());
+  Find(node->body());
+}
+
+
+void CallPrinter::VisitForInStatement(ForInStatement* node) {
+  Find(node->each());
+  Find(node->enumerable());
+  Find(node->body());
+}
+
+
+void CallPrinter::VisitForOfStatement(ForOfStatement* node) {
+  Find(node->each());
+  Find(node->iterable());
+  Find(node->body());
+}
+
+
+void CallPrinter::VisitTryCatchStatement(TryCatchStatement* node) {
+  Find(node->try_block());
+  Find(node->catch_block());
+}
+
+
+void CallPrinter::VisitTryFinallyStatement(TryFinallyStatement* node) {
+  Find(node->try_block());
+  Find(node->finally_block());
+}
+
+
+void CallPrinter::VisitDebuggerStatement(DebuggerStatement* node) {}
+
+
+void CallPrinter::VisitFunctionLiteral(FunctionLiteral* node) {
+  FindStatements(node->body());
+}
+
+
+void CallPrinter::VisitClassLiteral(ClassLiteral* node) {
+  if (node->extends()) Find(node->extends());
+  for (int i = 0; i < node->properties()->length(); i++) {
+    Find(node->properties()->at(i)->value());
+  }
+}
+
+
+void CallPrinter::VisitNativeFunctionLiteral(NativeFunctionLiteral* node) {}
+
+
+void CallPrinter::VisitConditional(Conditional* node) {
+  Find(node->condition());
+  Find(node->then_expression());
+  Find(node->else_expression());
+}
+
+
+void CallPrinter::VisitLiteral(Literal* node) {
+  PrintLiteral(node->value(), true);
+}
+
+
+void CallPrinter::VisitRegExpLiteral(RegExpLiteral* node) {
+  Print("/");
+  PrintLiteral(node->pattern(), false);
+  Print("/");
+  PrintLiteral(node->flags(), false);
+}
+
+
+void CallPrinter::VisitObjectLiteral(ObjectLiteral* node) {
+  for (int i = 0; i < node->properties()->length(); i++) {
+    Find(node->properties()->at(i)->value());
+  }
+}
+
+
+void CallPrinter::VisitArrayLiteral(ArrayLiteral* node) {
+  Print("[");
+  for (int i = 0; i < node->values()->length(); i++) {
+    if (i != 0) Print(",");
+    Find(node->values()->at(i), true);
+  }
+  Print("]");
+}
+
+
+void CallPrinter::VisitVariableProxy(VariableProxy* node) {
+  PrintLiteral(node->name(), false);
+}
+
+
+void CallPrinter::VisitAssignment(Assignment* node) {
+  Find(node->target());
+  Find(node->value());
+}
+
+
+void CallPrinter::VisitYield(Yield* node) { Find(node->expression()); }
+
+
+void CallPrinter::VisitThrow(Throw* node) { Find(node->exception()); }
+
+
+void CallPrinter::VisitProperty(Property* node) {
+  Expression* key = node->key();
+  Literal* literal = key->AsLiteral();
+  if (literal != NULL && literal->value()->IsInternalizedString()) {
+    Find(node->obj(), true);
+    Print(".");
+    PrintLiteral(literal->value(), false);
+  } else {
+    Find(node->obj(), true);
+    Print("[");
+    Find(key, true);
+    Print("]");
+  }
+}
+
+
+void CallPrinter::VisitCall(Call* node) {
+  bool was_found = !found_ && node->position() == position_;
+  if (was_found) found_ = true;
+  Find(node->expression(), true);
+  if (!was_found) Print("(...)");
+  FindArguments(node->arguments());
+  if (was_found) done_ = true;
+}
+
+
+void CallPrinter::VisitCallNew(CallNew* node) {
+  bool was_found = !found_ && node->expression()->position() == position_;
+  if (was_found) found_ = true;
+  Find(node->expression(), was_found);
+  FindArguments(node->arguments());
+  if (was_found) done_ = true;
+}
+
+
+void CallPrinter::VisitCallRuntime(CallRuntime* node) {
+  FindArguments(node->arguments());
+}
+
+
+void CallPrinter::VisitUnaryOperation(UnaryOperation* node) {
+  Token::Value op = node->op();
+  bool needsSpace =
+      op == Token::DELETE || op == Token::TYPEOF || op == Token::VOID;
+  Print("(%s%s", Token::String(op), needsSpace ? " " : "");
+  Find(node->expression(), true);
+  Print(")");
+}
+
+
+void CallPrinter::VisitCountOperation(CountOperation* node) {
+  Print("(");
+  if (node->is_prefix()) Print("%s", Token::String(node->op()));
+  Find(node->expression(), true);
+  if (node->is_postfix()) Print("%s", Token::String(node->op()));
+  Print(")");
+}
+
+
+void CallPrinter::VisitBinaryOperation(BinaryOperation* node) {
+  Print("(");
+  Find(node->left(), true);
+  Print(" %s ", Token::String(node->op()));
+  Find(node->right(), true);
+  Print(")");
+}
+
+
+void CallPrinter::VisitCompareOperation(CompareOperation* node) {
+  Print("(");
+  Find(node->left(), true);
+  Print(" %s ", Token::String(node->op()));
+  Find(node->right(), true);
+  Print(")");
+}
+
+
+void CallPrinter::VisitThisFunction(ThisFunction* node) {}
+
+
+void CallPrinter::VisitSuperReference(SuperReference* node) {}
+
+
+void CallPrinter::FindStatements(ZoneList<Statement*>* statements) {
+  if (statements == NULL) return;
+  for (int i = 0; i < statements->length(); i++) {
+    Find(statements->at(i));
+  }
+}
+
+
+void CallPrinter::FindArguments(ZoneList<Expression*>* arguments) {
+  if (found_) return;
+  for (int i = 0; i < arguments->length(); i++) {
+    Find(arguments->at(i));
+  }
+}
+
+
+void CallPrinter::PrintLiteral(Handle<Object> value, bool quote) {
+  Object* object = *value;
+  if (object->IsString()) {
+    String* string = String::cast(object);
+    if (quote) Print("\"");
+    for (int i = 0; i < string->length(); i++) {
+      Print("%c", string->Get(i));
+    }
+    if (quote) Print("\"");
+  } else if (object->IsNull()) {
+    Print("null");
+  } else if (object->IsTrue()) {
+    Print("true");
+  } else if (object->IsFalse()) {
+    Print("false");
+  } else if (object->IsUndefined()) {
+    Print("undefined");
+  } else if (object->IsNumber()) {
+    Print("%g", object->Number());
+  }
+}
+
+
+void CallPrinter::PrintLiteral(const AstRawString* value, bool quote) {
+  PrintLiteral(value->string(), quote);
+}
+
+
+//-----------------------------------------------------------------------------
+
+
 #ifdef DEBUG
 
 PrettyPrinter::PrettyPrinter(Zone* zone) {

src/prettyprinter.h

@@ -11,6 +11,43 @@
 namespace v8 {
 namespace internal {
 
+class CallPrinter : public AstVisitor {
+ public:
+  explicit CallPrinter(Zone* zone);
+  virtual ~CallPrinter();
+
+  // The following routine prints the node with position |position| into a
+  // string. The result string is alive as long as the CallPrinter is alive.
+  const char* Print(FunctionLiteral* program, int position);
+
+  void Print(const char* format, ...);
+
+  void Find(AstNode* node, bool print = false);
+
+// Individual nodes
+#define DECLARE_VISIT(type) void Visit##type(type* node) OVERRIDE;
+  AST_NODE_LIST(DECLARE_VISIT)
+#undef DECLARE_VISIT
+
+ private:
+  void Init();
+  char* output_;  // output string buffer
+  int size_;      // output_ size
+  int pos_;       // current printing position
+  int position_;  // position of ast node to print
+  bool found_;
+  bool done_;
+
+  DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
+
+ protected:
+  void PrintLiteral(Handle<Object> value, bool quote);
+  void PrintLiteral(const AstRawString* value, bool quote);
+  void FindStatements(ZoneList<Statement*>* statements);
+  void FindArguments(ZoneList<Expression*>* arguments);
+};
+
+
 #ifdef DEBUG
 
 class PrettyPrinter: public AstVisitor {

src/runtime.js

@@ -377,7 +377,9 @@ function FILTER_KEY(key) {
 function CALL_NON_FUNCTION() {
   var delegate = %GetFunctionDelegate(this);
   if (!IS_FUNCTION(delegate)) {
-    throw %MakeTypeError('called_non_callable', [typeof this]);
+    var callsite = %RenderCallSite();
+    if (callsite == "") callsite = typeof this;
+    throw %MakeTypeError('called_non_callable', [callsite]);
   }
   return %Apply(delegate, this, arguments, 0, %_ArgumentsLength());
 }
@@ -386,7 +388,9 @@ function CALL_NON_FUNCTION() {
 function CALL_NON_FUNCTION_AS_CONSTRUCTOR() {
   var delegate = %GetConstructorDelegate(this);
   if (!IS_FUNCTION(delegate)) {
-    throw %MakeTypeError('called_non_callable', [typeof this]);
+    var callsite = %RenderCallSite();
+    if (callsite == "") callsite = typeof this;
+    throw %MakeTypeError('called_non_callable', [callsite]);
   }
   return %Apply(delegate, this, arguments, 0, %_ArgumentsLength());
 }

src/runtime/runtime-internal.cc

@@ -7,6 +7,9 @@
 #include "src/arguments.h"
 #include "src/bootstrapper.h"
 #include "src/debug.h"
+#include "src/messages.h"
+#include "src/parser.h"
+#include "src/prettyprinter.h"
 #include "src/runtime/runtime-utils.h"
 
 namespace v8 {
@@ -153,6 +156,30 @@ RUNTIME_FUNCTION(Runtime_CollectStackTrace) {
 }
 
 
+RUNTIME_FUNCTION(Runtime_RenderCallSite) {
+  HandleScope scope(isolate);
+  DCHECK(args.length() == 0);
+  MessageLocation location;
+  isolate->ComputeLocation(&location);
+  if (location.start_pos() == -1) return isolate->heap()->empty_string();
+
+  Zone zone(isolate);
+  if (location.function()->shared()->is_function()) {
+    CompilationInfo info(location.function(), &zone);
+    if (!Parser::Parse(&info)) return isolate->heap()->empty_string();
+    CallPrinter printer(&zone);
+    const char* string = printer.Print(info.function(), location.start_pos());
+    return *isolate->factory()->NewStringFromAsciiChecked(string);
+  }
+
+  CompilationInfo info(location.script(), &zone);
+  if (!Parser::Parse(&info)) return isolate->heap()->empty_string();
+  CallPrinter printer(&zone);
+  const char* string = printer.Print(info.function(), location.start_pos());
+  return *isolate->factory()->NewStringFromAsciiChecked(string);
+}
+
+
 RUNTIME_FUNCTION(Runtime_GetFromCache) {
   SealHandleScope shs(isolate);
   // This is only called from codegen, so checks might be more lax.

src/runtime/runtime.h

@@ -383,6 +383,7 @@ namespace internal {
   F(Abort, 1, 1)                                       \
   F(AbortJS, 1, 1)                                     \
   F(NativeScriptsCount, 0, 1)                          \
+  F(RenderCallSite, 0, 1)                              \
   /* ES5 */                                            \
   F(OwnKeys, 1, 1)                                     \
                                                        \

test/cctest/test-api.cc

@@ -11353,7 +11353,7 @@ THREADED_TEST(ConstructorForObject) {
     value = CompileRun("new obj2(28)");
     CHECK(try_catch.HasCaught());
     String::Utf8Value exception_value1(try_catch.Exception());
-    CHECK_EQ("TypeError: object is not a function", *exception_value1);
+    CHECK_EQ("TypeError: obj2 is not a function", *exception_value1);
     try_catch.Reset();
 
     Local<Value> args[] = { v8_num(29) };
@@ -11714,8 +11714,7 @@ THREADED_TEST(CallAsFunction) {
     CHECK(try_catch.HasCaught());
     String::Utf8Value exception_value1(try_catch.Exception());
     // TODO(verwaest): Better message
-    CHECK_EQ("TypeError: object is not a function",
-             *exception_value1);
+    CHECK_EQ("TypeError: obj2 is not a function", *exception_value1);
     try_catch.Reset();
 
     // Call an object without call-as-function handler through the API
@@ -13095,7 +13094,7 @@ THREADED_PROFILED_TEST(InterceptorCallICFastApi_SimpleSignature_Miss3) {
       "}");
   CHECK(try_catch.HasCaught());
   // TODO(verwaest): Adjust message.
-  CHECK_EQ(v8_str("TypeError: undefined is not a function"),
+  CHECK_EQ(v8_str("TypeError: receiver.method is not a function"),
            try_catch.Exception()->ToString(isolate));
   CHECK_EQ(42, context->Global()->Get(v8_str("saved_result"))->Int32Value());
   CHECK_GE(interceptor_call_count, 50);
@@ -13270,7 +13269,7 @@ THREADED_PROFILED_TEST(CallICFastApi_SimpleSignature_Miss2) {
       "}");
   CHECK(try_catch.HasCaught());
   // TODO(verwaest): Adjust message.
-  CHECK_EQ(v8_str("TypeError: undefined is not a function"),
+  CHECK_EQ(v8_str("TypeError: receiver.method is not a function"),
            try_catch.Exception()->ToString(isolate));
   CHECK_EQ(42, context->Global()->Get(v8_str("saved_result"))->Int32Value());
 }

test/mozilla/mozilla.status

@@ -389,7 +389,6 @@
   'js1_5/LexicalConventions/regress-469940': [FAIL_OK],
   'js1_5/Exceptions/regress-332472': [FAIL_OK],
   'js1_5/Regress/regress-173067': [FAIL_OK],
-  'js1_5/Regress/regress-355556': [FAIL_OK],
   'js1_5/Regress/regress-328664': [FAIL_OK],
   'js1_5/Regress/regress-252892': [FAIL_OK],
   'js1_5/Regress/regress-352208': [FAIL_OK],

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: undefined is not a function.
+PASS new {}.undefined threw exception TypeError: (intermediate value).undefined is not a function.
 PASS 1 instanceof {}.undefined threw exception TypeError: Expecting a function in instanceof check, but got undefined.
 PASS successfullyParsed is true
 

test/webkit/fast/js/toString-overrides-expected.txt

@@ -28,13 +28,12 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS [1].toString() is '1'
 PASS [1].toLocaleString() is 'toLocaleString'
-FAIL [1].toLocaleString() should be 1. Threw exception TypeError: string is not a function
+FAIL [1].toLocaleString() should be 1. Threw exception TypeError: [1].toLocaleString is not a function
 PASS [/r/].toString() is 'toString2'
 PASS [/r/].toLocaleString() is 'toLocaleString2'
-FAIL [/r/].toLocaleString() should be toString2. Threw exception TypeError: string is not a function
+FAIL [/r/].toLocaleString() should be toString2. Threw exception TypeError: [/r/].toLocaleString is not a function
 PASS caught is true
 PASS successfullyParsed is true
 
-
 TEST COMPLETE
 

test/webkit/run-json-stringify-expected.txt

@@ -83,7 +83,7 @@ PASS tests[i](nativeJSON) is tests[i](JSON)
 function (jsonObject){
         return jsonObject.stringify({toJSON: Date.prototype.toJSON});
     }
-PASS tests[i](nativeJSON) threw exception TypeError: undefined is not a function.
+PASS tests[i](nativeJSON) threw exception TypeError: jsonObject.stringify is not a function.
 function (jsonObject){
         return jsonObject.stringify({toJSON: Date.prototype.toJSON, toISOString: function(){ return "custom toISOString"; }});
     }
@@ -101,7 +101,7 @@ function (jsonObject){
         d.toISOString = null;
         return jsonObject.stringify(d);
     }
-PASS tests[i](nativeJSON) threw exception TypeError: object is not a function.
+PASS tests[i](nativeJSON) threw exception TypeError: jsonObject.stringify is not a function.
 function (jsonObject){
         var d = new Date(0);
         d.toJSON = undefined;