Attachment 370695 Details for Bug 194435 – WIP - Patch

[patch] WIP - Patch

private_accessors.diff (text/plain), 39.46 KB, created by Caio Lima on 2019-05-27 10:00:12 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2019-05-27 10:00:12 PDT

Size: 39.46 KB

patch

obsolete

>diff --git a/JSTests/stress/private-accesor-duplicate-name-early-errors.js b/JSTests/stress/private-accesor-duplicate-name-early-errors.js
>new file mode 100644
>index 00000000000..55327e02a16
>--- /dev/null
>+++ b/JSTests/stress/private-accesor-duplicate-name-early-errors.js
>@@ -0,0 +1,92 @@
>+//@ run("--useClassFields=true --usePrivateMethods=true")
>+
>+let assert = {
>+    sameValue: function (lhs, rhs) {
>+        if (lhs !== rhs)
>+            throw new Error("Expected: " + lhs + " bug got: " + rhs);
>+    },
>+
>+    throws: function (expectedError, op) {
>+        try {
>+          op();
>+        } catch(e) {
>+            if (!(e instanceof expectedError))
>+                throw new Error("Expected to throw: " + expectedError + " but threw: " + e);
>+        }
>+    }
>+}
>+
>+assert.throws(SyntaxError, function() {
>+    eval(`
>+        class C {
>+            set #m(v) { this._v = v; }
>+            set #m(u) {}
>+        }
>+    `);
>+});
>+
>+assert.throws(SyntaxError, function() {
>+    eval(`
>+        class C {
>+            #m;
>+            set #m(v) { this._v = v; }
>+        }
>+    `);
>+});
>+
>+assert.throws(SyntaxError, function() {
>+    eval(`
>+        class C {
>+            set #m(v) { this._v = v; }
>+            #m;
>+        }
>+    `);
>+});
>+
>+assert.throws(SyntaxError, function() {
>+    eval(`
>+        class C {
>+            set #m(v) { this._v = v; }
>+            #m() {}
>+        }
>+    `);
>+});
>+
>+assert.throws(SyntaxError, function() {
>+    eval(`
>+        class C {
>+            #m() {}
>+            get #m() {}
>+        }
>+    `);
>+});
>+
>+assert.throws(SyntaxError, function() {
>+    eval(`
>+        class C {
>+            get #m() {}
>+            get #m() {}
>+        }
>+    `);
>+});
>+
>+assert.throws(SyntaxError, function() {
>+    eval(`
>+        class C {
>+            get #m() {}
>+            set #m() {}
>+            #m;
>+        }
>+    `);
>+});
>+
>+assert.throws(SyntaxError, function() {
>+    eval(`
>+        class C {
>+            get #m() {}
>+            set #m() {}
>+            #m() {}
>+        }
>+    `);
>+});
>+
>diff --git a/JSTests/stress/private-getter-brand-check.js b/JSTests/stress/private-getter-brand-check.js
>new file mode 100644
>index 00000000000..5911ffa45fe
>--- /dev/null
>+++ b/JSTests/stress/private-getter-brand-check.js
>@@ -0,0 +1,91 @@
>+//@ run("--useClassFields=true --usePrivateMethods=true")
>+
>+let assert = {
>+    sameValue: function (lhs, rhs) {
>+        if (lhs !== rhs)
>+            throw new Error("Expected: " + lhs + " bug got: " + rhs);
>+    },
>+
>+    throws: function (expectedError, op) {
>+        try {
>+          op();
>+        } catch(e) {
>+            if (!(e instanceof expectedError))
>+                throw new Error("Expected to throw: " + expectedError + " but threw: " + e);
>+        }
>+    }
>+};
>+
>+(function () {
>+    let createAndInstantiateClass = function () {
>+        class C {
>+            get #m() { return 'test'; }
>+
>+            access(o) {
>+                return o.#m;
>+            }
>+        }
>+
>+        return new C();
>+    }
>+
>+    let c1 = createAndInstantiateClass();
>+    let c2 = createAndInstantiateClass();
>+
>+    assert.sameValue(c1.access(c1), 'test');
>+    assert.sameValue(c2.access(c2), 'test');
>+
>+    assert.throws(TypeError, function() {
>+        c1.access(c2);
>+    });
>+
>+    assert.throws(TypeError, function() {
>+        c2.access(c1);
>+    });
>+})();
>+
>+(function () {
>+    class S {
>+        get #m() { return 'super class'; }
>+
>+        superAccess() { return this.#m; }
>+    }
>+
>+    class C extends S {
>+        get #m() { return 'subclass'; }
>+
>+        access() {
>+            return this.#m;
>+        }
>+    }
>+
>+    let c = new C();
>+
>+    assert.sameValue(c.access(), 'subclass');
>+    assert.sameValue(c.superAccess(), 'super class');
>+
>+    let s = new S();
>+    assert.sameValue(s.superAccess(), 'super class');
>+    assert.throws(TypeError, function() {
>+        c.access.call(s);
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        get #m() { return 'test'; }
>+
>+        access(o) {
>+            return o.#m;
>+        }
>+    }
>+
>+    let c = new C();
>+    assert.sameValue(c.access(c), 'test');
>+
>+    let o = {};
>+    assert.throws(TypeError, function() {
>+        c.access(o);
>+    });
>+})();
>+
>diff --git a/JSTests/stress/private-getter-inner-class.js b/JSTests/stress/private-getter-inner-class.js
>new file mode 100644
>index 00000000000..b7bead57e4d
>--- /dev/null
>+++ b/JSTests/stress/private-getter-inner-class.js
>@@ -0,0 +1,132 @@
>+//@ run("--useClassFields=true --usePrivateMethods=true")
>+
>+let assert = {
>+    sameValue: function (lhs, rhs) {
>+        if (lhs !== rhs)
>+            throw new Error("Expected: " + lhs + " bug got: " + rhs);
>+    },
>+
>+    throws: function (expectedError, op) {
>+        try {
>+          op();
>+        } catch(e) {
>+            if (!(e instanceof expectedError))
>+                throw new Error("Expected to throw: " + expectedError + " but threw: " + e);
>+        }
>+    }
>+};
>+
>+(function () {
>+    class C {
>+        get #m() { return 'test'; }
>+
>+        B = class {
>+            method(o) {
>+                return o.#m;
>+            }
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+    assert.sameValue(innerB.method(c), 'test');
>+})();
>+
>+(function () {
>+    class C {
>+        get #m() { return 'outer class'; }
>+
>+        method() { return this.#m; }
>+
>+        B = class {
>+            method(o) {
>+                return o.#m;
>+            }
>+
>+            #m = 'test';
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+    assert.sameValue(innerB.method(innerB), 'test');
>+    assert.sameValue(c.method(), 'outer class');
>+    assert.throws(TypeError, function() {
>+        innerB.method(c);
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        get #m() { return 'outer class'; }
>+
>+        method() { return this.#m; }
>+
>+        B = class {
>+            method(o) {
>+                return o.#m;
>+            }
>+
>+            get #m() { return 'test'; }
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+    assert.sameValue(innerB.method(innerB), 'test');
>+    assert.sameValue(c.method(), 'outer class');
>+    assert.throws(TypeError, function() {
>+        innerB.method(c);
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        get #m() { throw new Error('Should never execute'); }
>+
>+        B = class {
>+            method(o) {
>+                return o.#m();
>+            }
>+
>+            #m() { return 'test'; }
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+    assert.sameValue(innerB.method(innerB), 'test');
>+    assert.throws(TypeError, function() {
>+        innerB.method(c);
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        get #m() { return 'outer class'; }
>+
>+        method() { return this.#m; }
>+
>+        B = class {
>+            method(o) {
>+                return o.#m;
>+            }
>+
>+            set #m(v) { this._v = v; }
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+
>+    assert.throws(TypeError, function() {
>+        innerB.method(innerB);
>+    });
>+
>+    assert.sameValue(c.method(), 'outer class');
>+
>+    assert.throws(TypeError, function() {
>+        innerB.method(c);
>+    });
>+})();
>+
>diff --git a/JSTests/stress/private-members-get-and-set.js b/JSTests/stress/private-members-get-and-set.js
>new file mode 100644
>index 00000000000..7d99e3e0203
>--- /dev/null
>+++ b/JSTests/stress/private-members-get-and-set.js
>@@ -0,0 +1,63 @@
>+//@ run("--useClassFields=true --usePrivateMethods=true")
>+
>+let assert = {
>+    sameValue: function (lhs, rhs) {
>+        if (lhs !== rhs)
>+            throw new Error("Expected: " + lhs + " bug got: " + rhs);
>+    },
>+
>+    throws: function (expectedError, op) {
>+        try {
>+          op();
>+        } catch(e) {
>+            if (!(e instanceof expectedError))
>+                throw new Error("Expected to throw: " + expectedError + " but threw: " + e);
>+        }
>+    }
>+};
>+
>+(function () {
>+    class C {
>+        set #m(v) { this._v = v; }
>+        
>+        accessPrivateMember() {
>+            return this.#m;
>+        }
>+    }
>+
>+    let c = new C();
>+    assert.throws(TypeError, function () {
>+        c.accessPrivateMember();
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        get #m() { return 'test'; }
>+        
>+        accessPrivateMember(v) {
>+            this.#m = v;
>+        }
>+    }
>+
>+    let c = new C();
>+    assert.throws(TypeError, function () {
>+        c.accessPrivateMember('test');
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        #m() { return 'test'; }
>+        
>+        accessPrivateMember(v) {
>+            this.#m = v;
>+        }
>+    }
>+
>+    let c = new C();
>+    assert.throws(TypeError, function () {
>+        c.accessPrivateMember('test');
>+    });
>+})();
>+
>diff --git a/JSTests/stress/private-names-available-on-direct-eval.js b/JSTests/stress/private-names-available-on-direct-eval.js
>index e2707210dac..b35fdce36ad 100644
>--- a/JSTests/stress/private-names-available-on-direct-eval.js
>+++ b/JSTests/stress/private-names-available-on-direct-eval.js
>@@ -21,3 +21,36 @@ let assert = {
>     assert.sameValue(c.callMethodFromEval(), 'test');
> })();
> 
>+(function () {
>+    class C {
>+        get #m() {
>+            return 'test';
>+        }
>+
>+        callGetterFromEval() {
>+            let self = this;
>+            return eval('self.#m');
>+        }
>+    }
>+
>+    let c = new C();
>+    assert.sameValue(c.callGetterFromEval(), 'test');
>+})();
>+
>+(function () {
>+    class C {
>+        set #m(v) {
>+            this._v = v;
>+        }
>+
>+        callSetterFromEval(v) {
>+            let self = this;
>+            eval('self.#m = v');
>+        }
>+    }
>+
>+    let c = new C();
>+    c.callSetterFromEval('test')
>+    assert.sameValue(c._v, 'test');
>+})();
>+
>diff --git a/JSTests/stress/private-names-available-on-eval-during-field-initialization.js b/JSTests/stress/private-names-available-on-eval-during-field-initialization.js
>new file mode 100644
>index 00000000000..400fbf388bf
>--- /dev/null
>+++ b/JSTests/stress/private-names-available-on-eval-during-field-initialization.js
>@@ -0,0 +1,31 @@
>+//@ run("--useClassFields=true --usePrivateMethods=true")
>+
>+let assert = {
>+    sameValue: function (lhs, rhs) {
>+        if (lhs !== rhs)
>+            throw new Error("Expected: " + rhs + " bug got: " + lhs);
>+    }
>+};
>+
>+(function () {
>+    class C {
>+        #m() { return 'test'; }
>+
>+        field = eval('this.#m()');
>+    }
>+
>+    let c = new C();
>+    assert.sameValue(c.field, 'test');
>+})();
>+
>+(function () {
>+    class C {
>+        get #m() { return 'test'; }
>+
>+        field = eval('this.#m');
>+    }
>+
>+    let c = new C();
>+    assert.sameValue(c.field, 'test');
>+})();
>+
>diff --git a/JSTests/stress/private-setter-brand-check.js b/JSTests/stress/private-setter-brand-check.js
>new file mode 100644
>index 00000000000..af4d8cf3d4e
>--- /dev/null
>+++ b/JSTests/stress/private-setter-brand-check.js
>@@ -0,0 +1,100 @@
>+//@ run("--useClassFields=true --usePrivateMethods=true")
>+
>+let assert = {
>+    sameValue: function (lhs, rhs) {
>+        if (lhs !== rhs)
>+            throw new Error("Expected: " + lhs + " bug got: " + rhs);
>+    },
>+
>+    throws: function (expectedError, op) {
>+        try {
>+          op();
>+        } catch(e) {
>+            if (!(e instanceof expectedError))
>+                throw new Error("Expected to throw: " + expectedError + " but threw: " + e);
>+        }
>+    }
>+};
>+
>+(function () {
>+    let createAndInstantiateClass = function () {
>+        class C {
>+            set #m(v) { this._v = v; }
>+
>+            access(o, v) {
>+                o.#m = v;
>+            }
>+        }
>+
>+        let c = new C();
>+        return c;
>+    };
>+
>+    let c1 = createAndInstantiateClass();
>+    let c2 = createAndInstantiateClass();
>+
>+    c1.access(c1, 'test');
>+    assert.sameValue(c1._v, 'test');
>+    c2.access(c2, 'test');
>+    assert.sameValue(c2._v, 'test');
>+
>+    assert.throws(TypeError, function() {
>+        c1.access(c2, 'foo');
>+    });
>+
>+    assert.throws(TypeError, function() {
>+        c2.access(c1, 'foo');
>+    });
>+})();
>+
>+(function () {
>+    class S {
>+        set #m(v) { this._v = v }
>+
>+        superAccess(v) { this.#m = v; }
>+    }
>+
>+    class C extends S {
>+        set #m(v) { this._u = v; }
>+
>+        access(v) {
>+            return this.#m = v;
>+        }
>+    }
>+
>+    let c = new C();
>+
>+    c.access('test');
>+    assert.sameValue(c._u, 'test');
>+
>+    c.superAccess('super class');
>+    assert.sameValue(c._v, 'super class');
>+
>+    let s = new S();
>+    s.superAccess('super class')
>+    assert.sameValue(s._v, 'super class');
>+
>+    assert.throws(TypeError, function() {
>+        c.access.call(s, 'foo');
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        set #m(v) { this._v = v; }
>+
>+        access(o, v) {
>+            return o.#m = v;
>+        }
>+    }
>+
>+    let c = new C();
>+    c.access(c, 'test');
>+    assert.sameValue(c._v, 'test');
>+
>+    let o = {};
>+    assert.throws(TypeError, function() {
>+        c.access(o, 'foo');
>+    });
>+})();
>+
>diff --git a/JSTests/stress/private-setter-inner-class.js b/JSTests/stress/private-setter-inner-class.js
>new file mode 100644
>index 00000000000..90c575a68d9
>--- /dev/null
>+++ b/JSTests/stress/private-setter-inner-class.js
>@@ -0,0 +1,155 @@
>+//@ run("--useClassFields=true --usePrivateMethods=true")
>+
>+let assert = {
>+    sameValue: function (lhs, rhs) {
>+        if (lhs !== rhs)
>+            throw new Error("Expected: " + lhs + " bug got: " + rhs);
>+    },
>+
>+    throws: function (expectedError, op) {
>+        try {
>+          op();
>+        } catch(e) {
>+            if (!(e instanceof expectedError))
>+                throw new Error("Expected to throw: " + expectedError + " but threw: " + e);
>+        }
>+    }
>+};
>+
>+(function () {
>+    class C {
>+        set #m(v) { this._v = v; }
>+
>+        B = class {
>+            method(o, v) {
>+                o.#m = v;
>+            }
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+    innerB.method(c, 'test');
>+    assert.sameValue(c._v, 'test');
>+})();
>+
>+(function () {
>+    class C {
>+        set #m(v) { this._v = v; }
>+
>+        method(v) { this.#m = v; }
>+
>+        B = class {
>+            method(o, v) {
>+                o.#m = v;
>+            }
>+
>+            get m() { return this.#m; }
>+
>+            #m;
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+
>+    innerB.method(innerB, 'test');
>+    assert.sameValue(innerB.m, 'test');
>+
>+    c.method('outer class');
>+    assert.sameValue(c._v, 'outer class');
>+
>+    assert.throws(TypeError, function() {
>+        innerB.method(c, 'foo');
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        set #m(v) { this._v = v; }
>+
>+        method(v) { this.#m = v; }
>+
>+        B = class {
>+            method(o, v) {
>+                o.#m = v;
>+            }
>+
>+            get #m() { return 'test'; }
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+
>+    assert.throws(TypeError, function() {
>+        innerB.method(innerB);
>+    });
>+
>+    c.method('outer class');
>+    assert.sameValue(c._v, 'outer class');
>+
>+    assert.throws(TypeError, function() {
>+        innerB.method(c);
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        set #m(v) { this._v = v; }
>+
>+        method(v) { this.#m = v; }
>+
>+        B = class {
>+            method(o, v) {
>+                o.#m = v;
>+            }
>+
>+            #m() { return 'test'; }
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+	
>+    assert.throws(TypeError, function() {
>+        innerB.method(innerB, 'foo');
>+    });
>+
>+    c.method('outer class');
>+    assert.sameValue(c._v, 'outer class');
>+
>+    assert.throws(TypeError, function() {
>+        innerB.method(c);
>+    });
>+})();
>+
>+(function () {
>+    class C {
>+        set #m(v) { this._v = v; }
>+
>+        method(v) { this.#m = v; }
>+
>+        B = class {
>+            method(o, v) {
>+                o.#m = v;
>+            }
>+
>+            set #m(v) { this._v = v; }
>+        }
>+    }
>+
>+    let c = new C();
>+    let innerB = new c.B();
>+
>+    innerB.method(innerB, 'test262');
>+    assert.sameValue(innerB._v, 'test262');
>+
>+    c.method('outer class');
>+    assert.sameValue(c._v, 'outer class');
>+
>+    assert.throws(TypeError, function() {
>+        innerB.method(c, 'foo');
>+    });
>+})();
>+
>diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>index d62de78ea01..6cd76533ff2 100644
>--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>@@ -2940,6 +2940,30 @@ bool BytecodeGenerator::isPrivateMethod(const Identifier& ident)
>     return false;
> }
> 
>+bool BytecodeGenerator::isPrivateSetter(const Identifier& ident)
>+{
>+    for (unsigned i = m_privateNamesStack.size(); i--; ) {
>+        auto& map = m_privateNamesStack[i];
>+        auto it = map.find(ident.impl());
>+        if (it != map.end())
>+            return it->value.isSetter();
>+    }
>+
>+    return false;
>+}
>+
>+bool BytecodeGenerator::isPrivateGetter(const Identifier& ident)
>+{
>+    for (unsigned i = m_privateNamesStack.size(); i--; ) {
>+        auto& map = m_privateNamesStack[i];
>+        auto it = map.find(ident.impl());
>+        if (it != map.end())
>+            return it->value.isGetter();
>+    }
>+
>+    return false;
>+}
>+
> void BytecodeGenerator::pushPrivateAccessNames(const VariableEnvironment& environment)
> {
>      if (!environment.privateNamesSize())
>diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>index fba6d2320d5..c4f490d1258 100644
>--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>@@ -1204,6 +1204,8 @@ namespace JSC {
>         }
> 
>         bool isPrivateMethod(const Identifier&);
>+        bool isPrivateSetter(const Identifier&);
>+        bool isPrivateGetter(const Identifier&);
> 
>         void pushPrivateAccessNames(const VariableEnvironment&);
>         void popPrivateAccessNames();
>diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>index e79f348d681..1a2d41b499e 100644
>--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>@@ -713,6 +713,20 @@ void PropertyListNode::emitPutConstantProperty(BytecodeGenerator& generator, Reg
>             return;
>         }
> 
>+        if (node.type() & PropertyNode::PrivateSetter) {
>+            Identifier setterIdentifier = Identifier::fromString(generator.vm(), makeString(String(node.name()->impl()), "-setter"));
>+            Variable var = generator.variable(setterIdentifier);
>+            generator.emitPutToScope(generator.scopeRegister(), var, value.get(), DoNotThrowIfNotFound, InitializationMode::ConstInitialization);
>+            return;
>+        }
>+
>+        if (node.type() & PropertyNode::PrivateGetter) {
>+            Identifier getterIdentifier = Identifier::fromString(generator.vm(), makeString(String(node.name()->impl()), "-getter"));
>+            Variable var = generator.variable(getterIdentifier);
>+            generator.emitPutToScope(generator.scopeRegister(), var, value.get(), DoNotThrowIfNotFound, InitializationMode::ConstInitialization);
>+            return;
>+        }
>+
>         RefPtr<RegisterID> propertyNameRegister;
>         if (node.name())
>             propertyNameRegister = generator.emitLoad(nullptr, *node.name());
>@@ -819,8 +833,9 @@ RegisterID* DotAccessorNode::emitBytecode(BytecodeGenerator& generator, Register
> RegisterID* BaseDotNode::emitGetPropertyValue(BytecodeGenerator& generator, RegisterID* dst, RegisterID* base, RefPtr<RegisterID>& thisValue)
> {
>     if (isPrivateName()) {
>-        if (generator.isPrivateMethod(identifier())) {
>-            Variable var = generator.variable(identifier());
>+        auto identifierName = identifier();
>+        if (generator.isPrivateMethod(identifierName)) {
>+            Variable var = generator.variable(identifierName);
>             RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
> 
>             RegisterID* privateBrandSymbol = generator.emitGetPrivateBrand(generator.newTemporary(), scope.get());
>@@ -829,6 +844,32 @@ RegisterID* BaseDotNode::emitGetPropertyValue(BytecodeGenerator& generator, Regi
>             return generator.emitGetFromScope(dst, scope.get(), var, ThrowIfNotFound);
>         }
> 
>+        if (generator.isPrivateGetter(identifierName)) {
>+            Identifier getterIdentifier = Identifier::fromString(generator.vm(), makeString(String(identifierName.impl()), "-getter"));
>+            Variable var = generator.variable(getterIdentifier);
>+            RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
>+
>+            RegisterID* privateBrandSymbol = generator.emitGetPrivateBrand(generator.newTemporary(), scope.get());
>+            generator.emitCheckPrivateBrand(base, privateBrandSymbol);
>+            
>+            RefPtr<RegisterID> getterFunction = generator.emitGetFromScope(generator.newTemporary(), scope.get(), var, ThrowIfNotFound);
>+            CallArguments args(generator, nullptr);
>+            generator.move(args.thisRegister(), base);
>+            return generator.emitCall(dst, getterFunction.get(), NoExpectedFunction, args, m_position, m_position, m_position, DebuggableCall::Yes);
>+        }
>+
>+        if (generator.isPrivateSetter(identifierName)) {
>+            // We need to perform brand check to follow the spec
>+            Identifier setterIdentifier = Identifier::fromString(generator.vm(), makeString(String(identifierName.impl()), "-setter"));
>+            Variable var = generator.variable(setterIdentifier);
>+            RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
>+
>+            RegisterID* privateBrandSymbol = generator.emitGetPrivateBrand(generator.newTemporary(), scope.get());
>+            generator.emitCheckPrivateBrand(base, privateBrandSymbol);
>+            generator.emitThrowTypeError("Trying to access a not defined private getter");
>+            return dst;
>+        }
>+
>         return generator.emitPrivateFieldGet(dst, base, identifier());
>     }
> 
>@@ -851,8 +892,28 @@ RegisterID* BaseDotNode::emitPutProperty(BytecodeGenerator& generator, RegisterI
> {
>     if (isPrivateName()) {
>         auto identifierName = identifier();
>-        if (generator.isPrivateMethod(identifierName)) {
>-            Variable var = generator.variable(identifierName);
>+        if (generator.isPrivateSetter(identifierName)) {
>+            Identifier setterIdentifier = Identifier::fromString(generator.vm(), makeString(String(identifierName.impl()), "-setter"));
>+            Variable var = generator.variable(setterIdentifier);
>+            RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
>+
>+            RegisterID* privateBrandSymbol = generator.emitGetPrivateBrand(generator.newTemporary(), scope.get());
>+            generator.emitCheckPrivateBrand(base, privateBrandSymbol);
>+            
>+            RefPtr<RegisterID> setterFunction = generator.emitGetFromScope(generator.newTemporary(), scope.get(), var, ThrowIfNotFound);
>+            CallArguments args(generator, nullptr, 1);
>+            generator.move(args.thisRegister(), base);
>+            generator.move(args.argumentRegister(0), value);
>+            generator.emitCall(generator.newTemporary(), setterFunction.get(), NoExpectedFunction, args, m_position, m_position, m_position, DebuggableCall::Yes);
>+
>+            return value;
>+        }
>+
>+        bool isPrivateGetter = generator.isPrivateGetter(identifierName);
>+        if (isPrivateGetter || generator.isPrivateMethod(identifierName)) {
>+            Identifier operationIdentifier = isPrivateGetter ? Identifier::fromString(generator.vm(), makeString(String(identifierName.impl()), "-getter")) : identifierName;
>+
>+            Variable var = generator.variable(operationIdentifier);
>             RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
> 
>             RegisterID* privateBrandSymbol = generator.emitGetPrivateBrand(generator.newTemporary(), scope.get());
>diff --git a/Source/JavaScriptCore/parser/NodeConstructors.h b/Source/JavaScriptCore/parser/NodeConstructors.h
>index fb5224f77b8..e8895c38252 100644
>--- a/Source/JavaScriptCore/parser/NodeConstructors.h
>+++ b/Source/JavaScriptCore/parser/NodeConstructors.h
>@@ -253,7 +253,7 @@ namespace JSC {
>         , m_type(type)
>         , m_needsSuperBinding(superBinding == SuperBinding::Needed)
>         , m_putType(putType)
>-        , m_isPrivate(!!(type & (PrivateField | PrivateMethod)))
>+        , m_isPrivate(!!(type & (PrivateField | PrivateMethod | PrivateGetter | PrivateSetter)))
>         , m_classElementTag(static_cast<unsigned>(tag))
>         , m_isOverriddenByDuplicate(false)
>     {
>@@ -266,7 +266,7 @@ namespace JSC {
>         , m_type(type)
>         , m_needsSuperBinding(superBinding == SuperBinding::Needed)
>         , m_putType(putType)
>-        , m_isPrivate(!!(type & (PrivateField | PrivateMethod)))
>+        , m_isPrivate(!!(type & (PrivateField | PrivateMethod | PrivateGetter | PrivateSetter)))
>         , m_classElementTag(static_cast<unsigned>(tag))
>         , m_isOverriddenByDuplicate(false)
>     {
>@@ -279,7 +279,7 @@ namespace JSC {
>         , m_type(type)
>         , m_needsSuperBinding(superBinding == SuperBinding::Needed)
>         , m_putType(putType)
>-        , m_isPrivate(!!(type & (PrivateField | PrivateMethod)))
>+        , m_isPrivate(!!(type & (PrivateField | PrivateMethod | PrivateGetter | PrivateSetter)))
>         , m_classElementTag(static_cast<unsigned>(tag))
>         , m_isOverriddenByDuplicate(false)
>     {
>@@ -292,7 +292,7 @@ namespace JSC {
>         , m_type(type)
>         , m_needsSuperBinding(superBinding == SuperBinding::Needed)
>         , m_putType(putType)
>-        , m_isPrivate(!!(type & (PrivateField | PrivateMethod)))
>+        , m_isPrivate(!!(type & (PrivateField | PrivateMethod | PrivateGetter | PrivateSetter)))
>         , m_classElementTag(static_cast<unsigned>(tag))
>         , m_isOverriddenByDuplicate(false)
>     {
>diff --git a/Source/JavaScriptCore/parser/Nodes.h b/Source/JavaScriptCore/parser/Nodes.h
>index 531b7de12ff..c6354c5bf11 100644
>--- a/Source/JavaScriptCore/parser/Nodes.h
>+++ b/Source/JavaScriptCore/parser/Nodes.h
>@@ -706,7 +706,7 @@ namespace JSC {
>     enum class ClassElementTag : uint8_t { No, Instance, Static, LastTag };
>     class PropertyNode final : public ParserArenaFreeable {
>     public:
>-        enum Type : uint8_t { Constant = 1, Getter = 2, Setter = 4, Computed = 8, Shorthand = 16, Spread = 32, PrivateField = 64, PrivateMethod = 128 };
>+        enum Type : uint16_t { Constant = 1, Getter = 2, Setter = 4, Computed = 8, Shorthand = 16, Spread = 32, PrivateField = 64, PrivateMethod = 128, PrivateSetter = 256, PrivateGetter = 512 };
>         enum PutType : uint8_t { Unknown, KnownDirect };
> 
>         PropertyNode(const Identifier&, ExpressionNode*, Type, PutType, SuperBinding, ClassElementTag);
>@@ -736,7 +736,7 @@ namespace JSC {
>         const Identifier* m_name;
>         ExpressionNode* m_expression;
>         ExpressionNode* m_assign;
>-        unsigned m_type;
>+        unsigned m_type : 10;
>         unsigned m_needsSuperBinding : 1;
>         unsigned m_putType : 1;
>         unsigned m_isPrivate : 1;
>diff --git a/Source/JavaScriptCore/parser/Parser.cpp b/Source/JavaScriptCore/parser/Parser.cpp
>index 71944385f46..8c88cc8cafd 100644
>--- a/Source/JavaScriptCore/parser/Parser.cpp
>+++ b/Source/JavaScriptCore/parser/Parser.cpp
>@@ -2916,7 +2916,7 @@ parseMethod:
>             ident = m_token.m_data.ident;
>             ASSERT(ident);
>             next();
>-            if (parseMode == SourceParseMode::MethodMode && (matchIdentifierOrKeyword() || match(STRING) || match(DOUBLE) || match(INTEGER) || match(OPENBRACKET))) {
>+            if (parseMode == SourceParseMode::MethodMode && (matchIdentifierOrKeyword() || match(STRING) || match(DOUBLE) || match(INTEGER) || match(OPENBRACKET) || (Options::usePrivateMethods() && match(PRIVATENAME)))) {
>                 isGetter = *ident == propertyNames.get;
>                 isSetter = *ident == propertyNames.set;
>             }
>@@ -2960,8 +2960,19 @@ parseMethod:
>         TreeProperty property;
>         const bool alwaysStrictInsideClass = true;
>         if (isGetter || isSetter) {
>-            type = static_cast<PropertyNode::Type>(type & ~PropertyNode::Constant);
>-            type = static_cast<PropertyNode::Type>(type | (isGetter ? PropertyNode::Getter : PropertyNode::Setter));
>+            if (Options::usePrivateMethods() && match(PRIVATENAME)) {
>+                ident = m_token.m_data.ident;
>+                if (isSetter) {
>+                    semanticFailIfTrue(classScope->declarePrivateSetter(m_vm, *ident) & DeclarationResult::InvalidDuplicateDeclaration, "Declared private setter with an already used name");
>+                     type = static_cast<PropertyNode::Type>(type | PropertyNode::PrivateSetter);
>+                } else {
>+                    semanticFailIfTrue(classScope->declarePrivateGetter(m_vm, *ident) & DeclarationResult::InvalidDuplicateDeclaration, "Declared private getter with an already used name");
>+                    type = static_cast<PropertyNode::Type>(type | PropertyNode::PrivateGetter);
>+                }
>+            } else {
>+                type = static_cast<PropertyNode::Type>(type & ~PropertyNode::Constant);
>+                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) {
>@@ -4218,12 +4229,17 @@ template <class TreeBuilder> TreeProperty Parser<LexerType>::parseGetterSetter(T
> 
>     JSTokenLocation location(tokenLocation());
> 
>-    if (matchSpecIdentifier() || match(STRING) || m_token.m_type & KeywordTokenFlag) {
>+    if (matchSpecIdentifier() || match(STRING) || (Options::usePrivateMethods() && match(PRIVATENAME)) || m_token.m_type & KeywordTokenFlag) {
>         stringPropertyName = m_token.m_data.ident;
>         semanticFailIfTrue(tag == ClassElementTag::Static && *stringPropertyName == m_vm->propertyNames->prototype,
>             "Cannot declare a static method named 'prototype'");
>         semanticFailIfTrue(tag == ClassElementTag::Instance && *stringPropertyName == m_vm->propertyNames->constructor,
>             "Cannot declare a getter or setter named 'constructor'");
>+
>+        if (match(PRIVATENAME)) {
>+            semanticFailIfTrue(tag == ClassElementTag::No, "Cannot declare a private setter or getter outside a class");
>+            semanticFailIfTrue(tag == ClassElementTag::Static, "Cannot declare a private setter or getter as static");
>+        }
>         next();
>     } else if (match(DOUBLE) || match(INTEGER)) {
>         numericPropertyName = m_token.m_data.doubleValue;
>@@ -4240,9 +4256,15 @@ template <class TreeBuilder> TreeProperty Parser<LexerType>::parseGetterSetter(T
>     if (type & PropertyNode::Getter) {
>         failIfFalse(match(OPENPAREN), "Expected a parameter list for getter definition");
>         failIfFalse((parseFunctionInfo(context, FunctionNameRequirements::Unnamed, SourceParseMode::GetterMode, false, constructorKind, SuperBinding::Needed, getterOrSetterStartOffset, info, FunctionDefinitionType::Method)), "Cannot parse getter definition");
>-    } else {
>+    } else if (type & PropertyNode::Setter){
>         failIfFalse(match(OPENPAREN), "Expected a parameter list for setter definition");
>         failIfFalse((parseFunctionInfo(context, FunctionNameRequirements::Unnamed, SourceParseMode::SetterMode, false, constructorKind, SuperBinding::Needed, getterOrSetterStartOffset, info, FunctionDefinitionType::Method)), "Cannot parse setter definition");
>+    } else if (type & PropertyNode::PrivateSetter){
>+        failIfFalse(match(OPENPAREN), "Expected a parameter list for private setter definition");
>+        failIfFalse((parseFunctionInfo(context, FunctionNameRequirements::Unnamed, SourceParseMode::SetterMode, false, constructorKind, SuperBinding::Needed, getterOrSetterStartOffset, info, FunctionDefinitionType::Method)), "Cannot parse setter definition");
>+    } else if (type & PropertyNode::PrivateGetter){
>+        failIfFalse(match(OPENPAREN), "Expected a parameter list for private getter definition");
>+        failIfFalse((parseFunctionInfo(context, FunctionNameRequirements::Unnamed, SourceParseMode::GetterMode, false, constructorKind, SuperBinding::Needed, getterOrSetterStartOffset, info, FunctionDefinitionType::Method)), "Cannot parse setter definition");
>     }
> 
>     if (stringPropertyName)
>diff --git a/Source/JavaScriptCore/parser/Parser.h b/Source/JavaScriptCore/parser/Parser.h
>index 17692b39880..a8a37a744d6 100644
>--- a/Source/JavaScriptCore/parser/Parser.h
>+++ b/Source/JavaScriptCore/parser/Parser.h
>@@ -519,6 +519,50 @@ public:
>         return result;
>     }
> 
>+    DeclarationResultMask declarePrivateSetter(VM* vm, const Identifier& ident)
>+    {
>+        ASSERT(m_allowsLexicalDeclarations);
>+        DeclarationResultMask result = DeclarationResult::Valid;
>+        bool addResult = m_lexicalVariables.declarePrivateSetter(ident);
>+
>+        if (!addResult) {
>+            result |= DeclarationResult::InvalidDuplicateDeclaration;
>+            return result;
>+        }
>+
>+        // We declare it since we store private setters on class
>+        // scope.
>+        Identifier setterIdentifier = Identifier::fromString(vm, makeString(String(ident.impl()), "-setter"));
>+        DeclarationResultMask declarationResult = declareLexicalVariable(&setterIdentifier, false);
>+        ASSERT_UNUSED(declarationResult, declarationResult == DeclarationResult::Valid);
>+        useVariable(&setterIdentifier, false);
>+        addClosedVariableCandidateUnconditionally(setterIdentifier.impl());
>+
>+        return result;
>+    }
>+
>+    DeclarationResultMask declarePrivateGetter(VM* vm, const Identifier& ident)
>+    {
>+        ASSERT(m_allowsLexicalDeclarations);
>+        DeclarationResultMask result = DeclarationResult::Valid;
>+        bool addResult = m_lexicalVariables.declarePrivateGetter(ident);
>+
>+        if (!addResult) {
>+            result |= DeclarationResult::InvalidDuplicateDeclaration;
>+            return result;
>+        }
>+
>+        // We declare it since we store private getters on class
>+        // scope.
>+        Identifier getterIdentifier = Identifier::fromString(vm, makeString(String(ident.impl()), "-getter"));
>+        DeclarationResultMask declarationResult = declareLexicalVariable(&getterIdentifier, false);
>+        ASSERT_UNUSED(declarationResult, declarationResult == DeclarationResult::Valid);
>+        useVariable(&getterIdentifier, false);
>+        addClosedVariableCandidateUnconditionally(getterIdentifier.impl());
>+
>+        return result;
>+    }
>+
>     DeclarationResultMask declarePrivateName(const Identifier& ident)
>     {
>         ASSERT(m_allowsLexicalDeclarations);
>diff --git a/Source/JavaScriptCore/parser/VariableEnvironment.cpp b/Source/JavaScriptCore/parser/VariableEnvironment.cpp
>index 300caceebdf..2463e431c58 100644
>--- a/Source/JavaScriptCore/parser/VariableEnvironment.cpp
>+++ b/Source/JavaScriptCore/parser/VariableEnvironment.cpp
>@@ -141,6 +141,66 @@ bool VariableEnvironment::declarePrivateMethod(const RefPtr<UniquedStringImpl>&
>     return !addResult.isNewEntry;
> }
> 
>+bool VariableEnvironment::declarePrivateSetter(const RefPtr<UniquedStringImpl>& identifier)
>+{
>+    if (!m_rareData)
>+        m_rareData = std::make_unique<VariableEnvironmentRareData>();
>+
>+    auto findResult = m_rareData->m_privateNames.find(identifier);
>+
>+    if (findResult == m_rareData->m_privateNames.end()) {
>+        PrivateNameEntry newEntry(PrivateNameEntry::Traits::IsDeclared | PrivateNameEntry::Traits::IsSetter);
>+        m_rareData->m_privateNames.add(identifier, newEntry);
>+        return true;
>+    }
>+
>+    PrivateNameEntry currentEntry = findResult->value;
>+    if (currentEntry.isDeclared()) {
>+        if (currentEntry.isSetter() || currentEntry.isMethod() || !currentEntry.isGetter())
>+            return false; // Error: declaring a duplicate private name.
>+
>+        ASSERT(currentEntry.isGetter());
>+        PrivateNameEntry newEntry(currentEntry.bits() | PrivateNameEntry::Traits::IsSetter);
>+        m_rareData->m_privateNames.set(identifier, newEntry);
>+        return true;
>+    }
>+
>+    // it was previously used, mark it as declared.
>+    PrivateNameEntry newEntry(currentEntry.bits() | PrivateNameEntry::Traits::IsDeclared | PrivateNameEntry::Traits::IsSetter);
>+    m_rareData->m_privateNames.set(identifier, newEntry);
>+    return true;
>+}
>+
>+bool VariableEnvironment::declarePrivateGetter(const RefPtr<UniquedStringImpl>& identifier)
>+{
>+    if (!m_rareData)
>+        m_rareData = std::make_unique<VariableEnvironmentRareData>();
>+
>+    auto findResult = m_rareData->m_privateNames.find(identifier);
>+
>+    if (findResult == m_rareData->m_privateNames.end()) {
>+        PrivateNameEntry newEntry(PrivateNameEntry::Traits::IsDeclared | PrivateNameEntry::Traits::IsGetter);
>+        m_rareData->m_privateNames.add(identifier, newEntry);
>+        return true;
>+    }
>+
>+    PrivateNameEntry currentEntry = findResult->value;
>+    if (currentEntry.isDeclared()) {
>+        if (currentEntry.isGetter() || currentEntry.isMethod() || !currentEntry.isSetter())
>+            return false; // Error: declaring a duplicate private name.
>+
>+        ASSERT(currentEntry.isSetter());
>+        PrivateNameEntry newEntry(currentEntry.bits() | PrivateNameEntry::Traits::IsGetter);
>+        m_rareData->m_privateNames.set(identifier, newEntry);
>+        return true;
>+    }
>+
>+    // it was previously used, mark it as declared.
>+    PrivateNameEntry newEntry(currentEntry.bits() | PrivateNameEntry::Traits::IsDeclared | PrivateNameEntry::Traits::IsGetter);
>+    m_rareData->m_privateNames.set(identifier, newEntry);
>+    return true;
>+}
>+
> bool VariableEnvironment::declarePrivateName(const RefPtr<UniquedStringImpl>& identifier)
> {
>     if (!m_rareData)
>@@ -252,7 +312,7 @@ VariableEnvironment CompactVariableEnvironment::toVariableEnvironment() const
>         addResult.iterator->value = m_variableMetadata[i];
>     }
> 
>-     for (size_t i = 0; i < m_privateAccessNames.size(); i++) {
>+    for (size_t i = 0; i < m_privateAccessNames.size(); i++) {
>         auto addResult = result.addPrivateName(m_privateAccessNames[i]);
>         ASSERT(addResult.isNewEntry);
>         addResult.iterator->value = m_privateNameMetadata[i];
>diff --git a/Source/JavaScriptCore/parser/VariableEnvironment.h b/Source/JavaScriptCore/parser/VariableEnvironment.h
>index 5385ee0a258..72ac869894a 100644
>--- a/Source/JavaScriptCore/parser/VariableEnvironment.h
>+++ b/Source/JavaScriptCore/parser/VariableEnvironment.h
>@@ -94,8 +94,10 @@ public:
>     ALWAYS_INLINE bool isUsed() const { return m_bits & IsUsed; }
>     ALWAYS_INLINE bool isDeclared() const { return m_bits & IsDeclared; }
>     ALWAYS_INLINE bool isMethod() const { return m_bits & IsMethod; }
>+    ALWAYS_INLINE bool isSetter() const { return m_bits & IsSetter; }
>+    ALWAYS_INLINE bool isGetter() const { return m_bits & IsGetter; }
> 
>-    bool isPrivateAccess() const { return isMethod(); }
>+    bool isPrivateAccess() const { return isMethod() || isSetter() || isGetter(); }
> 
>     ALWAYS_INLINE void setIsUsed() { m_bits |= IsUsed; }
>     ALWAYS_INLINE void setIsDeclared() { m_bits |= IsDeclared; }
>@@ -111,6 +113,8 @@ public:
>         IsUsed = 1 << 0,
>         IsDeclared = 1 << 1,
>         IsMethod = 1 << 2,
>+        IsGetter = 1 << 3,
>+        IsSetter = 1 << 4,
>     };
> 
> private:
>@@ -307,7 +311,6 @@ private:
>     Vector<VariableEnvironmentEntry> m_variableMetadata;
>     Vector<RefPtr<UniquedStringImpl>> m_privateAccessNames;
>     Vector<PrivateNameEntry> m_privateNameMetadata;
>-
>     unsigned m_hash;
>     bool m_isEverythingCaptured;
> };

Actions: View | Formatted Diff | Diff

Attachments on bug 194435: 370695 | 373127 | 391861 | 391866 | 419845 | 419866 | 419911 | 419994 | 420052 | 420127 | 420350