Skip to content

Commit e40702b

Browse files
committed
Support Ergonomic Brand Checks proposal (#x in obj)
https://bugs.webkit.org/show_bug.cgi?id=221093 Reviewed by Caio Araujo Neponoceno de Lima. JSTests: * stress/private-in.js: Added. * test262/config.yaml: Add feature flag. Source/JavaScriptCore: This patch implements the following Stage 3 proposal (behind a runtime option): https://github.com/tc39/proposal-private-fields-in-in Specifically, it extends the `in` keyword to allow the LHS to be a private name, thereby allowing users to implement Array.isArray-esque brand checks for their own classes *without* having to wrap a private member get in a try-catch. For example: ``` class C { #x; static isC(obj) { return #x in obj; } } ``` This is done by adding two new bytecode ops, HasPrivateName and HasPrivateBrand. For the moment, these are implemented without fast paths, as we should do so for InByVal first and then have these follow suit. * bytecode/BytecodeList.rb: * bytecode/BytecodeUseDef.cpp: (JSC::computeUsesForBytecodeIndexImpl): (JSC::computeDefsForBytecodeIndexImpl): * bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::emitHasPrivateName): (JSC::BytecodeGenerator::emitHasPrivateBrand): (JSC::BytecodeGenerator::emitCheckPrivateBrand): * bytecompiler/BytecodeGenerator.h: * bytecompiler/NodesCodegen.cpp: (JSC::InNode::emitBytecode): * dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::parseBlock): * dfg/DFGCapabilities.cpp: (JSC::DFG::capabilityLevel): * dfg/DFGClobberize.h: (JSC::DFG::clobberize): * dfg/DFGDoesGC.cpp: (JSC::DFG::doesGC): * dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupNode): * dfg/DFGNodeType.h: * dfg/DFGPredictionPropagationPhase.cpp: * dfg/DFGSafeToExecute.h: (JSC::DFG::safeToExecute): * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compileHasPrivateName): (JSC::DFG::SpeculativeJIT::compileHasPrivateBrand): * dfg/DFGSpeculativeJIT.h: * dfg/DFGSpeculativeJIT32_64.cpp: (JSC::DFG::SpeculativeJIT::compile): * dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): * ftl/FTLCapabilities.cpp: (JSC::FTL::canCompile): * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileNode): (JSC::FTL::DFG::LowerDFGToB3::compileHasPrivateName): (JSC::FTL::DFG::LowerDFGToB3::compileHasPrivateBrand): * jit/JIT.cpp: (JSC::JIT::privateCompileMainPass): * jit/JITOperations.cpp: (JSC::JSC_DEFINE_JIT_OPERATION): * jit/JITOperations.h: * llint/LowLevelInterpreter.asm: * parser/ASTBuilder.h: (JSC::ASTBuilder::createPrivateIdentifierNode): * parser/NodeConstructors.h: (JSC::PrivateIdentifierNode::PrivateIdentifierNode): * parser/Nodes.h: (JSC::ExpressionNode::isPrivateIdentifier const): * parser/Parser.cpp: (JSC::Parser<LexerType>::parseBinaryExpression): * parser/SyntaxChecker.h: (JSC::SyntaxChecker::createPrivateIdentifierNode): * parser/VariableEnvironment.h: * runtime/CommonSlowPaths.cpp: (JSC::JSC_DEFINE_COMMON_SLOW_PATH): * runtime/CommonSlowPaths.h: * runtime/JSObject.h: * runtime/JSObjectInlines.h: (JSC::JSObject::hasPrivateField): (JSC::JSObject::hasPrivateBrand): (JSC::JSObject::checkPrivateBrand): * runtime/OptionsList.h: Canonical link: https://commits.webkit.org/238057@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@277926 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent c2761b2 commit e40702b

39 files changed

Lines changed: 496 additions & 29 deletions

JSTests/ChangeLog

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
2021-05-22 Ross Kirsling <ross.kirsling@sony.com>
2+
3+
Support Ergonomic Brand Checks proposal (`#x in obj`)
4+
https://bugs.webkit.org/show_bug.cgi?id=221093
5+
6+
Reviewed by Caio Araujo Neponoceno de Lima.
7+
8+
* stress/private-in.js: Added.
9+
* test262/config.yaml: Add feature flag.
10+
111
2021-05-21 Angelos Oikonomopoulos <angelos@igalia.com>
212

313
Unskip type-check-hoisting-phase-hoist-check-structure-on-tdz-this-value on MIPS

JSTests/stress/private-in.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//@ requireOptions("--usePrivateIn=1")
2+
3+
function assert(b) {
4+
if (!b) throw new Error;
5+
}
6+
7+
function shouldThrowTypeError(func) {
8+
let error;
9+
try {
10+
func();
11+
} catch (e) {
12+
error = e;
13+
}
14+
15+
if (!(error instanceof TypeError))
16+
throw new Error('Expected TypeError!');
17+
}
18+
19+
class F { #x; static isF(obj) { return #x in obj; } }
20+
class SF { static #x; static isSF(obj) { return #x in obj; } }
21+
22+
class M { #x() {} static isM(obj) { return #x in obj; } }
23+
class SM { static #x() {} static isSM(obj) { return #x in obj; } }
24+
25+
class G { get #x() {} static isG(obj) { return #x in obj; } }
26+
class SG { static get #x() {} static isSG(obj) { return #x in obj; } }
27+
28+
class S { set #x(v) {} static isS(obj) { return #x in obj; } }
29+
class SS { static set #x(v) {} static isSS(obj) { return #x in obj; } }
30+
31+
function test() {
32+
assert(F.isF(new F));
33+
assert(!F.isF(F) && !F.isF(new M) && !F.isF({}));
34+
shouldThrowTypeError(() => F.isF(3));
35+
assert(SF.isSF(SF));
36+
assert(!SF.isSF(new SF) && !SF.isSF(SM) && !SF.isSF({}));
37+
shouldThrowTypeError(() => SF.isSF(3));
38+
39+
assert(M.isM(new M));
40+
assert(!M.isM(M) && !M.isM(new F) && !M.isM({}));
41+
shouldThrowTypeError(() => M.isM(3));
42+
assert(SM.isSM(SM));
43+
assert(!SM.isSM(new SM) && !SM.isSM(SF) && !SM.isSM({}));
44+
shouldThrowTypeError(() => SM.isSM(3));
45+
46+
assert(G.isG(new G));
47+
assert(!G.isG(G) && !G.isG(new F) && !G.isG({}));
48+
shouldThrowTypeError(() => G.isG(3));
49+
assert(SG.isSG(SG));
50+
assert(!SG.isSG(new SG) && !SG.isSG(SF) && !SG.isSG({}));
51+
shouldThrowTypeError(() => SG.isSG(3));
52+
53+
assert(S.isS(new S));
54+
assert(!S.isS(S) && !S.isS(new F) && !S.isS({}));
55+
shouldThrowTypeError(() => S.isS(3));
56+
assert(SS.isSS(SS));
57+
assert(!SS.isSS(new SS) && !SS.isSS(SF) && !SS.isSS({}));
58+
shouldThrowTypeError(() => SS.isSS(3));
59+
}
60+
noInline(test);
61+
62+
for (let i = 0; i < 10000; i++)
63+
test();

JSTests/test262/config.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ flags:
55
FinalizationRegistry: useWeakRefs
66
class-fields-private: usePrivateClassFields
77
class-methods-private: usePrivateMethods
8+
class-fields-private-in: usePrivateIn
89
class-static-fields-public: usePublicStaticClassFields
910
class-static-fields-private: usePrivateStaticClassFields
1011
class-static-methods-private: usePrivateMethods
@@ -23,8 +24,6 @@ skip:
2324
- regexp-lookbehind
2425
- legacy-regexp
2526
- cleanupSome
26-
# https://bugs.webkit.org/show_bug.cgi?id=221093
27-
- class-fields-private-in
2827
paths:
2928
files:
3029
# Slightly different formatting. We should update test262 side.

Source/JavaScriptCore/ChangeLog

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,96 @@
1+
2021-05-22 Ross Kirsling <ross.kirsling@sony.com>
2+
3+
Support Ergonomic Brand Checks proposal (`#x in obj`)
4+
https://bugs.webkit.org/show_bug.cgi?id=221093
5+
6+
Reviewed by Caio Araujo Neponoceno de Lima.
7+
8+
This patch implements the following Stage 3 proposal (behind a runtime option):
9+
https://github.com/tc39/proposal-private-fields-in-in
10+
11+
Specifically, it extends the `in` keyword to allow the LHS to be a private name,
12+
thereby allowing users to implement Array.isArray-esque brand checks for their own classes
13+
*without* having to wrap a private member get in a try-catch.
14+
15+
For example:
16+
```
17+
class C {
18+
#x;
19+
static isC(obj) { return #x in obj; }
20+
}
21+
```
22+
23+
This is done by adding two new bytecode ops, HasPrivateName and HasPrivateBrand. For the moment,
24+
these are implemented without fast paths, as we should do so for InByVal first and then have these follow suit.
25+
26+
* bytecode/BytecodeList.rb:
27+
* bytecode/BytecodeUseDef.cpp:
28+
(JSC::computeUsesForBytecodeIndexImpl):
29+
(JSC::computeDefsForBytecodeIndexImpl):
30+
* bytecompiler/BytecodeGenerator.cpp:
31+
(JSC::BytecodeGenerator::emitHasPrivateName):
32+
(JSC::BytecodeGenerator::emitHasPrivateBrand):
33+
(JSC::BytecodeGenerator::emitCheckPrivateBrand):
34+
* bytecompiler/BytecodeGenerator.h:
35+
* bytecompiler/NodesCodegen.cpp:
36+
(JSC::InNode::emitBytecode):
37+
* dfg/DFGAbstractInterpreterInlines.h:
38+
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
39+
* dfg/DFGByteCodeParser.cpp:
40+
(JSC::DFG::ByteCodeParser::parseBlock):
41+
* dfg/DFGCapabilities.cpp:
42+
(JSC::DFG::capabilityLevel):
43+
* dfg/DFGClobberize.h:
44+
(JSC::DFG::clobberize):
45+
* dfg/DFGDoesGC.cpp:
46+
(JSC::DFG::doesGC):
47+
* dfg/DFGFixupPhase.cpp:
48+
(JSC::DFG::FixupPhase::fixupNode):
49+
* dfg/DFGNodeType.h:
50+
* dfg/DFGPredictionPropagationPhase.cpp:
51+
* dfg/DFGSafeToExecute.h:
52+
(JSC::DFG::safeToExecute):
53+
* dfg/DFGSpeculativeJIT.cpp:
54+
(JSC::DFG::SpeculativeJIT::compileHasPrivateName):
55+
(JSC::DFG::SpeculativeJIT::compileHasPrivateBrand):
56+
* dfg/DFGSpeculativeJIT.h:
57+
* dfg/DFGSpeculativeJIT32_64.cpp:
58+
(JSC::DFG::SpeculativeJIT::compile):
59+
* dfg/DFGSpeculativeJIT64.cpp:
60+
(JSC::DFG::SpeculativeJIT::compile):
61+
* ftl/FTLCapabilities.cpp:
62+
(JSC::FTL::canCompile):
63+
* ftl/FTLLowerDFGToB3.cpp:
64+
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
65+
(JSC::FTL::DFG::LowerDFGToB3::compileHasPrivateName):
66+
(JSC::FTL::DFG::LowerDFGToB3::compileHasPrivateBrand):
67+
* jit/JIT.cpp:
68+
(JSC::JIT::privateCompileMainPass):
69+
* jit/JITOperations.cpp:
70+
(JSC::JSC_DEFINE_JIT_OPERATION):
71+
* jit/JITOperations.h:
72+
* llint/LowLevelInterpreter.asm:
73+
* parser/ASTBuilder.h:
74+
(JSC::ASTBuilder::createPrivateIdentifierNode):
75+
* parser/NodeConstructors.h:
76+
(JSC::PrivateIdentifierNode::PrivateIdentifierNode):
77+
* parser/Nodes.h:
78+
(JSC::ExpressionNode::isPrivateIdentifier const):
79+
* parser/Parser.cpp:
80+
(JSC::Parser<LexerType>::parseBinaryExpression):
81+
* parser/SyntaxChecker.h:
82+
(JSC::SyntaxChecker::createPrivateIdentifierNode):
83+
* parser/VariableEnvironment.h:
84+
* runtime/CommonSlowPaths.cpp:
85+
(JSC::JSC_DEFINE_COMMON_SLOW_PATH):
86+
* runtime/CommonSlowPaths.h:
87+
* runtime/JSObject.h:
88+
* runtime/JSObjectInlines.h:
89+
(JSC::JSObject::hasPrivateField):
90+
(JSC::JSObject::hasPrivateBrand):
91+
(JSC::JSObject::checkPrivateBrand):
92+
* runtime/OptionsList.h:
93+
194
2021-05-22 Chris Dumez <cdumez@apple.com>
295

396
Replace LockHolder with Locker in local variables

Source/JavaScriptCore/bytecode/BytecodeList.rb

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,20 @@
454454
property: unsigned,
455455
}
456456

457+
op :has_private_name,
458+
args: {
459+
dst: VirtualRegister,
460+
base: VirtualRegister,
461+
property: VirtualRegister,
462+
}
463+
464+
op :has_private_brand,
465+
args: {
466+
dst: VirtualRegister,
467+
base: VirtualRegister,
468+
brand: VirtualRegister,
469+
}
470+
457471
op :get_by_id,
458472
args: {
459473
dst: VirtualRegister,
@@ -608,7 +622,6 @@
608622
structureID: StructureID,
609623
brand: WriteBarrier[JSCell],
610624
}
611-
612625

613626
op :put_by_val,
614627
args: {

Source/JavaScriptCore/bytecode/BytecodeUseDef.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,8 @@ void computeUsesForBytecodeIndexImpl(VirtualRegister scopeRegister, const Instru
232232
USES(OpSetPrivateBrand, base, brand)
233233
USES(OpCheckPrivateBrand, base, brand)
234234
USES(OpInByVal, base, property)
235+
USES(OpHasPrivateName, base, property)
236+
USES(OpHasPrivateBrand, base, brand)
235237
USES(OpOverridesHasInstance, constructor, hasInstanceValue)
236238
USES(OpInstanceof, value, prototype)
237239
USES(OpAdd, lhs, rhs)
@@ -509,6 +511,8 @@ void computeDefsForBytecodeIndexImpl(unsigned numVars, const Instruction* instru
509511
DEFS(OpIsConstructor, dst)
510512
DEFS(OpInById, dst)
511513
DEFS(OpInByVal, dst)
514+
DEFS(OpHasPrivateName, dst)
515+
DEFS(OpHasPrivateBrand, dst)
512516
DEFS(OpToNumber, dst)
513517
DEFS(OpToNumeric, dst)
514518
DEFS(OpToString, dst)

Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2724,6 +2724,12 @@ RegisterID* BytecodeGenerator::emitGetPrivateName(RegisterID* dst, RegisterID* b
27242724
return dst;
27252725
}
27262726

2727+
RegisterID* BytecodeGenerator::emitHasPrivateName(RegisterID* dst, RegisterID* base, RegisterID* property)
2728+
{
2729+
OpHasPrivateName::emit(this, dst, base, property);
2730+
return dst;
2731+
}
2732+
27272733
RegisterID* BytecodeGenerator::emitDirectPutByVal(RegisterID* base, RegisterID* property, RegisterID* value)
27282734
{
27292735
OpPutByValDirect::emit(this, base, property, value, ecmaMode());
@@ -2796,12 +2802,25 @@ RegisterID* BytecodeGenerator::emitPrivateFieldPut(RegisterID* base, RegisterID*
27962802
return value;
27972803
}
27982804

2805+
RegisterID* BytecodeGenerator::emitHasPrivateBrand(RegisterID* dst, RegisterID* base, RegisterID* brand, bool isStatic)
2806+
{
2807+
if (isStatic) {
2808+
Ref<Label> isObjectLabel = newLabel();
2809+
emitJumpIfTrue(emitIsObject(newTemporary(), base), isObjectLabel.get());
2810+
emitThrowTypeError("Cannot access static private method or accessor of a non-Object");
2811+
emitLabel(isObjectLabel.get());
2812+
emitEqualityOp<OpStricteq>(dst, base, brand);
2813+
} else
2814+
OpHasPrivateBrand::emit(this, dst, base, brand);
2815+
return dst;
2816+
}
2817+
27992818
void BytecodeGenerator::emitCheckPrivateBrand(RegisterID* base, RegisterID* brand, bool isStatic)
28002819
{
28012820
if (isStatic) {
28022821
Ref<Label> brandCheckOkLabel = newLabel();
28032822
emitJumpIfTrue(emitEqualityOp<OpStricteq>(newTemporary(), base, brand), brandCheckOkLabel.get());
2804-
emitThrowTypeError("Cannot access static private method or acessor");
2823+
emitThrowTypeError("Cannot access static private method or accessor");
28052824
emitLabel(brandCheckOkLabel.get());
28062825
return;
28072826

Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,12 +840,14 @@ namespace JSC {
840840
RegisterID* emitDefinePrivateField(RegisterID* base, RegisterID* property, RegisterID* value);
841841
RegisterID* emitPrivateFieldPut(RegisterID* base, RegisterID* property, RegisterID* value);
842842
RegisterID* emitGetPrivateName(RegisterID* dst, RegisterID* base, RegisterID* property);
843+
RegisterID* emitHasPrivateName(RegisterID* dst, RegisterID* base, RegisterID* property);
843844

844845
void emitCreatePrivateBrand(RegisterID* dst, const JSTextPosition& divot, const JSTextPosition& divotStart, const JSTextPosition& divotEnd);
845846
void emitInstallPrivateBrand(RegisterID* target);
846847
void emitInstallPrivateClassBrand(RegisterID* target);
847848

848849
RegisterID* emitGetPrivateBrand(RegisterID* dst, RegisterID* scope, bool isStatic);
850+
RegisterID* emitHasPrivateBrand(RegisterID* dst, RegisterID* base, RegisterID* brand, bool isStatic);
849851
void emitCheckPrivateBrand(RegisterID* base, RegisterID* brand, bool isStatic);
850852

851853
void emitSuperSamplerBegin();

Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3206,6 +3206,24 @@ RegisterID* InstanceOfNode::emitBytecode(BytecodeGenerator& generator, RegisterI
32063206

32073207
RegisterID* InNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
32083208
{
3209+
if (m_expr1->isPrivateIdentifier()) {
3210+
RefPtr<RegisterID> base = generator.emitNode(m_expr2);
3211+
3212+
auto identifier = static_cast<PrivateIdentifierNode*>(m_expr1)->value();
3213+
auto privateTraits = generator.getPrivateTraits(identifier);
3214+
Variable var = generator.variable(identifier);
3215+
RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
3216+
3217+
if (privateTraits.isField()) {
3218+
RefPtr<RegisterID> privateName = generator.emitGetFromScope(generator.newTemporary(), scope.get(), var, DoNotThrowIfNotFound);
3219+
return generator.emitHasPrivateName(generator.finalDestination(dst, base.get()), base.get(), privateName.get());
3220+
}
3221+
3222+
ASSERT(privateTraits.isPrivateMethodOrAccessor());
3223+
RefPtr<RegisterID> privateBrand = generator.emitGetPrivateBrand(generator.newTemporary(), scope.get(), privateTraits.isStatic());
3224+
return generator.emitHasPrivateBrand(generator.finalDestination(dst, base.get()), base.get(), privateBrand.get(), privateTraits.isStatic());
3225+
}
3226+
32093227
if (isNonIndexStringElement(*m_expr1)) {
32103228
RefPtr<RegisterID> base = generator.emitNode(m_expr2);
32113229
generator.emitExpressionInfo(divot(), divotStart(), divotEnd());

Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4212,6 +4212,15 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
42124212
break;
42134213
}
42144214

4215+
case HasPrivateName:
4216+
case HasPrivateBrand: {
4217+
clobberWorld();
4218+
filter(node->child1(), SpecObject);
4219+
filter(node->child2(), SpecSymbol);
4220+
setNonCellTypeForNode(node, SpecBoolean);
4221+
break;
4222+
}
4223+
42154224
case HasOwnProperty: {
42164225
clobberWorld();
42174226
setNonCellTypeForNode(node, SpecBoolean);

0 commit comments

Comments
 (0)