Skip to content

Commit 16d954a

Browse files
ConstellationConstellation
committed
[JSC] Use reifying system for "name" property of builtin JSFunction
https://bugs.webkit.org/show_bug.cgi?id=175260 Reviewed by Saam Barati. JSTests: * stress/accessors-get-set-prefix.js: * stress/builtin-function-name.js: Added. (shouldBe): (shouldThrow): (shouldBe.JSON.stringify.Object.getOwnPropertyDescriptor): (shouldBe.JSON.stringify.Object.getOwnPropertyNames.Array.prototype.filter.sort): Source/JavaScriptCore: Currently builtin JSFunction uses direct property for "name", which is different from usual JSFunction. Usual JSFunction uses reifying system for "name". We would like to apply this reifying mechanism to builtin JSFunction to simplify code and drop JSFunction::createBuiltinFunction. We would like to store the "correct" name in FunctionExecutable. For example, we would like to store the name like "get [Symbol.species]" to FunctionExecutable instead of specifying name when creating JSFunction. To do so, we add a new annotations, @Getter and @overriddenName. When @Getter is specified, the name of the function becomes "get xxx". And when @overriddenName="xxx" is specified, the name of the function becomes "xxx". * Scripts/builtins/builtins_generate_combined_header.py: (generate_section_for_code_table_macro): * Scripts/builtins/builtins_generate_combined_implementation.py: (BuiltinsCombinedImplementationGenerator.generate_secondary_header_includes): * Scripts/builtins/builtins_generate_separate_header.py: (generate_section_for_code_table_macro): * Scripts/builtins/builtins_generate_separate_implementation.py: (BuiltinsSeparateImplementationGenerator.generate_secondary_header_includes): * Scripts/builtins/builtins_model.py: (BuiltinFunction.__init__): (BuiltinFunction.fromString): * Scripts/builtins/builtins_templates.py: * Scripts/tests/builtins/JavaScriptCore-Builtin.prototype-Combined.js: (overriddenName.string_appeared_here.match): (intrinsic.RegExpTestIntrinsic.test): * Scripts/tests/builtins/JavaScriptCore-Builtin.prototype-Separate.js: (overriddenName.string_appeared_here.match): (intrinsic.RegExpTestIntrinsic.test): * Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-Combined.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-Separate.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-Builtin.prototype-Combined.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-Builtin.prototype-Separate.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-BuiltinConstructor-Combined.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-BuiltinConstructor-Separate.js-result: * Scripts/tests/builtins/expected/JavaScriptCore-InternalClashingNames-Combined.js-result: * Scripts/tests/builtins/expected/WebCore-AnotherGuardedInternalBuiltin-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-GuardedBuiltin-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-GuardedInternalBuiltin-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result: * Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result: * builtins/BuiltinExecutables.cpp: (JSC::BuiltinExecutables::BuiltinExecutables): * builtins/BuiltinExecutables.h: * builtins/FunctionPrototype.js: (symbolHasInstance): Deleted. * builtins/GlobalOperations.js: (globalPrivate.speciesGetter): Deleted. * builtins/IteratorPrototype.js: (symbolIteratorGetter): Deleted. * builtins/RegExpPrototype.js: (match): Deleted. (replace): Deleted. (search): Deleted. (split): Deleted. * jsc.cpp: (functionCreateBuiltin): * runtime/FunctionPrototype.cpp: (JSC::FunctionPrototype::addFunctionProperties): * runtime/IteratorPrototype.cpp: (JSC::IteratorPrototype::finishCreation): * runtime/JSFunction.cpp: (JSC::JSFunction::getOwnNonIndexPropertyNames): (JSC::JSFunction::reifyLazyBoundNameIfNeeded): (JSC::JSFunction::createBuiltinFunction): Deleted. * runtime/JSFunction.h: * runtime/JSGlobalObject.cpp: (JSC::JSGlobalObject::init): * runtime/JSObject.cpp: (JSC::JSObject::putDirectBuiltinFunction): (JSC::JSObject::putDirectBuiltinFunctionWithoutTransition): * runtime/JSTypedArrayViewPrototype.cpp: (JSC::JSTypedArrayViewPrototype::finishCreation): * runtime/Lookup.cpp: (JSC::reifyStaticAccessor): * runtime/RegExpPrototype.cpp: (JSC::RegExpPrototype::finishCreation): Source/WebCore: Use @Getter for JSBuiltin getters. * Modules/fetch/FetchResponse.js: (bodyUsed): Deleted. (body): Deleted. * Modules/streams/ReadableByteStreamController.js: (byobRequest): Deleted. (desiredSize): Deleted. * Modules/streams/ReadableStream.js: (locked): Deleted. * Modules/streams/ReadableStreamBYOBReader.js: (closed): Deleted. * Modules/streams/ReadableStreamBYOBRequest.js: (view): Deleted. * Modules/streams/ReadableStreamDefaultController.js: (desiredSize): Deleted. * Modules/streams/ReadableStreamDefaultReader.js: (closed): Deleted. * Modules/streams/WritableStream.js: (closed): Deleted. (ready): Deleted. (state): Deleted. * bindings/js/JSDOMBuiltinConstructor.h: (WebCore::JSDOMBuiltinConstructor<JSClass>::finishCreation): LayoutTests: * js/dom/builtin-getter-name-expected.txt: Added. * js/dom/builtin-getter-name.html: Added. Canonical link: https://commits.webkit.org/192754@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221327 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 8c7381f commit 16d954a

58 files changed

Lines changed: 808 additions & 172 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎JSTests/ChangeLog‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
2017-08-29 Yusuke Suzuki <utatane.tea@gmail.com>
2+
3+
[JSC] Use reifying system for "name" property of builtin JSFunction
4+
https://bugs.webkit.org/show_bug.cgi?id=175260
5+
6+
Reviewed by Saam Barati.
7+
8+
* stress/accessors-get-set-prefix.js:
9+
* stress/builtin-function-name.js: Added.
10+
(shouldBe):
11+
(shouldThrow):
12+
(shouldBe.JSON.stringify.Object.getOwnPropertyDescriptor):
13+
(shouldBe.JSON.stringify.Object.getOwnPropertyNames.Array.prototype.filter.sort):
14+
115
2017-08-25 Saam Barati <sbarati@apple.com>
216

317
Support compiling catch in the DFG

‎JSTests/stress/accessors-get-set-prefix.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,15 @@ function tryGetOwnPropertyDescriptorGetName(obj, property, expectedName)
1414
}
1515

1616
tryGetOwnPropertyDescriptorGetName(Array, Symbol.species, "get [Symbol.species]");
17+
tryGetOwnPropertyDescriptorGetName(Map, Symbol.species, "get [Symbol.species]");
18+
tryGetOwnPropertyDescriptorGetName(Set, Symbol.species, "get [Symbol.species]");
19+
tryGetOwnPropertyDescriptorGetName(RegExp, Symbol.species, "get [Symbol.species]");
20+
tryGetOwnPropertyDescriptorGetName(Promise, Symbol.species, "get [Symbol.species]");
1721
tryGetOwnPropertyDescriptorGetName(Map.prototype, "size", "get size");
1822
tryGetOwnPropertyDescriptorGetName(Set.prototype, "size", "get size");
23+
tryGetOwnPropertyDescriptorGetName(RegExp.prototype, "flags", "get flags");
24+
tryGetOwnPropertyDescriptorGetName(RegExp.prototype, "sticky", "get sticky");
25+
tryGetOwnPropertyDescriptorGetName(RegExp.prototype, "source", "get source");
1926

2027
if (Object.__lookupGetter__("__proto__").name !== "get __proto__")
2128
throw "Expected Object __proto__ getter to be named \"get __proto\"";

‎JSTests/stress/builtin-function-name.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
function shouldBe(actual, expected) {
2+
if (actual !== expected)
3+
throw new Error('bad value: ' + actual);
4+
}
5+
6+
function shouldThrow(func, errorMessage) {
7+
var errorThrown = false;
8+
var error = null;
9+
try {
10+
func();
11+
} catch (e) {
12+
errorThrown = true;
13+
error = e;
14+
}
15+
if (!errorThrown)
16+
throw new Error('not thrown');
17+
if (String(error) !== errorMessage)
18+
throw new Error(`bad error: ${String(error)}`);
19+
}
20+
21+
{
22+
shouldBe(JSON.stringify(Object.getOwnPropertyNames(Array.prototype.filter).sort()), `["length","name"]`);
23+
shouldBe(Array.prototype.filter.name, "filter");
24+
shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(Array.prototype.filter, 'name')), `{"value":"filter","writable":false,"enumerable":false,"configurable":true}`);
25+
shouldBe(delete Array.prototype.filter.name, true);
26+
shouldBe(JSON.stringify(Object.getOwnPropertyNames(Array.prototype.filter).sort()), `["length"]`);
27+
}
28+
29+
{
30+
shouldThrow(function () {
31+
"use strict";
32+
Array.prototype.forEach.name = 42;
33+
}, `TypeError: Attempted to assign to readonly property.`);
34+
}
35+
36+
{
37+
var resolve = null;
38+
var reject = null;
39+
new Promise(function (arg0, arg1) {
40+
resolve = arg0;
41+
reject = arg1;
42+
});
43+
shouldBe(resolve.name, "resolve");
44+
shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(resolve, 'name')), `{"value":"resolve","writable":false,"enumerable":false,"configurable":true}`);
45+
shouldBe(reject.name, "reject");
46+
shouldBe(JSON.stringify(Object.getOwnPropertyDescriptor(reject, 'name')), `{"value":"reject","writable":false,"enumerable":false,"configurable":true}`);
47+
}

‎LayoutTests/ChangeLog‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
2017-08-29 Yusuke Suzuki <utatane.tea@gmail.com>
2+
3+
[JSC] Use reifying system for "name" property of builtin JSFunction
4+
https://bugs.webkit.org/show_bug.cgi?id=175260
5+
6+
Reviewed by Saam Barati.
7+
8+
* js/dom/builtin-getter-name-expected.txt: Added.
9+
* js/dom/builtin-getter-name.html: Added.
10+
111
2017-08-29 Per Arne Vollan <pvollan@apple.com>
212

313
Rebaseline editing/style/5084241.html after r220706.

‎LayoutTests/js/dom/builtin-getter-name-expected.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
PASS FetchResponse property names
3+
PASS ReadableStream property names
4+
PASS ReadableStreamDefaultReader property names
5+
PASS ReadableStreamBYOBReader property names
6+
PASS WritableStream property names
7+

‎LayoutTests/js/dom/builtin-getter-name.html‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>Builtin JS getter names</title>
5+
<script src="../../resources/testharness.js"></script>
6+
<script src="../../resources/testharnessreport.js"></script>
7+
</head>
8+
<body>
9+
<script>
10+
11+
function tryGetOwnPropertyDescriptorGetName(obj, property, expectedName)
12+
{
13+
let descriptor = Object.getOwnPropertyDescriptor(obj, property);
14+
assert_true(!!descriptor, `Should find property descriptor for ${property}`);
15+
16+
let getter = descriptor.get;
17+
assert_true(!!getter, `Should find getter for ${property}`);
18+
19+
let getterName = getter.name;
20+
assert_equals(getterName, expectedName);
21+
}
22+
23+
test(() => {
24+
tryGetOwnPropertyDescriptorGetName(Response.prototype, 'body', 'get body');
25+
tryGetOwnPropertyDescriptorGetName(Response.prototype, 'bodyUsed', 'get bodyUsed');
26+
}, 'FetchResponse property names');
27+
28+
test(() => {
29+
tryGetOwnPropertyDescriptorGetName(ReadableStream.prototype, 'locked', 'get locked');
30+
}, 'ReadableStream property names');
31+
32+
test(() => {
33+
let stream = new ReadableStream();
34+
let reader = stream.getReader();
35+
tryGetOwnPropertyDescriptorGetName(reader.__proto__, 'closed', 'get closed');
36+
}, 'ReadableStreamDefaultReader property names');
37+
38+
test(() => {
39+
let stream = new ReadableStream({
40+
start: function(c) {
41+
controller = c;
42+
},
43+
type: "bytes"
44+
});
45+
let reader = stream.getReader({ mode: 'byob' });
46+
tryGetOwnPropertyDescriptorGetName(reader.__proto__, 'closed', 'get closed');
47+
}, 'ReadableStreamBYOBReader property names');
48+
49+
test(() => {
50+
tryGetOwnPropertyDescriptorGetName(WritableStream.prototype, 'closed', 'get closed');
51+
tryGetOwnPropertyDescriptorGetName(WritableStream.prototype, 'ready', 'get ready');
52+
tryGetOwnPropertyDescriptorGetName(WritableStream.prototype, 'state', 'get state');
53+
}, 'WritableStream property names');
54+
55+
</script>
56+
</body>
57+
</html>

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,90 @@
1+
2017-08-29 Yusuke Suzuki <utatane.tea@gmail.com>
2+
3+
[JSC] Use reifying system for "name" property of builtin JSFunction
4+
https://bugs.webkit.org/show_bug.cgi?id=175260
5+
6+
Reviewed by Saam Barati.
7+
8+
Currently builtin JSFunction uses direct property for "name", which is different
9+
from usual JSFunction. Usual JSFunction uses reifying system for "name". We would like
10+
to apply this reifying mechanism to builtin JSFunction to simplify code and drop
11+
JSFunction::createBuiltinFunction.
12+
13+
We would like to store the "correct" name in FunctionExecutable. For example,
14+
we would like to store the name like "get [Symbol.species]" to FunctionExecutable
15+
instead of specifying name when creating JSFunction. To do so, we add a new
16+
annotations, @getter and @overriddenName. When @getter is specified, the name of
17+
the function becomes "get xxx". And when @overriddenName="xxx" is specified,
18+
the name of the function becomes "xxx".
19+
20+
* Scripts/builtins/builtins_generate_combined_header.py:
21+
(generate_section_for_code_table_macro):
22+
* Scripts/builtins/builtins_generate_combined_implementation.py:
23+
(BuiltinsCombinedImplementationGenerator.generate_secondary_header_includes):
24+
* Scripts/builtins/builtins_generate_separate_header.py:
25+
(generate_section_for_code_table_macro):
26+
* Scripts/builtins/builtins_generate_separate_implementation.py:
27+
(BuiltinsSeparateImplementationGenerator.generate_secondary_header_includes):
28+
* Scripts/builtins/builtins_model.py:
29+
(BuiltinFunction.__init__):
30+
(BuiltinFunction.fromString):
31+
* Scripts/builtins/builtins_templates.py:
32+
* Scripts/tests/builtins/JavaScriptCore-Builtin.prototype-Combined.js:
33+
(overriddenName.string_appeared_here.match):
34+
(intrinsic.RegExpTestIntrinsic.test):
35+
* Scripts/tests/builtins/JavaScriptCore-Builtin.prototype-Separate.js:
36+
(overriddenName.string_appeared_here.match):
37+
(intrinsic.RegExpTestIntrinsic.test):
38+
* Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-Combined.js-result:
39+
* Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-Separate.js-result:
40+
* Scripts/tests/builtins/expected/JavaScriptCore-Builtin.prototype-Combined.js-result:
41+
* Scripts/tests/builtins/expected/JavaScriptCore-Builtin.prototype-Separate.js-result:
42+
* Scripts/tests/builtins/expected/JavaScriptCore-BuiltinConstructor-Combined.js-result:
43+
* Scripts/tests/builtins/expected/JavaScriptCore-BuiltinConstructor-Separate.js-result:
44+
* Scripts/tests/builtins/expected/JavaScriptCore-InternalClashingNames-Combined.js-result:
45+
* Scripts/tests/builtins/expected/WebCore-AnotherGuardedInternalBuiltin-Separate.js-result:
46+
* Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result:
47+
* Scripts/tests/builtins/expected/WebCore-GuardedBuiltin-Separate.js-result:
48+
* Scripts/tests/builtins/expected/WebCore-GuardedInternalBuiltin-Separate.js-result:
49+
* Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result:
50+
* Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:
51+
* builtins/BuiltinExecutables.cpp:
52+
(JSC::BuiltinExecutables::BuiltinExecutables):
53+
* builtins/BuiltinExecutables.h:
54+
* builtins/FunctionPrototype.js:
55+
(symbolHasInstance): Deleted.
56+
* builtins/GlobalOperations.js:
57+
(globalPrivate.speciesGetter): Deleted.
58+
* builtins/IteratorPrototype.js:
59+
(symbolIteratorGetter): Deleted.
60+
* builtins/RegExpPrototype.js:
61+
(match): Deleted.
62+
(replace): Deleted.
63+
(search): Deleted.
64+
(split): Deleted.
65+
* jsc.cpp:
66+
(functionCreateBuiltin):
67+
* runtime/FunctionPrototype.cpp:
68+
(JSC::FunctionPrototype::addFunctionProperties):
69+
* runtime/IteratorPrototype.cpp:
70+
(JSC::IteratorPrototype::finishCreation):
71+
* runtime/JSFunction.cpp:
72+
(JSC::JSFunction::getOwnNonIndexPropertyNames):
73+
(JSC::JSFunction::reifyLazyBoundNameIfNeeded):
74+
(JSC::JSFunction::createBuiltinFunction): Deleted.
75+
* runtime/JSFunction.h:
76+
* runtime/JSGlobalObject.cpp:
77+
(JSC::JSGlobalObject::init):
78+
* runtime/JSObject.cpp:
79+
(JSC::JSObject::putDirectBuiltinFunction):
80+
(JSC::JSObject::putDirectBuiltinFunctionWithoutTransition):
81+
* runtime/JSTypedArrayViewPrototype.cpp:
82+
(JSC::JSTypedArrayViewPrototype::finishCreation):
83+
* runtime/Lookup.cpp:
84+
(JSC::reifyStaticAccessor):
85+
* runtime/RegExpPrototype.cpp:
86+
(JSC::RegExpPrototype::finishCreation):
87+
188
2017-08-29 Saam Barati <sbarati@apple.com>
289

390
Throwing an exception in the DFG/FTL should not be a jettison-able OSR exit

‎Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,11 @@ def generate_section_for_code_table_macro(self):
123123
for function in self.model().all_functions():
124124
function_args = {
125125
'funcName': function.function_name,
126+
'overriddenName': function.overridden_name,
126127
'codeName': BuiltinsGenerator.mangledNameForFunction(function) + 'Code',
127128
}
128129

129-
lines.append(" macro(%(codeName)s, %(funcName)s, s_%(codeName)sLength) \\" % function_args)
130+
lines.append(" macro(%(codeName)s, %(funcName)s, %(overriddenName)s, s_%(codeName)sLength) \\" % function_args)
130131
return '\n'.join(lines)
131132

132133
def generate_section_for_code_name_macro(self):

‎Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_implementation.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ def generate_secondary_header_includes(self):
8989
(["JavaScriptCore", "WebCore"],
9090
("JavaScriptCore", "runtime/VM.h"),
9191
),
92+
(["JavaScriptCore", "WebCore"],
93+
("JavaScriptCore", "runtime/IdentifierInlines.h"),
94+
),
9295
(["JavaScriptCore", "WebCore"],
9396
("JavaScriptCore", "runtime/Intrinsic.h"),
9497
),

‎Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_header.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,11 @@ def generate_section_for_code_table_macro(self):
172172
for function in self.object.functions:
173173
function_args = {
174174
'funcName': function.function_name,
175+
'overriddenName': function.overridden_name,
175176
'codeName': BuiltinsGenerator.mangledNameForFunction(function) + 'Code',
176177
}
177178

178-
lines.append(" macro(%(codeName)s, %(funcName)s, s_%(codeName)sLength) \\" % function_args)
179+
lines.append(" macro(%(codeName)s, %(funcName)s, %(overriddenName)s, s_%(codeName)sLength) \\" % function_args)
179180
return '\n'.join(lines)
180181

181182
def generate_section_for_code_name_macro(self):

0 commit comments

Comments
 (0)