[builtins] Also simplify the Symbol constructor.

src/builtins.cc

 // Copyright 2012 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.
 
 #include "src/builtins.h"
 
 #include "src/api.h"
 #include "src/api-natives.h"
 #include "src/arguments.h"
 #include "src/base/once.h"
@@ skipping 1427 matching lines @@
 
   Handle<JSArray> result_array;
   if (Fast_ArrayConcat(isolate, &args).ToHandle(&result_array)) {
     return *result_array;
   }
   if (isolate->has_pending_exception()) return isolate->heap()->exception();
   return Slow_ArrayConcat(&args, isolate);
 }
 
 
-// -----------------------------------------------------------------------------
-//
-
-
-// 20.3.4.45 Date.prototype [ @@toPrimitive ] ( hint )
+// ES6 section 20.3.4.45 Date.prototype [ @@toPrimitive ] ( hint )
 BUILTIN(DateToPrimitive) {
   HandleScope scope(isolate);
   DCHECK_EQ(2, args.length());
   if (!args.receiver()->IsJSReceiver()) {
     THROW_NEW_ERROR_RETURN_FAILURE(
         isolate, NewTypeError(MessageTemplate::kIncompatibleMethodReceiver,
                               isolate->factory()->NewStringFromAsciiChecked(
                                   "Date.prototype [ @@toPrimitive ]"),
                               args.receiver()));
   }
   Handle<JSReceiver> receiver = args.at<JSReceiver>(0);
   Handle<Object> hint = args.at<Object>(1);
   Handle<Object> result;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result,
                                      JSDate::ToPrimitive(receiver, hint));
   return *result;
 }
 
 
+// ES6 section 19.4.1.1 Symbol ( [ description ] ) for the [[Call]] case.
+BUILTIN(SymbolConstructor) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(2, args.length());
+  Handle<Symbol> result = isolate->factory()->NewSymbol();
+  Handle<Object> description = args.at<Object>(1);
+  if (!description->IsUndefined()) {
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, description,

[adamk, 2015/09/16 19:33:31]

I'd like to understand why this approach is better than the previous version.
Here the call stack is:

JS -> C++ -> JS

instead of the old

JS -> JS

I thought I'd understood that part of the point of these cleanups was to reduce
the hops between JS and C++, but that seems to be increased by this change.

If the main point of this change is to get rid of %_IsConstructCall(), you could
replace it with new.target instead, which in ES6 exposes the IsConstruct-ness of
a [[Call]] to author script.

[Benedikt Meurer, 2015/09/17 02:58:10]

The main point of these cleanups is to make it correct, and as you guessed,
%_IsConstructCall is buggy (in the case of TCO), and unfortunately the same
applies to new.target (currently). But in case of the Symbol constructor it was
never

 JS -> JS 

but always

 JS -> (JS ->) C++ -> JS

because it always ended up with a call to %CreateSymbol, and it was also using
the obsolete ToString implementation, which is why it wouldn't do the callback
from C++ to JS previously. So fixing all of that was easiest by just writing
everything in C++, and that's what we agreed on as baseline implementation. If
that ever becomes a bottleneck we can turn it into a native builtin.

+                                       Object::ToString(isolate, description));
+    result->set_name(*description);
+  }
+  return *result;
+}
+
+
+// ES6 section 19.4.1.1 Symbol ( [ description ] ) for the [[Construct]] case.
+BUILTIN(SymbolConstructor_ConstructStub) {
+  HandleScope scope(isolate);
+  THROW_NEW_ERROR_RETURN_FAILURE(
+      isolate, NewTypeError(MessageTemplate::kNotConstructor,
+                            isolate->factory()->Symbol_string()));
+}
+
+
 // -----------------------------------------------------------------------------
 // Throwers for restricted function properties and strict arguments object
 // properties
 
 
 BUILTIN(RestrictedFunctionPropertiesThrower) {
   HandleScope scope(isolate);
   THROW_NEW_ERROR_RETURN_FAILURE(
       isolate, NewTypeError(MessageTemplate::kRestrictedFunctionProperties));
 }
@@ skipping 588 matching lines @@
 BUILTIN_LIST_C(DEFINE_BUILTIN_ACCESSOR_C)
 BUILTIN_LIST_A(DEFINE_BUILTIN_ACCESSOR_A)
 BUILTIN_LIST_H(DEFINE_BUILTIN_ACCESSOR_H)
 BUILTIN_LIST_DEBUG_A(DEFINE_BUILTIN_ACCESSOR_A)
 #undef DEFINE_BUILTIN_ACCESSOR_C
 #undef DEFINE_BUILTIN_ACCESSOR_A
 
 
 }  // namespace internal
 }  // namespace v8