Move CallSite.toString to C++

src/builtins/builtins-callsite.cc

Index: src/builtins/builtins-callsite.cc
diff --git a/src/builtins/builtins-callsite.cc b/src/builtins/builtins-callsite.cc
index 89b846927da78cae54e16450c4eac7a25a9f3713..e91b0e784b80109a7e3d099cabb8aed9a5fe30ef 100644
--- a/src/builtins/builtins-callsite.cc
+++ b/src/builtins/builtins-callsite.cc
@@ -199,28 +199,35 @@ MaybeHandle<String> FormatEvalOrigin(Isolate* isolate, Handle<Script> script) {
   return result;
 }
 
-}  // namespace
-
-BUILTIN(CallSitePrototypeGetEvalOrigin) {
-  HandleScope scope(isolate);
-  CHECK_CALLSITE(recv, "getEvalOrigin");
-
-  CallSite call_site(isolate, recv);
-  if (call_site.IsWasm()) return *isolate->factory()->undefined_value();
+MaybeHandle<Object> GetEvalOrigin(Isolate* isolate, Handle<JSObject> object) {

[Yang, 2016/07/28 06:04:10]

I'm inclined to move these helper functions to the CallSite class, similiar to
CallSite::GetFileName.

[jgruber, 2016/07/28 09:21:50]

Yes that's a good point. I ran into the same thing while porting
CallSite.toString, and I started moving helpers to the C++ CallSite class there.

If you prefer, I could finish that work and backport it to this CL.

[Yang, 2016/07/28 09:55:00]

I'm fine with having that in a separate CL.

+  CallSite call_site(isolate, object);
+  if (call_site.IsWasm()) return isolate->factory()->undefined_value();
 
   // Retrieve the function's script object.
 
   Handle<Object> function_obj;
   Handle<Symbol> symbol = isolate->factory()->call_site_function_symbol();
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, function_obj,
-                                     JSObject::GetProperty(recv, symbol));
+  ASSIGN_RETURN_ON_EXCEPTION(isolate, function_obj,
+                             JSObject::GetProperty(object, symbol), Object);
 
   DCHECK(function_obj->IsJSFunction());
   Handle<JSFunction> function = Handle<JSFunction>::cast(function_obj);
   Handle<Object> script = handle(function->shared()->script(), isolate);
 
-  RETURN_RESULT_OR_FAILURE(
-      isolate, FormatEvalOrigin(isolate, Handle<Script>::cast(script)));
+  Handle<String> str;
+  ASSIGN_RETURN_ON_EXCEPTION(
+      isolate, str, FormatEvalOrigin(isolate, Handle<Script>::cast(script)),
+      String);
+
+  return str;
+}
+
+}  // namespace
+
+BUILTIN(CallSitePrototypeGetEvalOrigin) {
+  HandleScope scope(isolate);
+  CHECK_CALLSITE(recv, "getEvalOrigin");
+  RETURN_RESULT_OR_FAILURE(isolate, GetEvalOrigin(isolate, recv));
 }
 
 BUILTIN(CallSitePrototypeGetFileName) {
@@ -320,25 +327,32 @@ BUILTIN(CallSitePrototypeGetThis) {
   return *receiver;
 }
 
-BUILTIN(CallSitePrototypeGetTypeName) {
-  HandleScope scope(isolate);
-  CHECK_CALLSITE(recv, "getTypeName");
+namespace {
 
+MaybeHandle<Object> GetTypeName(Isolate* isolate, Handle<JSObject> object) {

[Yang, 2016/07/28 06:04:10]

Same here.

[jgruber, 2016/07/28 09:21:49]

Acknowledged.

   Handle<Object> receiver;
   Handle<Symbol> symbol = isolate->factory()->call_site_receiver_symbol();
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
-                                     JSObject::GetProperty(recv, symbol));
+  ASSIGN_RETURN_ON_EXCEPTION(isolate, receiver,
+                             JSObject::GetProperty(object, symbol), Object);
 
   // TODO(jgruber): Check for strict/constructor here as above.
 
   if (receiver->IsNull(isolate) || receiver->IsUndefined(isolate))
-    return *isolate->factory()->null_value();
+    return isolate->factory()->null_value();
 
-  if (receiver->IsJSProxy()) return *isolate->factory()->Proxy_string();
+  if (receiver->IsJSProxy()) return isolate->factory()->Proxy_string();
 
   Handle<JSReceiver> receiver_object =
       Object::ToObject(isolate, receiver).ToHandleChecked();
-  return *JSReceiver::GetConstructorName(receiver_object);
+  return JSReceiver::GetConstructorName(receiver_object);
+}
+
+}  // namespace
+
+BUILTIN(CallSitePrototypeGetTypeName) {
+  HandleScope scope(isolate);
+  CHECK_CALLSITE(recv, "getTypeName");
+  RETURN_RESULT_OR_FAILURE(isolate, GetTypeName(isolate, recv));
 }
 
 BUILTIN(CallSitePrototypeIsConstructor) {
@@ -377,10 +391,192 @@ BUILTIN(CallSitePrototypeIsToplevel) {
   return isolate->heap()->ToBoolean(call_site.IsToplevel());
 }
 
+namespace {
+
+MaybeHandle<JSObject> AppendWasmToString(Isolate* isolate,
+                                         Handle<JSObject> recv,
+                                         CallSite* call_site,
+                                         IncrementalStringBuilder* builder) {
+  Handle<Object> name = call_site->GetFunctionName();
+  if (name->IsNull(isolate)) {
+    builder->AppendCString("<WASM UNNAMED>");
+  } else {
+    DCHECK(name->IsString());
+    builder->AppendString(Handle<String>::cast(name));
+  }
+
+  builder->AppendCString(" (<WASM>[");
+
+  Handle<String> ix = isolate->factory()->NumberToString(
+      handle(Smi::FromInt(call_site->wasm_func_index()), isolate));
+  builder->AppendString(ix);
+
+  builder->AppendCString("]+");
+
+  Handle<Object> pos;
+  ASSIGN_RETURN_ON_EXCEPTION(
+      isolate, pos, JSObject::GetProperty(
+                        recv, isolate->factory()->call_site_position_symbol()),
+      JSObject);
+  DCHECK(pos->IsNumber());
+  builder->AppendString(isolate->factory()->NumberToString(pos));
+  builder->AppendCString(")");
+
+  return recv;
+}
+
+MaybeHandle<JSObject> AppendFileLocation(Isolate* isolate,
+                                         Handle<JSObject> recv,
+                                         CallSite* call_site,
+                                         IncrementalStringBuilder* builder) {
+  if (call_site->IsNative()) {
+    builder->AppendCString("native");
+    return recv;
+  }
+
+  Handle<Object> file_name = call_site->GetScriptNameOrSourceUrl();
+  if (!file_name->IsString() && call_site->IsEval()) {
+    Handle<Object> eval_origin;
+    ASSIGN_RETURN_ON_EXCEPTION(isolate, eval_origin,
+                               GetEvalOrigin(isolate, recv), JSObject);
+    DCHECK(eval_origin->IsString());
+    builder->AppendString(Handle<String>::cast(eval_origin));
+    builder->AppendCString(", ");  // Expecting source position to follow.
+  }
+
+  if (file_name->IsString()) {
+    builder->AppendString(Handle<String>::cast(file_name));
+  } else {
+    // Source code does not originate from a file and is not native, but we
+    // can still get the source position inside the source string, e.g. in
+    // an eval string.
+    builder->AppendCString("<anonymous>");
+  }
+
+  int line_number = call_site->GetLineNumber();
+  if (line_number != -1) {
+    builder->AppendCharacter(':');
+    Handle<String> line_string = isolate->factory()->NumberToString(
+        handle(Smi::FromInt(line_number), isolate), isolate);
+    builder->AppendString(line_string);
+
+    int column_number = call_site->GetColumnNumber();
+    if (column_number != -1) {
+      builder->AppendCharacter(':');
+      Handle<String> column_string = isolate->factory()->NumberToString(
+          handle(Smi::FromInt(column_number), isolate), isolate);
+      builder->AppendString(column_string);
+    }
+  }
+
+  return recv;
+}
+
+int StringIndexOf(Isolate* isolate, Handle<String> subject,
+                  Handle<String> pattern) {
+  if (pattern->length() > subject->length()) return -1;
+  return String::IndexOf(isolate, subject, pattern, 0);
+}
+
+MaybeHandle<JSObject> AppendMethodCall(Isolate* isolate, Handle<JSObject> recv,
+                                       CallSite* call_site,
+                                       IncrementalStringBuilder* builder) {
+  Handle<Object> type_name;
+  ASSIGN_RETURN_ON_EXCEPTION(isolate, type_name, GetTypeName(isolate, recv),
+                             JSObject);
+  Handle<Object> method_name = call_site->GetMethodName();
+  Handle<Object> function_name = call_site->GetFunctionName();
+
+  if (function_name->IsString()) {
+    Handle<String> function_string = Handle<String>::cast(function_name);
+    if (type_name->IsString()) {
+      Handle<String> type_string = Handle<String>::cast(type_name);
+      bool starts_with_type_name =
+          (StringIndexOf(isolate, function_string, type_string) == 0);
+      if (!starts_with_type_name) {
+        builder->AppendString(type_string);
+        builder->AppendCharacter('.');
+      }
+    }
+    builder->AppendString(function_string);
+
+    if (method_name->IsString()) {
+      Handle<String> method_string = Handle<String>::cast(method_name);
+      Handle<String> dot_method_string;
+      ASSIGN_RETURN_ON_EXCEPTION(
+          isolate, dot_method_string,
+          isolate->factory()->NewConsString(isolate->factory()->dot_string(),

[Yang, 2016/07/28 06:04:10]

This looks really inefficient to me. We want to know whether the function name
ends with the method name, with a preceding ".". So can't we just call
StringIndexOf with the function string and the method string, and check for
preceding "."? Even better would be just to flatten and compare the string
contents starting from end, using a FlatStringReader.

[jgruber, 2016/07/28 09:21:49]

Done. This also fixes a bug (already present in the original JS code) in which
ends_with_method_name is set when function_string == method_string, since
IndexOf returns -1 and fstr.len - mstr.len - 1 is also -1.

+                                            method_string),
+          JSObject);
+
+      bool ends_with_method_name =
+          (StringIndexOf(isolate, function_string, dot_method_string) ==
+           function_string->length() - dot_method_string->length());
+      if (!ends_with_method_name) {
+        builder->AppendCString(" [as ");
+        builder->AppendString(method_string);
+        builder->AppendCharacter(']');
+      }
+    }
+  } else {
+    builder->AppendString(Handle<String>::cast(type_name));
+    builder->AppendCharacter('.');
+    if (method_name->IsString()) {
+      builder->AppendString(Handle<String>::cast(method_name));
+    } else {
+      builder->AppendCString("<anonymous>");
+    }
+  }
+
+  return recv;
+}
+
+}  // namespace
+
 BUILTIN(CallSitePrototypeToString) {
   HandleScope scope(isolate);
-  // TODO(jgruber)
-  return *isolate->factory()->undefined_value();
+  CHECK_CALLSITE(recv, "toString");
+
+  IncrementalStringBuilder builder(isolate);
+
+  CallSite call_site(isolate, recv);
+  if (call_site.IsWasm()) {
+    RETURN_FAILURE_ON_EXCEPTION(
+        isolate, AppendWasmToString(isolate, recv, &call_site, &builder));
+    RETURN_RESULT_OR_FAILURE(isolate, builder.Finish());
+  }
+
+  DCHECK(!call_site.IsWasm());
+  Handle<Object> function_name = call_site.GetFunctionName();
+
+  const bool is_toplevel = call_site.IsToplevel();
+  const bool is_constructor = call_site.IsConstructor();
+  const bool is_method_call = !(is_toplevel || is_constructor);
+
+  if (is_method_call) {
+    RETURN_FAILURE_ON_EXCEPTION(
+        isolate, AppendMethodCall(isolate, recv, &call_site, &builder));
+  } else if (is_constructor) {
+    builder.AppendCString("new ");
+    if (function_name->IsString()) {
+      builder.AppendString(Handle<String>::cast(function_name));
+    } else {
+      builder.AppendCString("<anonymous>");
+    }
+  } else if (function_name->IsString()) {
+    builder.AppendString(Handle<String>::cast(function_name));
+  } else {
+    RETURN_FAILURE_ON_EXCEPTION(
+        isolate, AppendFileLocation(isolate, recv, &call_site, &builder));
+    RETURN_RESULT_OR_FAILURE(isolate, builder.Finish());
+  }
+
+  builder.AppendCString(" (");
+  RETURN_FAILURE_ON_EXCEPTION(
+      isolate, AppendFileLocation(isolate, recv, &call_site, &builder));
+  builder.AppendCString(")");
+
+  RETURN_RESULT_OR_FAILURE(isolate, builder.Finish());
 }
 
 #undef CHECK_CALLSITE