Move Error methods to C++

src/accessors.cc

Index: src/accessors.cc
diff --git a/src/accessors.cc b/src/accessors.cc
index 5c89c2552fe840173697654e26efc545552fdf43..6680cdb52c337c31ef3e039233146820a5196cd7 100644
--- a/src/accessors.cc
+++ b/src/accessors.cc
@@ -1116,6 +1116,131 @@ Handle<AccessorInfo> Accessors::BoundFunctionNameInfo(
                       attributes);
 }
 
+//
+// Accessors::ErrorStack
+//
+
+namespace {
+
+bool IsErrorObject(Isolate* isolate, Handle<JSObject> receiver) {
+  // Anything which has a stack_trace symbol property is considered as an error
+  // object. This is obviously the case for standard error objects such as
+  // Error and ReferenceError; but the user can also create error-like objects
+  // by calling Error.captureStackTrace.
+  Maybe<bool> ret = JSReceiver::HasOwnProperty(
+      receiver, isolate->factory()->stack_trace_symbol());
+  return ret.IsJust() && ret.FromJust();
+}
+
+MaybeHandle<JSReceiver> SetFormattedTrace(Isolate* isolate,
+                                          Handle<JSObject> error,
+                                          Handle<Object> value) {
+  RETURN_ON_EXCEPTION(
+      isolate,
+      JSReceiver::SetProperty(error, isolate->factory()->stack_trace_symbol(),
+                              isolate->factory()->undefined_value(), STRICT),
+      JSReceiver);
+  RETURN_ON_EXCEPTION(
+      isolate, JSReceiver::SetProperty(
+                   error, isolate->factory()->formatted_stack_trace_symbol(),

[Yang, 2016/07/15 13:10:11]

How about we simply reconfigure the "stack" property to a normal data property?
For the user there is no difference between an accessor property that's marked
as special data property, and a normal data property. So instead of caching it
in a hidden symbol, we could just overwrite the stack property with a normal
data property containing the lazily formatted stack trace.

[jgruber, 2016/07/18 13:06:11]

Good idea, done.

+                   value, STRICT),
+      JSReceiver);
+  return error;
+}
+
+MaybeHandle<Object> FormatStackTrace(Isolate* isolate, Handle<JSObject> error,
+                                     Handle<Object> stack_trace) {
+  // TODO(jgruber): Port FormatStackTrace from JS.
+  Handle<JSFunction> fun = isolate->error_format_stack_trace();
+
+  int argc = 2;
+  ScopedVector<Handle<Object>> argv(argc);
+  argv[0] = error;
+  argv[1] = stack_trace;
+
+  Handle<Object> formatted_stack_trace;
+  ASSIGN_RETURN_ON_EXCEPTION(
+      isolate, formatted_stack_trace,
+      Execution::Call(isolate, fun, error, argc, argv.start()), Object);
+
+  return formatted_stack_trace;
+}
+
+bool MaybeGetProperty(Isolate* isolate, Handle<JSObject> obj,

[Yang, 2016/07/15 13:10:11]

We can inline this once this only has one call site.

[jgruber, 2016/07/18 13:06:11]

Done.

+                      Handle<Symbol> symbol, Handle<Object>* out) {
+  MaybeHandle<Object> maybe = JSObject::GetProperty(obj, symbol);
+  if (!maybe.ToHandle(out)) return false;
+  return !(*out)->IsUndefined(isolate);
+}
+
+}  // namespace
+
+void Accessors::ErrorStackGetter(
+    v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
+  HandleScope scope(isolate);
+  Handle<Object> obj = Utils::OpenHandle(*info.Holder());
+
+  while (obj->IsJSObject()) {
+    Handle<JSObject> holder = Handle<JSObject>::cast(obj);
+    Handle<Object> formatted_stack_trace;
+    Handle<Symbol> fst_symbol =
+        isolate->factory()->formatted_stack_trace_symbol();
+    if (!MaybeGetProperty(isolate, holder, fst_symbol,
+                          &formatted_stack_trace)) {
+      // No formatted stack trace available.
+      Handle<Object> stack_trace;
+      Handle<Symbol> st_symbol = isolate->factory()->stack_trace_symbol();
+      if (!MaybeGetProperty(isolate, holder, st_symbol, &stack_trace)) {
+        // Neither formatted nor structured stack trace available.
+        // Look further up the prototype chain.
+        obj = JSReceiver::GetPrototype(isolate, holder).ToHandleChecked();
+        continue;
+      }
+
+      if (!FormatStackTrace(isolate, holder, stack_trace)
+               .ToHandle(&formatted_stack_trace)) {
+        isolate->OptionalRescheduleException(false);
+        return;
+      }
+
+      if (SetFormattedTrace(isolate, holder, formatted_stack_trace).is_null()) {
+        isolate->OptionalRescheduleException(false);
+        return;
+      }
+    }
+
+    info.GetReturnValue().Set(Utils::ToLocal(formatted_stack_trace));
+    return;
+  }
+
+  Handle<Object> result = isolate->factory()->undefined_value();
+  info.GetReturnValue().Set(Utils::ToLocal(result));
+}
+
+void Accessors::ErrorStackSetter(v8::Local<v8::Name> name,
+                                 v8::Local<v8::Value> val,
+                                 const v8::PropertyCallbackInfo<void>& info) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
+  HandleScope scope(isolate);
+  Handle<JSObject> obj =
+      Handle<JSObject>::cast(Utils::OpenHandle(*info.This()));
+
+  // Clear internal properties to avoid memory leaks.
+  if (IsErrorObject(isolate, obj)) {

[Yang, 2016/07/15 13:10:11]

I would just do this check inline (instead of defining what an Error object is).

[jgruber, 2016/07/18 13:06:11]

Done.

+    SetFormattedTrace(isolate, obj, isolate->factory()->undefined_value());
+  }
+
+  Accessors::ReconfigureToDataProperty(name, val, info);
+}
+
+Handle<AccessorInfo> Accessors::ErrorStackInfo(Isolate* isolate,
+                                               PropertyAttributes attributes) {
+  Handle<AccessorInfo> info =
+      MakeAccessor(isolate, isolate->factory()->stack_string(),
+                   &ErrorStackGetter, &ErrorStackSetter, attributes);
+  return info;
+}
 
 }  // namespace internal
 }  // namespace v8