Diff
Modified: trunk/JSTests/ChangeLog (269800 => 269801)
--- trunk/JSTests/ChangeLog 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/JSTests/ChangeLog 2020-11-13 22:32:01 UTC (rev 269801)
@@ -1,3 +1,19 @@
+2020-11-13 Xan López <[email protected]>
+
+ [JSC] Use symbols as identifiers for class fields computed names storage
+ https://bugs.webkit.org/show_bug.cgi?id=216172
+
+ Reviewed by Yusuke Suzuki.
+
+ Use private symbols for the property keys of the class fields with
+ computed names. This is cleaner than using raw numeric identifiers and
+ will be less cumbersome when we add static fields. It also prevents
+ potential collisions if other features want to store data in the class
+ scope.
+
+ * stress/class-fields-harmony.js: new test, make sure
+ setFunctionName works properly with computed fields.
+
2020-11-13 Yusuke Suzuki <[email protected]>
Unreviewed, skip new ICU related tests in MIPS and ARMv7 and adjust it for ICU 65
Modified: trunk/JSTests/stress/class-fields-harmony.js (269800 => 269801)
--- trunk/JSTests/stress/class-fields-harmony.js 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/JSTests/stress/class-fields-harmony.js 2020-11-13 22:32:01 UTC (rev 269801)
@@ -906,3 +906,24 @@
assertSame(thisInConstructor, c);
}
+// Additional tests by the WebKit project.
+
+{
+ let x = 0;
+ let y = 'foo';
+ let z = { name: 'test' };
+
+ let C = class {
+ [x] = () => {
+ return 2;
+ };
+ [y] = class {};
+ [z] = class D {};
+ }
+
+ let c = new C();
+ assertSame(c[x](), 2);
+ assertSame(c[x].name, '0');
+ assertSame(c[y].name, 'foo');
+ assertSame(c[z].name, 'D');
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (269800 => 269801)
--- trunk/Source/_javascript_Core/ChangeLog 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-11-13 22:32:01 UTC (rev 269801)
@@ -1,3 +1,32 @@
+2020-11-13 Xan López <[email protected]>
+
+ [JSC] Use symbols as identifiers for class fields computed names storage
+ https://bugs.webkit.org/show_bug.cgi?id=216172
+
+ Reviewed by Yusuke Suzuki.
+
+ Use private symbols for the property keys of the class fields with
+ computed names. This is cleaner than using raw numeric identifiers and
+ will be less cumbersome when we add static fields. It also prevents
+ potential collisions if other features want to store data in the class
+ scope.
+
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::PropertyListNode::emitSaveComputedFieldName): adapt a comment.
+ * parser/Parser.cpp:
+ (JSC::Parser<LexerType>::parseClass): use private identifiers for computed fields property keys.
+ (JSC::Parser<LexerType>::parseInstanceFieldInitializerSourceElements): ditto.
+ * parser/ParserArena.cpp:
+ (JSC::IdentifierArena::makePrivateIdentifier): method to create a private identifier.
+ * parser/ParserArena.h:
+ * runtime/CachedTypes.cpp:
+ (JSC::CachedUniquedStringImplBase::encode): consider registered symbols, they are used by the parser now.
+ (JSC::CachedUniquedStringImplBase::decode const): ditto.
+ * runtime/VM.cpp:
+ (JSC::VM::VM):
+ * runtime/VM.h:
+ (JSC::VM::privateSymbolRegistry): create a private symbol registry too.
+
2020-11-13 Sergey Rubanov <[email protected]>
WebAssembly: opcodes for table.grow and table.size are mixed up
Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (269800 => 269801)
--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2020-11-13 22:32:01 UTC (rev 269801)
@@ -802,7 +802,7 @@
ASSERT(node.isComputedClassField());
RefPtr<RegisterID> propertyExpr;
- // The 'name' refers to a synthetic numeric variable name in the private name scope, where the property key is saved for later use.
+ // The 'name' refers to a synthetic private name in the class scope, where the property key is saved for later use.
const Identifier& description = *node.name();
Variable var = generator.variable(description);
ASSERT(!var.local());
@@ -4817,12 +4817,14 @@
void DefineFieldNode::emitBytecode(BytecodeGenerator& generator, RegisterID*)
{
RefPtr<RegisterID> value = generator.newTemporary();
+ bool shouldSetFunctionName = false;
if (!m_assign)
generator.emitLoad(value.get(), jsUndefined());
else {
generator.emitNode(value.get(), m_assign);
- if (m_ident && generator.shouldSetFunctionName(m_assign))
+ shouldSetFunctionName = generator.shouldSetFunctionName(m_assign);
+ if (m_ident && shouldSetFunctionName && m_type != DefineFieldNode::Type::ComputedName)
generator.emitSetFunctionName(value.get(), *m_ident);
}
@@ -4850,7 +4852,7 @@
// https://bugs.webkit.org/show_bug.cgi?id=198330
// For ComputedNames, the _expression_ has already been evaluated earlier during evaluation of a ClassExprNode.
- // Here, `m_ident` refers to an integer ID in a class lexical scope, containing the value already converted to an _expression_.
+ // Here, `m_ident` refers to private symbol ID in a class lexical scope, containing the value already converted to an _expression_.
Variable var = generator.variable(*m_ident);
ASSERT_WITH_MESSAGE(!var.local(), "Computed names must be stored in captured variables");
@@ -4858,6 +4860,8 @@
RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
RefPtr<RegisterID> privateName = generator.newTemporary();
generator.emitGetFromScope(privateName.get(), scope.get(), var, ThrowIfNotFound);
+ if (shouldSetFunctionName)
+ generator.emitSetFunctionName(value.get(), privateName.get());
generator.emitProfileType(privateName.get(), var, m_position, m_position + m_ident->length());
generator.emitCallDefineProperty(generator.thisRegister(), privateName.get(), value.get(), nullptr, nullptr, BytecodeGenerator::PropertyConfigurable | BytecodeGenerator::PropertyWritable | BytecodeGenerator::PropertyEnumerable, m_position);
break;
Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (269800 => 269801)
--- trunk/Source/_javascript_Core/parser/Parser.cpp 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp 2020-11-13 22:32:01 UTC (rev 269801)
@@ -2837,6 +2837,8 @@
return context.createClassDeclStatement(location, classExpr, classStart, classEnd, classStartLine, classEndLine);
}
+static constexpr ASCIILiteral instanceComputedNamePrefix { "instanceComputedName"_s };
+
template <typename LexerType>
template <class TreeBuilder> TreeClassExpression Parser<LexerType>::parseClass(TreeBuilder& context, FunctionNameRequirements requirements, ParserClassInfo<TreeBuilder>& info)
{
@@ -2996,7 +2998,7 @@
}
if (computedPropertyName) {
- ident = &m_parserArena.identifierArena().makeNumericIdentifier(m_vm, numComputedFields++);
+ ident = &m_parserArena.identifierArena().makePrivateIdentifier(m_vm, instanceComputedNamePrefix, numComputedFields++);
DeclarationResultMask declarationResult = classScope->declareLexicalVariable(ident, true);
ASSERT_UNUSED(declarationResult, declarationResult == DeclarationResult::Valid);
classScope->useVariable(ident, false);
@@ -3077,7 +3079,6 @@
JSTokenLocation fieldLocation = tokenLocation();
const Identifier* ident = nullptr;
- TreeExpression computedPropertyName = 0;
DefineFieldNode::Type type = DefineFieldNode::Type::Name;
switch (m_token.m_type) {
case PRIVATENAME:
@@ -3101,14 +3102,15 @@
ASSERT(ident);
next();
break;
- case OPENBRACKET:
+ case OPENBRACKET: {
next();
- computedPropertyName = parseAssignmentExpression(context);
+ TreeExpression computedPropertyName = parseAssignmentExpression(context);
failIfFalse(computedPropertyName, "Cannot parse computed property name");
handleProductionOrFail(CLOSEBRACKET, "]", "end", "computed property name");
- ident = &m_parserArena.identifierArena().makeNumericIdentifier(m_vm, numComputedFields++);
+ ident = &m_parserArena.identifierArena().makePrivateIdentifier(m_vm, instanceComputedNamePrefix, numComputedFields++);
type = DefineFieldNode::Type::ComputedName;
break;
+ }
default:
if (m_token.m_type & KeywordTokenFlag)
goto namedKeyword;
Modified: trunk/Source/_javascript_Core/parser/ParserArena.cpp (269800 => 269801)
--- trunk/Source/_javascript_Core/parser/ParserArena.cpp 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/_javascript_Core/parser/ParserArena.cpp 2020-11-13 22:32:01 UTC (rev 269801)
@@ -107,4 +107,12 @@
return m_identifiers.last();
}
+const Identifier& IdentifierArena::makePrivateIdentifier(VM& vm, ASCIILiteral prefix, unsigned identifier)
+{
+ String symbolName = makeString(prefix, identifier);
+ auto symbol = vm.privateSymbolRegistry().symbolForKey(symbolName);
+ m_identifiers.append(Identifier::fromUid(symbol));
+ return m_identifiers.last();
}
+
+}
Modified: trunk/Source/_javascript_Core/parser/ParserArena.h (269800 => 269801)
--- trunk/Source/_javascript_Core/parser/ParserArena.h 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/_javascript_Core/parser/ParserArena.h 2020-11-13 22:32:01 UTC (rev 269801)
@@ -52,6 +52,7 @@
const Identifier& makeBigIntDecimalIdentifier(VM&, const Identifier&, uint8_t radix);
const Identifier& makeNumericIdentifier(VM&, double number);
+ const Identifier& makePrivateIdentifier(VM&, ASCIILiteral, unsigned);
public:
static const int MaximumCachableCharacter = 128;
Modified: trunk/Source/_javascript_Core/runtime/CachedTypes.cpp (269800 => 269801)
--- trunk/Source/_javascript_Core/runtime/CachedTypes.cpp 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/_javascript_Core/runtime/CachedTypes.cpp 2020-11-13 22:32:01 UTC (rev 269801)
@@ -702,14 +702,18 @@
{
m_isAtomic = string.isAtom();
m_isSymbol = string.isSymbol();
+ m_isRegistered = false;
m_isWellKnownSymbol = false;
+ m_isPrivate = false;
RefPtr<StringImpl> impl = const_cast<StringImpl*>(&string);
if (m_isSymbol) {
SymbolImpl* symbol = static_cast<SymbolImpl*>(impl.get());
+ m_isRegistered = symbol->isRegistered();
+ m_isPrivate = symbol->isPrivate();
if (!symbol->isNullSymbol()) {
// We have special handling for well-known symbols.
- if (!symbol->isPrivate()) {
+ if (!m_isPrivate) {
m_isWellKnownSymbol = true;
impl = symbol->substring(strlen("Symbol."));
}
@@ -742,10 +746,17 @@
return AtomStringImpl::add(buffer, m_length).leakRef();
SymbolImpl* symbol;
- if (m_isWellKnownSymbol)
- symbol = decoder.vm().propertyNames->builtinNames().lookUpWellKnownSymbol(buffer, m_length);
+ VM& vm = decoder.vm();
+ if (m_isRegistered) {
+ String str(buffer, m_length);
+ if (m_isPrivate)
+ symbol = static_cast<SymbolImpl*>(&vm.privateSymbolRegistry().symbolForKey(str).leakRef());
+ else
+ symbol = static_cast<SymbolImpl*>(&vm.symbolRegistry().symbolForKey(str).leakRef());
+ } else if (m_isWellKnownSymbol)
+ symbol = vm.propertyNames->builtinNames().lookUpWellKnownSymbol(buffer, m_length);
else
- symbol = decoder.vm().propertyNames->builtinNames().lookUpPrivateName(buffer, m_length);
+ symbol = vm.propertyNames->builtinNames().lookUpPrivateName(buffer, m_length);
RELEASE_ASSERT(symbol);
String str = symbol;
StringImpl* impl = str.releaseImpl().get();
@@ -773,6 +784,8 @@
bool m_isSymbol : 1;
bool m_isWellKnownSymbol : 1;
bool m_isAtomic : 1;
+ bool m_isRegistered : 1;
+ bool m_isPrivate : 1;
unsigned m_length;
};
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (269800 => 269801)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2020-11-13 22:32:01 UTC (rev 269801)
@@ -382,6 +382,7 @@
, topCallFrame(CallFrame::noCaller())
, deferredWorkTimer(DeferredWorkTimer::create(*this))
, m_atomStringTable(vmType == Default ? Thread::current().atomStringTable() : new AtomStringTable)
+ , m_privateSymbolRegistry(WTF::SymbolRegistry::Type::PrivateSymbol)
, propertyNames(nullptr)
, emptyList(new ArgList)
, machineCodeBytesPerBytecodeWordForBaselineJIT(makeUnique<SimpleStats>())
Modified: trunk/Source/_javascript_Core/runtime/VM.h (269800 => 269801)
--- trunk/Source/_javascript_Core/runtime/VM.h 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2020-11-13 22:32:01 UTC (rev 269801)
@@ -714,6 +714,7 @@
AtomStringTable* m_atomStringTable;
WTF::SymbolRegistry m_symbolRegistry;
+ WTF::SymbolRegistry m_privateSymbolRegistry;
CommonIdentifiers* propertyNames;
const ArgList* emptyList;
SmallStrings smallStrings;
@@ -725,6 +726,7 @@
AtomStringTable* atomStringTable() const { return m_atomStringTable; }
WTF::SymbolRegistry& symbolRegistry() { return m_symbolRegistry; }
+ WTF::SymbolRegistry& privateSymbolRegistry() { return m_privateSymbolRegistry; }
Strong<JSBigInt> heapBigIntConstantOne;
Modified: trunk/Source/WTF/ChangeLog (269800 => 269801)
--- trunk/Source/WTF/ChangeLog 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/WTF/ChangeLog 2020-11-13 22:32:01 UTC (rev 269801)
@@ -1,3 +1,24 @@
+2020-11-13 Xan López <[email protected]>
+
+ [JSC] Use symbols as identifiers for class fields computed names storage
+ https://bugs.webkit.org/show_bug.cgi?id=216172
+
+ Reviewed by Yusuke Suzuki.
+
+ Use private symbols for the property keys of the class fields with
+ computed names. This is cleaner than using raw numeric identifiers and
+ will be less cumbersome when we add static fields. It also prevents
+ potential collisions if other features want to store data in the class
+ scope.
+
+ * wtf/text/SymbolImpl.cpp:
+ (WTF::RegisteredSymbolImpl::createPrivate): add a method to create a registered private symbol from a string key.
+ * wtf/text/SymbolImpl.h:
+ * wtf/text/SymbolRegistry.cpp:
+ (WTF::SymbolRegistry::symbolForKey): consider that we can hold private symbols now too.
+ * wtf/text/SymbolRegistry.h:
+ (WTF::SymbolRegistry::SymbolRegistry): new enum type for public/private symbols.
+
2020-11-12 Darin Adler <[email protected]>
Remove unused advanced plug-in features: snapshotting and plug-in load policy
Modified: trunk/Source/WTF/wtf/text/SymbolImpl.cpp (269800 => 269801)
--- trunk/Source/WTF/wtf/text/SymbolImpl.cpp 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/WTF/wtf/text/SymbolImpl.cpp 2020-11-13 22:32:01 UTC (rev 269801)
@@ -79,4 +79,13 @@
return adoptRef(*new RegisteredSymbolImpl(rep.m_data16, rep.length(), *ownerRep, symbolRegistry));
}
+Ref<RegisteredSymbolImpl> RegisteredSymbolImpl::createPrivate(StringImpl& rep, SymbolRegistry& symbolRegistry)
+{
+ auto* ownerRep = (rep.bufferOwnership() == BufferSubstring) ? rep.substringBuffer() : &rep;
+ ASSERT(ownerRep->bufferOwnership() != BufferSubstring);
+ if (rep.is8Bit())
+ return adoptRef(*new RegisteredSymbolImpl(rep.m_data8, rep.length(), *ownerRep, symbolRegistry, s_flagIsRegistered | s_flagIsPrivate));
+ return adoptRef(*new RegisteredSymbolImpl(rep.m_data16, rep.length(), *ownerRep, symbolRegistry, s_flagIsRegistered | s_flagIsPrivate));
+}
+
} // namespace WTF
Modified: trunk/Source/WTF/wtf/text/SymbolImpl.h (269800 => 269801)
--- trunk/Source/WTF/wtf/text/SymbolImpl.h 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/WTF/wtf/text/SymbolImpl.h 2020-11-13 22:32:01 UTC (rev 269801)
@@ -156,15 +156,16 @@
void clearSymbolRegistry() { m_symbolRegistry = nullptr; }
static Ref<RegisteredSymbolImpl> create(StringImpl& rep, SymbolRegistry&);
+ static Ref<RegisteredSymbolImpl> createPrivate(StringImpl& rep, SymbolRegistry&);
- RegisteredSymbolImpl(const LChar* characters, unsigned length, Ref<StringImpl>&& base, SymbolRegistry& registry)
- : SymbolImpl(characters, length, WTFMove(base), s_flagIsRegistered)
+ RegisteredSymbolImpl(const LChar* characters, unsigned length, Ref<StringImpl>&& base, SymbolRegistry& registry, Flags flags = s_flagIsRegistered)
+ : SymbolImpl(characters, length, WTFMove(base), flags)
, m_symbolRegistry(®istry)
{
}
- RegisteredSymbolImpl(const UChar* characters, unsigned length, Ref<StringImpl>&& base, SymbolRegistry& registry)
- : SymbolImpl(characters, length, WTFMove(base), s_flagIsRegistered)
+ RegisteredSymbolImpl(const UChar* characters, unsigned length, Ref<StringImpl>&& base, SymbolRegistry& registry, Flags flags = s_flagIsRegistered)
+ : SymbolImpl(characters, length, WTFMove(base), flags)
, m_symbolRegistry(®istry)
{
}
Modified: trunk/Source/WTF/wtf/text/SymbolRegistry.cpp (269800 => 269801)
--- trunk/Source/WTF/wtf/text/SymbolRegistry.cpp 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/WTF/wtf/text/SymbolRegistry.cpp 2020-11-13 22:32:01 UTC (rev 269801)
@@ -28,6 +28,11 @@
namespace WTF {
+SymbolRegistry::SymbolRegistry(Type type)
+ : m_symbolType(type)
+{
+}
+
SymbolRegistry::~SymbolRegistry()
{
for (auto& key : m_table) {
@@ -44,9 +49,14 @@
return *static_cast<SymbolImpl*>(addResult.iterator->impl())->asRegisteredSymbolImpl();
}
- auto symbol = RegisteredSymbolImpl::create(*rep.impl(), *this);
- *addResult.iterator = SymbolRegistryKey(&symbol.get());
- return symbol;
+ RefPtr<RegisteredSymbolImpl> symbol;
+ if (m_symbolType == Type::PrivateSymbol)
+ symbol = RegisteredSymbolImpl::createPrivate(*rep.impl(), *this);
+ else
+ symbol = RegisteredSymbolImpl::create(*rep.impl(), *this);
+
+ *addResult.iterator = SymbolRegistryKey(symbol.get());
+ return symbol.releaseNonNull();
}
void SymbolRegistry::remove(RegisteredSymbolImpl& uid)
Modified: trunk/Source/WTF/wtf/text/SymbolRegistry.h (269800 => 269801)
--- trunk/Source/WTF/wtf/text/SymbolRegistry.h 2020-11-13 22:29:34 UTC (rev 269800)
+++ trunk/Source/WTF/wtf/text/SymbolRegistry.h 2020-11-13 22:32:01 UTC (rev 269801)
@@ -78,7 +78,8 @@
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(SymbolRegistry);
public:
- SymbolRegistry() = default;
+ enum class Type : uint8_t { PublicSymbol, PrivateSymbol };
+ WTF_EXPORT_PRIVATE SymbolRegistry(Type = Type::PublicSymbol);
WTF_EXPORT_PRIVATE ~SymbolRegistry();
WTF_EXPORT_PRIVATE Ref<RegisteredSymbolImpl> symbolForKey(const String&);
@@ -87,6 +88,7 @@
private:
HashSet<SymbolRegistryKey> m_table;
+ Type m_symbolType;
};
inline SymbolRegistryKey::SymbolRegistryKey(StringImpl* uid)
