AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Reland "[builtins] Clean up the use of class_name / ES5 [[Class]]"

Browse Source 
This is a reland of 29c1eab92e

Original change's description:
> [builtins] Clean up the use of class_name / ES5 [[Class]]
>
> Before ES2015, the ES spec had a [[Class]] internal slot for all
> objects, which Object.prototype.toString() would use to figure the
> returned string. Post-ES2015, the [[Class]] slot was removed in spec for
> all objects, with the @@toStringTag well-known symbol the proper way to
> change Object.prototype.toString() output.
>
> At the time, spec-identical handling without the use of [[Class]] was
> implemented in V8 for all objects other than API objects, where issues
> with the Web IDL spec [1] prevented Blink, and hence V8, to totally
> migrate to @@toStringTag. However, since 2016 [2] Blink has been setting
> @@toStringTag on API class prototypes to manage the
> Object.prototype.toString() output, so the legacy [[Class]] handling in
> V8 has not been necessary for the past couple of years.
>
> This CL removes the remaining legacy [[Class]] handling in
> Object.prototype.toString(), JSReceiver::class_name(), and
> GetConstructorName(). However, it does not remove the class_name field
> in FunctionTemplateInfo, as it is still used for the `name` property of
> created functions.
>
> This CL also cleans up other places in the codebase that still reference
> [[Class]].
>
> This change should have minimal impact on web-compatibility. For the
> change to be observable, a script must do one of the following:
>
> 1. delete APIConstructor.prototype[Symbol.toStringTag];
> 2. Object.setPrototypeOf(apiObject, somethingElse);
>
> Before this CL, these changes will not change the apiObject.toString()
> output. But after this CL, they will make apiObject.toString() show
> "[object Object]" (in the first case) or the @@toStringTag of the other
> prototype (in the latter case).
>
> However, both are deemed unlikely. @@toStringTag is not well-known
> feature of JavaScript, nor does it get tampered much on API
> constructors. In the second case, setting the prototype of an API object
> would effectly render the object useless, as all its methods (including
> property getters/setters) would no longer be accessible.
>
> Currently, @@toStringTag-based API object branding is not yet
> implemented by other browsers. This V8 bug in particular has been an
> impediment to standardizing toString behavior. Fixing this bug will
> unblock [3] and lead to a better Web IDL spec, and better toString()
> compatibility for all.
>
> [1]: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28244
> [2]: https://crrev.com/909c0d7d5a53c8526ded351683c65ea7d17531d4
> [3]: https://github.com/heycam/webidl/pull/357
>
> Bug: chromium:793406
> Cq-Include-Trybots: luci.chromium.try:linux-rel
> Change-Id: Iceded24e37afa2646ec385d5018909f55b177f93
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2146996
> Commit-Queue: Timothy Gu <timothygu@chromium.org>
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#67327}

Bug: chromium:793406
Change-Id: Ia5d97bd4e1c44cadc6f18a17ffc9d06b038cf8f1
Cq-Include-Trybots: luci.chromium.try:linux-rel
Cq-Include-Trybots: luci.v8.try:v8_linux_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2163881
Auto-Submit: Timothy Gu <timothygu@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67361}
This commit is contained in:
Timothy Gu 2020-04-20 11:16:57 -07:00 committed by Commit Bot
parent 780746d64c
commit 1aa51b498e
11 changed files with 81 additions and 150 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/builtins/builtins-object-gen.cc
View File

@ -709,12 +709,12 @@ TF_BUILTIN(ObjectPrototypeIsPrototypeOf, ObjectBuiltinsAssembler) {
}
TF_BUILTIN(ObjectToString, ObjectBuiltinsAssembler) {
Label checkstringtag(this), if_apiobject(this, Label::kDeferred),
if_arguments(this), if_array(this), if_boolean(this), if_date(this),
if_error(this), if_function(this), if_number(this, Label::kDeferred),
if_object(this), if_primitive(this), if_proxy(this, Label::kDeferred),
if_regexp(this), if_string(this), if_symbol(this, Label::kDeferred),
if_value(this), if_bigint(this, Label::kDeferred);
Label checkstringtag(this), if_arguments(this), if_array(this),
if_boolean(this), if_date(this), if_error(this), if_function(this),
if_number(this, Label::kDeferred), if_object(this), if_primitive(this),
if_proxy(this, Label::kDeferred), if_regexp(this), if_string(this),
if_symbol(this, Label::kDeferred), if_value(this),
if_bigint(this, Label::kDeferred);
TNode<Object> receiver = CAST(Parameter(Descriptor::kReceiver));
TNode<Context> context = CAST(Parameter(Descriptor::kContext));
@ -740,8 +740,8 @@ TF_BUILTIN(ObjectToString, ObjectBuiltinsAssembler) {
{JS_ARGUMENTS_OBJECT_TYPE, &if_arguments},
{JS_DATE_TYPE, &if_date},
{JS_BOUND_FUNCTION_TYPE, &if_function},
{JS_API_OBJECT_TYPE, &if_apiobject},
{JS_SPECIAL_API_OBJECT_TYPE, &if_apiobject},
{JS_API_OBJECT_TYPE, &if_object},
{JS_SPECIAL_API_OBJECT_TYPE, &if_object},
{JS_PROXY_TYPE, &if_proxy},
{JS_ERROR_TYPE, &if_error},
{JS_PRIMITIVE_WRAPPER_TYPE, &if_value}};
@ -755,25 +755,6 @@ TF_BUILTIN(ObjectToString, ObjectBuiltinsAssembler) {
Switch(receiver_instance_type, &if_object, case_values, case_labels,
arraysize(case_values));
BIND(&if_apiobject);
{
// Lookup the @@toStringTag property on the {receiver}.
TVARIABLE(Object, var_tag,
GetProperty(context, receiver,
isolate()->factory()->to_string_tag_symbol()));
Label if_tagisnotstring(this), if_tagisstring(this);
GotoIf(TaggedIsSmi(var_tag.value()), &if_tagisnotstring);
Branch(IsString(CAST(var_tag.value())), &if_tagisstring,
&if_tagisnotstring);
BIND(&if_tagisnotstring);
{
var_tag = CallRuntime(Runtime::kClassOf, context, receiver);
Goto(&if_tagisstring);
}
BIND(&if_tagisstring);
ReturnToStringFormat(context, CAST(var_tag.value()));
}
BIND(&if_arguments);
{
var_default = ArgumentsToStringConstant();

1
src/debug/debug-evaluate.cc
View File

@ -313,7 +313,6 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
V(ArrayIncludes_Slow) \
V(ArrayIndexOf) \
V(ArrayIsArray) \
V(ClassOf) \
V(GetFunctionName) \
V(GetOwnPropertyDescriptor) \
V(GlobalPrint) \

34
src/objects/js-objects.cc
View File

@ -405,19 +405,6 @@ String JSReceiver::class_name() {
if (IsJSWeakSet()) return roots.WeakSet_string();
if (IsJSGlobalProxy()) return roots.global_string();
Object maybe_constructor = map().GetConstructor();
if (maybe_constructor.IsJSFunction()) {
JSFunction constructor = JSFunction::cast(maybe_constructor);
if (constructor.shared().IsApiFunction()) {
maybe_constructor = constructor.shared().get_api_func_data();
}
}
if (maybe_constructor.IsFunctionTemplateInfo()) {
FunctionTemplateInfo info = FunctionTemplateInfo::cast(maybe_constructor);
if (info.class_name().IsString()) return String::cast(info.class_name());
}
return roots.Object_string();
}
@ -441,12 +428,6 @@ std::pair<MaybeHandle<JSFunction>, Handle<String>> GetConstructorHelper(
return std::make_pair(handle(constructor, isolate),
handle(name, isolate));
}
} else if (maybe_constructor.IsFunctionTemplateInfo()) {
FunctionTemplateInfo info = FunctionTemplateInfo::cast(maybe_constructor);
if (info.class_name().IsString()) {
return std::make_pair(MaybeHandle<JSFunction>(),
handle(String::cast(info.class_name()), isolate));
}
}
}
@ -4510,14 +4491,13 @@ Maybe<bool> JSObject::SetPrototype(Handle<JSObject> object,
NewTypeError(MessageTemplate::kImmutablePrototypeSet, object));
}
// From 8.6.2 Object Internal Methods
// ...
// In addition, if [[Extensible]] is false the value of the [[Class]] and
// [[Prototype]] internal properties of the object may not be modified.
// ...
// Implementation specific extensions that modify [[Class]], [[Prototype]]
// or [[Extensible]] must not violate the invariants defined in the preceding
// paragraph.
// From 6.1.7.3 Invariants of the Essential Internal Methods
//
// [[SetPrototypeOf]] ( V )
// * ...
// * If target is non-extensible, [[SetPrototypeOf]] must return false,
// unless V is the SameValue as the target's observed [[GetPrototypeOf]]
// value.
if (!all_extensible) {
RETURN_FAILURE(isolate, should_throw,
NewTypeError(MessageTemplate::kNonExtensibleProto, object));

8
src/objects/js-objects.h
View File

@ -206,15 +206,17 @@ class JSReceiver : public HeapObject {
V8_WARN_UNUSED_RESULT static Maybe<bool> IsExtensible(
Handle<JSReceiver> object);
// Returns the class name ([[Class]] property in the specification).
// Returns the class name.
V8_EXPORT_PRIVATE String class_name();
// Returns the constructor (the function that was used to instantiate the
// object).
static MaybeHandle<JSFunction> GetConstructor(Handle<JSReceiver> receiver);
// Returns the constructor name (the name (possibly, inferred name) of the
// function that was used to instantiate the object).
// Returns the constructor name (the (possibly inferred) name of the function
// that was used to instantiate the object), if any. If a FunctionTemplate is
// used to instantiate the object, the class_name of the FunctionTemplate is
// returned instead.
static Handle<String> GetConstructorName(Handle<JSReceiver> receiver);
V8_EXPORT_PRIVATE Handle<NativeContext> GetCreationContext();

8
src/runtime/runtime-object.cc
View File

@ -989,14 +989,6 @@ RUNTIME_FUNCTION(Runtime_IsJSReceiver) {
return isolate->heap()->ToBoolean(obj.IsJSReceiver());
}
RUNTIME_FUNCTION(Runtime_ClassOf) {
SealHandleScope shs(isolate);
DCHECK_EQ(1, args.length());
CONVERT_ARG_CHECKED(Object, obj, 0);
if (!obj.IsJSReceiver()) return ReadOnlyRoots(isolate).null_value();
return JSReceiver::cast(obj).class_name();
}
RUNTIME_FUNCTION(Runtime_GetFunctionName) {
HandleScope scope(isolate);
DCHECK_EQ(1, args.length());

1
src/runtime/runtime.h
View File

@ -285,7 +285,6 @@ namespace internal {
F(AddPrivateField, 3, 1) \
F(AddPrivateBrand, 3, 1) \
F(AllocateHeapNumber, 0, 1) \
F(ClassOf, 1, 1) \
F(CollectTypeProfile, 3, 1) \
F(CompleteInobjectSlackTrackingForMap, 1, 1) \
I(CopyDataProperties, 2, 1) \

119
test/cctest/test-api.cc
View File

@ -1075,20 +1075,23 @@ template<typename Constructor, typename Accessor>
static void TestFunctionTemplateAccessor(Constructor constructor,
Accessor accessor) {
LocalContext env;
v8::HandleScope scope(env->GetIsolate());
v8::Isolate* isolate = env->GetIsolate();
v8::HandleScope scope(isolate);
Local<v8::FunctionTemplate> fun_templ =
v8::FunctionTemplate::New(env->GetIsolate(), constructor);
fun_templ->SetClassName(v8_str("funky"));
v8::FunctionTemplate::New(isolate, constructor);
fun_templ->PrototypeTemplate()->Set(
v8::Symbol::GetToStringTag(isolate), v8_str("funky"),
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontEnum));
fun_templ->InstanceTemplate()->SetAccessor(v8_str("m"), accessor);
Local<Function> fun = fun_templ->GetFunction(env.local()).ToLocalChecked();
CHECK(env->Global()->Set(env.local(), v8_str("obj"), fun).FromJust());
Local<Value> result =
v8_compile("(new obj()).toString()")->Run(env.local()).ToLocalChecked();
Local<Value> result = CompileRun("(new obj()).toString()");
CHECK(v8_str("[object funky]")->Equals(env.local(), result).FromJust());
CompileRun("var obj_instance = new obj();");
Local<Script> script;
script = v8_compile("obj_instance.x");
Local<Script> script = v8_compile("obj_instance.x");
for (int i = 0; i < 30; i++) {
CHECK_EQ(1, v8_run_int32value(script));
}
@ -12633,22 +12636,20 @@ THREADED_TEST(NewTargetHandler) {
}
THREADED_TEST(ObjectProtoToString) {
LocalContext context;
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
templ->SetClassName(v8_str("MyClass"));
LocalContext context;
Local<String> customized_tostring = v8_str("customized toString");
// Replace Object.prototype.toString
v8_compile(
"Object.prototype.toString = function() {"
" return 'customized toString';"
"}")
->Run(context.local())
.ToLocalChecked();
CompileRun(R"(
Object.prototype.toString = function() {
return 'customized toString';
})");
// Normal ToString call should call replaced Object.prototype.toString
Local<v8::Object> instance = templ->GetFunction(context.local())
@ -12659,57 +12660,12 @@ THREADED_TEST(ObjectProtoToString) {
CHECK(value->IsString() &&
value->Equals(context.local(), customized_tostring).FromJust());
// ObjectProtoToString should not call replace toString function.
// ObjectProtoToString should not call replace toString function. It should
// not look at the class name either.
value = instance->ObjectProtoToString(context.local()).ToLocalChecked();
CHECK(value->IsString() &&
value->Equals(context.local(), v8_str("[object MyClass]")).FromJust());
// Check global
value =
context->Global()->ObjectProtoToString(context.local()).ToLocalChecked();
CHECK(value->IsString() &&
value->Equals(context.local(), v8_str("[object Object]")).FromJust());
// Check ordinary object
Local<Value> object =
v8_compile("new Object()")->Run(context.local()).ToLocalChecked();
value = object.As<v8::Object>()
->ObjectProtoToString(context.local())
.ToLocalChecked();
CHECK(value->IsString() &&
value->Equals(context.local(), v8_str("[object Object]")).FromJust());
}
TEST(ObjectProtoToStringES6) {
LocalContext context;
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
templ->SetClassName(v8_str("MyClass"));
Local<String> customized_tostring = v8_str("customized toString");
// Replace Object.prototype.toString
CompileRun(
"Object.prototype.toString = function() {"
" return 'customized toString';"
"}");
// Normal ToString call should call replaced Object.prototype.toString
Local<v8::Object> instance = templ->GetFunction(context.local())
.ToLocalChecked()
->NewInstance(context.local())
.ToLocalChecked();
Local<String> value = instance->ToString(context.local()).ToLocalChecked();
CHECK(value->IsString() &&
value->Equals(context.local(), customized_tostring).FromJust());
// ObjectProtoToString should not call replace toString function.
value = instance->ObjectProtoToString(context.local()).ToLocalChecked();
CHECK(value->IsString() &&
value->Equals(context.local(), v8_str("[object MyClass]")).FromJust());
// Check global
value =
context->Global()->ObjectProtoToString(context.local()).ToLocalChecked();
@ -12723,10 +12679,49 @@ TEST(ObjectProtoToStringES6) {
.ToLocalChecked();
CHECK(value->IsString() &&
value->Equals(context.local(), v8_str("[object Object]")).FromJust());
}
// Check that ES6 semantics using @@toStringTag work
TEST(ObjectProtoToStringES6) {
LocalContext context;
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
// Check that ES6 semantics using @@toStringTag work.
Local<v8::Symbol> toStringTag = v8::Symbol::GetToStringTag(isolate);
Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
templ->SetClassName(v8_str("MyClass"));
templ->PrototypeTemplate()->Set(
toStringTag, v8_str("MyClassToStringTag"),
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontEnum));
Local<String> customized_tostring = v8_str("customized toString");
// Replace Object.prototype.toString
CompileRun(R"(
Object.prototype.toString = function() {
return 'customized toString';
})");
// Normal ToString call should call replaced Object.prototype.toString
Local<v8::Object> instance = templ->GetFunction(context.local())
.ToLocalChecked()
->NewInstance(context.local())
.ToLocalChecked();
Local<String> value = instance->ToString(context.local()).ToLocalChecked();
CHECK(value->IsString() &&
value->Equals(context.local(), customized_tostring).FromJust());
// ObjectProtoToString should not call replace toString function. Instead it
// should look at the @@toStringTag property.
value = instance->ObjectProtoToString(context.local()).ToLocalChecked();
CHECK(value->IsString() &&
value->Equals(context.local(), v8_str("[object MyClassToStringTag]"))
.FromJust());
Local<Value> object;
#define TEST_TOSTRINGTAG(type, tag, expected) \
do { \
object = CompileRun("new " #type "()"); \

2
test/mjsunit/es6/array-copywithin.js
View File

@ -42,7 +42,7 @@
Array.prototype.copyWithin.call(args, -2, 0);
assertArrayEquals([1, 1, 2], Array.prototype.slice.call(args));
// [[Class]] does not change
// Object.prototype.toString branding does not change
assertArrayEquals("[object Arguments]", Object.prototype.toString.call(args));
})();

2
test/mjsunit/mjsunit.js
View File

@ -295,7 +295,7 @@ var prettyPrinted;
default:
return objectClass + "(" + String(value) + ")";
}
// [[Class]] is "Object".
// classOf() returned "Object".
var name = value.constructor.name;
if (name) return name + "()";
return "Object()";

4
test/mjsunit/class-of-builtins.js → test/mjsunit/object-tostring-builtins.js
View File

@ -25,8 +25,8 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// The [[Class]] property of (instances of) builtin functions must be
// correctly set.
// Object.prototype.toString should return the correct values for instances of
// various built-in classes.
var funs = {
Object: [ Object ],
Function: [ Function ],

17
test/unittests/api/remote-object-unittest.cc
View File

@ -98,22 +98,5 @@ TEST_F(RemoteObjectTest, TypeOfRemoteObject) {
EXPECT_STREQ("object", *result);
}
TEST_F(RemoteObjectTest, ClassOf) {
Local<FunctionTemplate> constructor_template =
FunctionTemplate::New(isolate(), Constructor);
constructor_template->InstanceTemplate()->SetAccessCheckCallbackAndHandler(
AccessCheck, NamedPropertyHandlerConfiguration(NamedGetter),
IndexedPropertyHandlerConfiguration());
constructor_template->SetClassName(
String::NewFromUtf8Literal(isolate(), "test_class"));
Local<Object> remote_object =
constructor_template->NewRemoteInstance().ToLocalChecked();
Local<String> class_name = Utils::ToLocal(
i::handle(Utils::OpenHandle(*remote_object)->class_name(), i_isolate()));
String::Utf8Value result(isolate(), class_name);
EXPECT_STREQ("test_class", *result);
}
} // namespace remote_object_unittest
} // namespace v8