Attachment 373113 Details for Bug 194095 – Class fields static (private and public)

[patch] Class fields static (private and public)

class-fields-static-2806.diff (text/plain), 17.91 KB, created by Xan Lopez on 2019-06-28 06:47:54 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Xan Lopez

Created: 2019-06-28 06:47:54 PDT

Size: 17.91 KB

patch

obsolete

>From 014db76a4f391511f0d7bcfd8522b991ce5f08ae Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Xan=20L=C3=B3pez?= <xan@igalia.com>
>Date: Fri, 1 Feb 2019 10:08:23 +0100
>Subject: [PATCH] static fields: implement static public and private fields
>
>TODO:
>
>- The semantic documents for static fields require that the
>  constructor be available as 'this' during the static field
>  evaluation. We are doing this through a
>  setForScope<RegisterID>setThisToCtor(this, ctr) call, which requires
>  marking RegisterID as copyable. Unclear if it's a good idea.
>
>- Maybe refactor the parser so that methods and fields are in two
>  separate lists at code generation. This might simplify some nasty
>  code.
>---
> JSTests/test262/config.yaml                   |   2 -
> .../bytecompiler/NodesCodegen.cpp             | 137 ++++++++++++------
> .../JavaScriptCore/bytecompiler/RegisterID.h  |   2 -
> Source/JavaScriptCore/parser/Nodes.h          |   9 ++
> Source/JavaScriptCore/parser/Parser.cpp       |   5 +-
> Source/JavaScriptCore/runtime/JSFunction.cpp  |   1 -
> 6 files changed, 108 insertions(+), 48 deletions(-)
>
>diff --git a/JSTests/test262/config.yaml b/JSTests/test262/config.yaml
>index 8a5a99b2078..80a424ad8de 100644
>--- a/JSTests/test262/config.yaml
>+++ b/JSTests/test262/config.yaml
>@@ -15,8 +15,6 @@ skip:
>     # https://bugs.webkit.org/show_bug.cgi?id=174931
>     - regexp-lookbehind
>     - class-methods-private
>-    - class-static-fields-public
>-    - class-static-fields-private
>     - class-static-methods-private
>     - Intl.DateTimeFormat-datetimestyle
>     - Intl.DateTimeFormat-formatRange
>diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>index dce588c95d6..38b5d1b232d 100644
>--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>@@ -538,15 +538,90 @@ static inline void emitPutHomeObject(BytecodeGenerator& generator, RegisterID* f
>     generator.emitPutById(function, generator.propertyNames().builtinNames().homeObjectPrivateName(), homeObject);
> }
> 
>+static void emitClassField(BytecodeGenerator& generator, RegisterID* dst, ExpressionNode* assignNode, ExpressionNode* expressionNode, const Identifier* name, const JSTextPosition& position, DefineFieldNode::Type fieldType, ClassElementTag elementType)
>+{
>+    ASSERT (elementType == ClassElementTag::Instance || elementType == ClassElementTag::Static);
>+    RefPtr<RegisterID> value = generator.newTemporary();
>+
>+    if (!assignNode)
>+        generator.emitLoad(value.get(), jsUndefined());
>+    else {
>+        if (elementType == ClassElementTag::Static) {
>+            SetForScope<RegisterID>setThisToConstructor(*generator.thisRegister(), *dst);
>+            generator.emitNode(value.get(), assignNode);
>+        } else
>+            generator.emitNode(value.get(), assignNode);
>+        if (name && generator.shouldEmitSetFunctionName(assignNode))
>+            generator.emitSetFunctionName(value.get(), *name);
>+    }
>+
>+    switch (fieldType) {
>+    case DefineFieldNode::Name: {
>+        // FIXME: Improve performance of public class fields
>+        // https://bugs.webkit.org/show_bug.cgi?id=198330
>+        RefPtr<RegisterID> propertyName = generator.emitLoad(nullptr, *name);
>+        generator.emitCallDefineProperty(dst, propertyName.get(), value.get(), nullptr, nullptr, BytecodeGenerator::PropertyConfigurable | BytecodeGenerator::PropertyWritable | BytecodeGenerator::PropertyEnumerable, position);
>+        break;
>+    }
>+    case DefineFieldNode::PrivateName:
>+        generator.emitExpressionInfo(position, position, position + name->length());
>+        generator.emitPrivateFieldAdd(dst, *name, value.get());
>+        break;
>+    case DefineFieldNode::ComputedName: {
>+        // FIXME: Improve performance of public class fields
>+        // https://bugs.webkit.org/show_bug.cgi?id=198330
>+        RefPtr<RegisterID> privateName = generator.newTemporary();
>+        if (elementType == ClassElementTag::Instance) {
>+            Variable var = generator.variable(*name);
>+            ASSERT_WITH_MESSAGE(!var.local(), "Computed names for instance fields must be stored in captured variables");
>+
>+            generator.emitExpressionInfo(position, position, position + 1);
>+            RefPtr<RegisterID> scope = generator.emitResolveScope(generator.newTemporary(), var);
>+            generator.emitGetFromScope(privateName.get(), scope.get(), var, ThrowIfNotFound);
>+            generator.emitProfileType(privateName.get(), var, position, JSTextPosition(-1, position.offset + name->length(), -1));
>+        } else {
>+            ASSERT(expressionNode);
>+            RefPtr<RegisterID>propertyExpr = generator.emitNode(expressionNode);
>+            privateName = generator.emitToPropertyKey(propertyExpr.get(), propertyExpr.get());
>+            Ref<Label> validPropertyNameLabel = generator.newLabel();
>+            generator.emitJumpIfFalse(generator.emitBinaryOp<OpEq>(generator.newTemporary(), generator.emitLoad(nullptr, JSValue(generator.addStringConstant(generator.propertyNames().prototype))), privateName.get(), OperandTypes(ResultType::stringType(), ResultType::stringType())), validPropertyNameLabel.get());
>+            generator.emitThrowTypeError("Cannot declare a static field named 'prototype'");
>+            generator.emitLabel(validPropertyNameLabel.get());
>+        }
>+        generator.emitCallDefineProperty(dst, privateName.get(), value.get(), nullptr, nullptr, BytecodeGenerator::PropertyConfigurable | BytecodeGenerator::PropertyWritable | BytecodeGenerator::PropertyEnumerable, position);
>+        break;
>+    }
>+    default:
>+        ASSERT_NOT_REACHED();
>+    }
>+}
>+
>+static DefineFieldNode::Type fieldTypeFromNodeType(PropertyNode::Type nodeType)
>+{
>+    DefineFieldNode::Type fieldType;
>+
>+    // FIXME: this in not exactly elegant, ideally we'd only deal with DefineFieldNodes.
>+    if (nodeType == PropertyNode::Constant)
>+        fieldType = DefineFieldNode::Name;
>+    else if (nodeType == PropertyNode::Constant + PropertyNode::Private)
>+        fieldType = DefineFieldNode::PrivateName;
>+    else if (nodeType == PropertyNode::Constant + PropertyNode::Computed)
>+        fieldType = DefineFieldNode::ComputedName;
>+    else
>+        ASSERT_NOT_REACHED();
>+
>+    return fieldType;
>+}
> RegisterID* PropertyListNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dstOrConstructor, RegisterID* prototype, Vector<JSTextPosition>* instanceFieldLocations)
> {
>     // Fast case: this loop just handles regular value properties.
>+    Vector<PropertyNode*> staticFields;
>     PropertyListNode* p = this;
>     RegisterID* dst = nullptr;
>     for (; p && (p->m_node->m_type & PropertyNode::Constant); p = p->m_next) {
>         dst = p->m_node->isInstanceClassProperty() ? prototype : dstOrConstructor;
> 
>-        if (p->isComputedClassField())
>+        if (p->isComputedClassField() && !p->isStaticClassField())
>             emitSaveComputedFieldName(generator, *p->m_node);
> 
>         if (p->isInstanceClassField()) {
>@@ -555,6 +630,11 @@ RegisterID* PropertyListNode::emitBytecode(BytecodeGenerator& generator, Registe
>             continue;
>         }
> 
>+        if (p->isStaticClassField()) {
>+            staticFields.append(p->m_node);
>+            continue;
>+        }
>+
>         emitPutConstantProperty(generator, dst, *p->m_node);
>     }
> 
>@@ -602,7 +682,7 @@ RegisterID* PropertyListNode::emitBytecode(BytecodeGenerator& generator, Registe
>             PropertyNode* node = p->m_node;
>             dst = node->isInstanceClassProperty() ? prototype : dstOrConstructor;
> 
>-            if (p->isComputedClassField())
>+            if (p->isComputedClassField() && !p->isStaticClassField())
>                 emitSaveComputedFieldName(generator, *p->m_node);
> 
>             if (p->isInstanceClassField()) {
>@@ -611,6 +691,11 @@ RegisterID* PropertyListNode::emitBytecode(BytecodeGenerator& generator, Registe
>                 continue;
>             }
> 
>+            if (p->isStaticClassField()) {
>+                staticFields.append(p->m_node);
>+                continue;
>+            }
>+
>             // Handle regular values.
>             if (node->m_type & PropertyNode::Constant) {
>                 emitPutConstantProperty(generator, dst, *node);
>@@ -695,6 +780,9 @@ RegisterID* PropertyListNode::emitBytecode(BytecodeGenerator& generator, Registe
>         }
>     }
> 
>+    for (auto staticField : staticFields)
>+        emitClassField(generator, dst, staticField->assignNode(), staticField->expressionName(), staticField->name(), position(), fieldTypeFromNodeType(static_cast<PropertyNode::Type>(staticField->m_type)), ClassElementTag::Static);
>+
>     return dstOrConstructor;
> }
> 
>@@ -705,7 +793,6 @@ void PropertyListNode::emitPutConstantProperty(BytecodeGenerator& generator, Reg
>         emitPutHomeObject(generator, value.get(), newObj);
> 
>     if (node.isClassProperty()) {
>-        ASSERT(node.needsSuperBinding());
>         RefPtr<RegisterID> propertyNameRegister;
>         if (node.name())
>             propertyNameRegister = generator.emitLoad(nullptr, *node.name());
>@@ -735,6 +822,7 @@ void PropertyListNode::emitPutConstantProperty(BytecodeGenerator& generator, Reg
> void PropertyListNode::emitSaveComputedFieldName(BytecodeGenerator& generator, PropertyNode& node)
> {
>     ASSERT(node.isComputedClassField());
>+    ASSERT(!node.isStaticClassField());
>     RefPtr<RegisterID> propertyExpr;
>     const Identifier& description = *node.name();
>     auto length = node.isPrivate() ? description.length() : 1;
>@@ -743,7 +831,11 @@ void PropertyListNode::emitSaveComputedFieldName(BytecodeGenerator& generator, P
> 
>     propertyExpr = generator.emitNode(node.m_expression);
>     RegisterID* propertyName = generator.emitToPropertyKey(propertyExpr.get(), propertyExpr.get());
>+    Ref<Label> validPropertyNameLabel = generator.newLabel();
>+    generator.emitJumpIfFalse(generator.emitBinaryOp<OpEq>(generator.newTemporary(), generator.emitLoad(nullptr, JSValue(generator.addStringConstant(generator.propertyNames().prototype))), propertyName, OperandTypes(ResultType::stringType(), ResultType::stringType())), validPropertyNameLabel.get());
>+    generator.emitThrowTypeError("Cannot declare a static field named 'prototype'");
> 
>+    generator.emitLabel(validPropertyNameLabel.get());
>     RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
>     generator.emitPutToScope(scope.get(), var, propertyName, ThrowIfNotFound, InitializationMode::ConstInitialization);
>     generator.emitProfileType(propertyName, var, position(), JSTextPosition(-1, position().offset + length, -1));
>@@ -4028,44 +4120,7 @@ RegisterID* AwaitExprNode::emitBytecode(BytecodeGenerator& generator, RegisterID
> 
> void DefineFieldNode::emitBytecode(BytecodeGenerator& generator, RegisterID*)
> {
>-    RefPtr<RegisterID> value = generator.newTemporary();
>-
>-    if (!m_assign)
>-        generator.emitLoad(value.get(), jsUndefined());
>-    else {
>-        generator.emitNode(value.get(), m_assign);
>-        if (m_ident && generator.shouldEmitSetFunctionName(m_assign))
>-            generator.emitSetFunctionName(value.get(), *m_ident);
>-    }
>-
>-    switch (m_type) {
>-    case DefineFieldNode::Name: {
>-        // FIXME: Improve performance of public class fields
>-        // https://bugs.webkit.org/show_bug.cgi?id=198330
>-        RefPtr<RegisterID> propertyName = generator.emitLoad(nullptr, *m_ident);
>-        generator.emitCallDefineProperty(generator.thisRegister(), propertyName.get(), value.get(), nullptr, nullptr, BytecodeGenerator::PropertyConfigurable | BytecodeGenerator::PropertyWritable | BytecodeGenerator::PropertyEnumerable, m_position);
>-        break;
>-    }
>-    case DefineFieldNode::PrivateName: {
>-        generator.emitExpressionInfo(position(), position(), position() + m_ident->length());
>-        generator.emitPrivateFieldAdd(generator.thisRegister(), *m_ident, value.get());
>-        break;
>-    }
>-    case DefineFieldNode::ComputedName: {
>-        // FIXME: Improve performance of public class fields
>-        // https://bugs.webkit.org/show_bug.cgi?id=198330
>-        Variable var = generator.variable(*m_ident);
>-        ASSERT_WITH_MESSAGE(!var.local(), "Computed names must be stored in captured variables");
>-
>-        generator.emitExpressionInfo(position(), position(), position() + 1);
>-        RefPtr<RegisterID> scope = generator.emitResolveScope(generator.newTemporary(), var);
>-        RefPtr<RegisterID> privateName = generator.newTemporary();
>-        generator.emitGetFromScope(privateName.get(), scope.get(), var, ThrowIfNotFound);
>-        generator.emitProfileType(privateName.get(), var, m_position, JSTextPosition(-1, m_position.offset + m_ident->length(), -1));
>-        generator.emitCallDefineProperty(generator.thisRegister(), privateName.get(), value.get(), nullptr, nullptr, BytecodeGenerator::PropertyConfigurable | BytecodeGenerator::PropertyWritable | BytecodeGenerator::PropertyEnumerable, m_position);
>-        break;
>-    }
>-    }
>+    emitClassField(generator, generator.thisRegister(), m_assign, nullptr, m_ident, m_position, m_type, ClassElementTag::Instance);
> }
> 
> // ------------------------------ ClassDeclNode ---------------------------------
>diff --git a/Source/JavaScriptCore/bytecompiler/RegisterID.h b/Source/JavaScriptCore/bytecompiler/RegisterID.h
>index d9adffce4d8..8d261086913 100644
>--- a/Source/JavaScriptCore/bytecompiler/RegisterID.h
>+++ b/Source/JavaScriptCore/bytecompiler/RegisterID.h
>@@ -36,8 +36,6 @@
> namespace JSC {
> 
>     class RegisterID {
>-        WTF_MAKE_NONCOPYABLE(RegisterID);
>-
>         friend class VirtualRegister;
>     public:
>         RegisterID()
>diff --git a/Source/JavaScriptCore/parser/Nodes.h b/Source/JavaScriptCore/parser/Nodes.h
>index 3ca42f8bbaa..7e4afba4a8c 100644
>--- a/Source/JavaScriptCore/parser/Nodes.h
>+++ b/Source/JavaScriptCore/parser/Nodes.h
>@@ -716,6 +716,7 @@ namespace JSC {
>         PropertyNode(const Identifier&, ExpressionNode* propertyName, ExpressionNode*, Type, PutType, SuperBinding, ClassElementTag);
> 
>         ExpressionNode* expressionName() const { return m_expression; }
>+        ExpressionNode* assignNode() const { return m_assign; }
>         const Identifier* name() const { return m_name; }
> 
>         Type type() const { return static_cast<Type>(m_type); }
>@@ -725,6 +726,7 @@ namespace JSC {
>         bool isInstanceClassProperty() const { return static_cast<ClassElementTag>(m_classElementTag) == ClassElementTag::Instance; }
>         bool isClassField() const { return isClassProperty() && !needsSuperBinding(); }
>         bool isInstanceClassField() const { return isInstanceClassProperty() && !needsSuperBinding(); }
>+        bool isStaticClassField() const { return isStaticClassProperty() && !needsSuperBinding(); }
>         bool isOverriddenByDuplicate() const { return m_isOverriddenByDuplicate; }
>         bool isPrivate() const { return m_isPrivate; }
>         bool hasComputedName() const { return m_expression; }
>@@ -762,17 +764,24 @@ namespace JSC {
>         }
>         bool hasInstanceFields() const;
> 
>+        bool isStaticClassField() const
>+        {
>+            return m_node->isStaticClassField();
>+        }
>+
>         static bool shouldCreateLexicalScopeForClass(PropertyListNode*);
> 
>         RegisterID* emitBytecode(BytecodeGenerator&, RegisterID*, RegisterID*, Vector<JSTextPosition>*);
> 
>     private:
>+        friend class PropertyNode;
>         RegisterID* emitBytecode(BytecodeGenerator& generator, RegisterID* dst = nullptr) override
>         {
>             return emitBytecode(generator, dst, nullptr, nullptr);
>         }
>         void emitPutConstantProperty(BytecodeGenerator&, RegisterID*, PropertyNode&);
>         void emitSaveComputedFieldName(BytecodeGenerator&, PropertyNode&);
>+        void emitStaticField(BytecodeGenerator&, RegisterID*, PropertyNode&);
> 
>         PropertyNode* m_node;
>         PropertyListNode* m_next { nullptr };
>diff --git a/Source/JavaScriptCore/parser/Parser.cpp b/Source/JavaScriptCore/parser/Parser.cpp
>index 7de1dc1746f..03eb109424a 100644
>--- a/Source/JavaScriptCore/parser/Parser.cpp
>+++ b/Source/JavaScriptCore/parser/Parser.cpp
>@@ -2952,7 +2952,6 @@ parseMethod:
>             ASSERT(Options::useClassFields());
>             JSToken token = m_token;
>             ident = m_token.m_data.ident;
>-            failIfTrue(tag == ClassElementTag::Static, "Static class element cannot be private");
>             failIfTrue(isGetter || isSetter, "Cannot parse class method with private name");
>             ASSERT(ident);
>             next();
>@@ -2974,10 +2973,12 @@ parseMethod:
>             type = static_cast<PropertyNode::Type>(type | (isGetter ? PropertyNode::Getter : PropertyNode::Setter));
>             property = parseGetterSetter(context, alwaysStrictInsideClass, type, methodStart, ConstructorKind::None, tag);
>             failIfFalse(property, "Cannot parse this method");
>-        } else if (Options::useClassFields() && !match(OPENPAREN) && tag == ClassElementTag::Instance && parseMode == SourceParseMode::MethodMode && !isGetter && !isSetter) {
>+        } else if (Options::useClassFields() && !match(OPENPAREN) && parseMode == SourceParseMode::MethodMode && !isGetter && !isSetter) {
>             if (ident) {
>                 semanticFailIfTrue(*ident == propertyNames.constructor, "Cannot declare class field named 'constructor'");
>                 semanticFailIfTrue(*ident == propertyNames.constructorPrivateField, "Cannot declare private class field named '#constructor'");
>+                if (tag == ClassElementTag::Static)
>+                    semanticFailIfTrue(*ident == propertyNames.prototype, "Cannot declare a static field named 'prototype'");
>             }
> 
>             if (computedPropertyName) {
>diff --git a/Source/JavaScriptCore/runtime/JSFunction.cpp b/Source/JavaScriptCore/runtime/JSFunction.cpp
>index 20edccfbce4..2f3d26898d2 100644
>--- a/Source/JavaScriptCore/runtime/JSFunction.cpp
>+++ b/Source/JavaScriptCore/runtime/JSFunction.cpp
>@@ -685,7 +685,6 @@ void JSFunction::setFunctionName(ExecState* exec, JSValue value)
>         return;
> 
>     ASSERT(!isHostFunction());
>-    ASSERT(jsExecutable()->ecmaName().isNull());
>     String name;
>     if (value.isSymbol()) {
>         PrivateName privateName = asSymbol(value)->privateName();
>-- 
>2.21.0
>

Actions: View | Formatted Diff | Diff

Attachments on bug 194095: 373113 | 400054 | 401133 | 404092 | 404819 | 405349 | 412412 | 414161 | 414165 | 414315