Revert "[builtins] Clean up the use of class_name / ES5 [[Class]]"
This reverts commit 29c1eab92e.
Reason for revert: Causes Blink test failures:
https://ci.chromium.org/p/v8/builders/ci/V8%20Blink%20Linux/4222
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}
TBR=verwaest@chromium.org,timothygu@chromium.org
Change-Id: I678d2ffc1064b1d1ddb62024cc23c6c41b216ef4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:793406
Cq-Include-Trybots: luci.chromium.try:linux-rel
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2163956
Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67349}
This commit is contained in:
Bill Budge
Commit Bot
parent
c47d1631a3
commit
213016d65a
@ -709,12 +709,12 @@ TF_BUILTIN(ObjectPrototypeIsPrototypeOf, ObjectBuiltinsAssembler) {
|
||||
}
|
||||
|
||||
TF_BUILTIN(ObjectToString, ObjectBuiltinsAssembler) {
|
||||
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);
|
||||
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);
|
||||
|
||||
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_object},
|
||||
{JS_SPECIAL_API_OBJECT_TYPE, &if_object},
|
||||
{JS_API_OBJECT_TYPE, &if_apiobject},
|
||||
{JS_SPECIAL_API_OBJECT_TYPE, &if_apiobject},
|
||||
{JS_PROXY_TYPE, &if_proxy},
|
||||
{JS_ERROR_TYPE, &if_error},
|
||||
{JS_PRIMITIVE_WRAPPER_TYPE, &if_value}};
|
||||
@ -755,6 +755,25 @@ 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();
|
||||
|
||||
@ -313,6 +313,7 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
|
||||
V(ArrayIncludes_Slow) \
|
||||
V(ArrayIndexOf) \
|
||||
V(ArrayIsArray) \
|
||||
V(ClassOf) \
|
||||
V(GetFunctionName) \
|
||||
V(GetOwnPropertyDescriptor) \
|
||||
V(GlobalPrint) \
|
||||
|
||||
@ -405,6 +405,19 @@ 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();
|
||||
}
|
||||
|
||||
@ -428,6 +441,12 @@ 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));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -4491,13 +4510,14 @@ Maybe<bool> JSObject::SetPrototype(Handle<JSObject> object,
|
||||
NewTypeError(MessageTemplate::kImmutablePrototypeSet, object));
|
||||
}
|
||||
|
||||
// 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.
|
||||
// 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.
|
||||
if (!all_extensible) {
|
||||
RETURN_FAILURE(isolate, should_throw,
|
||||
NewTypeError(MessageTemplate::kNonExtensibleProto, object));
|
||||
|
||||
@ -206,17 +206,15 @@ class JSReceiver : public HeapObject {
|
||||
V8_WARN_UNUSED_RESULT static Maybe<bool> IsExtensible(
|
||||
Handle<JSReceiver> object);
|
||||
|
||||
// Returns the class name.
|
||||
// Returns the class name ([[Class]] property in the specification).
|
||||
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 (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.
|
||||
// Returns the constructor name (the name (possibly, inferred name) of the
|
||||
// function that was used to instantiate the object).
|
||||
static Handle<String> GetConstructorName(Handle<JSReceiver> receiver);
|
||||
|
||||
V8_EXPORT_PRIVATE Handle<NativeContext> GetCreationContext();
|
||||
|
||||
@ -989,6 +989,14 @@ 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());
|
||||
|
||||
@ -285,6 +285,7 @@ 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) \
|
||||
|
||||
@ -1075,23 +1075,20 @@ template<typename Constructor, typename Accessor>
|
||||
static void TestFunctionTemplateAccessor(Constructor constructor,
|
||||
Accessor accessor) {
|
||||
LocalContext env;
|
||||
v8::Isolate* isolate = env->GetIsolate();
|
||||
v8::HandleScope scope(isolate);
|
||||
v8::HandleScope scope(env->GetIsolate());
|
||||
|
||||
Local<v8::FunctionTemplate> fun_templ =
|
||||
v8::FunctionTemplate::New(isolate, constructor);
|
||||
fun_templ->PrototypeTemplate()->Set(
|
||||
v8::Symbol::GetToStringTag(isolate), v8_str("funky"),
|
||||
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontEnum));
|
||||
v8::FunctionTemplate::New(env->GetIsolate(), constructor);
|
||||
fun_templ->SetClassName(v8_str("funky"));
|
||||
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 = CompileRun("(new obj()).toString()");
|
||||
Local<Value> result =
|
||||
v8_compile("(new obj()).toString()")->Run(env.local()).ToLocalChecked();
|
||||
CHECK(v8_str("[object funky]")->Equals(env.local(), result).FromJust());
|
||||
CompileRun("var obj_instance = new obj();");
|
||||
|
||||
Local<Script> script = v8_compile("obj_instance.x");
|
||||
Local<Script> script;
|
||||
script = v8_compile("obj_instance.x");
|
||||
for (int i = 0; i < 30; i++) {
|
||||
CHECK_EQ(1, v8_run_int32value(script));
|
||||
}
|
||||
@ -12636,20 +12633,22 @@ 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
|
||||
CompileRun(R"(
|
||||
Object.prototype.toString = function() {
|
||||
return 'customized toString';
|
||||
})");
|
||||
v8_compile(
|
||||
"Object.prototype.toString = function() {"
|
||||
" return 'customized toString';"
|
||||
"}")
|
||||
->Run(context.local())
|
||||
.ToLocalChecked();
|
||||
|
||||
// Normal ToString call should call replaced Object.prototype.toString
|
||||
Local<v8::Object> instance = templ->GetFunction(context.local())
|
||||
@ -12660,12 +12659,57 @@ THREADED_TEST(ObjectProtoToString) {
|
||||
CHECK(value->IsString() &&
|
||||
value->Equals(context.local(), customized_tostring).FromJust());
|
||||
|
||||
// ObjectProtoToString should not call replace toString function. It should
|
||||
// not look at the class name either.
|
||||
// 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();
|
||||
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();
|
||||
@ -12679,49 +12723,10 @@ THREADED_TEST(ObjectProtoToString) {
|
||||
.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);
|
||||
|
||||
// Check that ES6 semantics using @@toStringTag work.
|
||||
// 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 "()"); \
|
||||
|
||||
@ -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.
|
||||
|
||||
// Object.prototype.toString should return the correct values for instances of
|
||||
// various built-in classes.
|
||||
// The [[Class]] property of (instances of) builtin functions must be
|
||||
// correctly set.
|
||||
var funs = {
|
||||
Object: [ Object ],
|
||||
Function: [ Function ],
|
||||
@ -42,7 +42,7 @@
|
||||
Array.prototype.copyWithin.call(args, -2, 0);
|
||||
assertArrayEquals([1, 1, 2], Array.prototype.slice.call(args));
|
||||
|
||||
// Object.prototype.toString branding does not change
|
||||
// [[Class]] does not change
|
||||
assertArrayEquals("[object Arguments]", Object.prototype.toString.call(args));
|
||||
})();
|
||||
|
||||
|
||||
@ -295,7 +295,7 @@ var prettyPrinted;
|
||||
default:
|
||||
return objectClass + "(" + String(value) + ")";
|
||||
}
|
||||
// classOf() returned "Object".
|
||||
// [[Class]] is "Object".
|
||||
var name = value.constructor.name;
|
||||
if (name) return name + "()";
|
||||
return "Object()";
|
||||
|
||||
@ -98,5 +98,22 @@ 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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user