[class] Fix compound assignment w/ private accessors

The original commit implementing private accessor propertiers (*) claims
it's not a thing, but it is.

(*) https://chromium-review.googlesource.com/c/v8/v8/+/1695205/11/src/interpreter/bytecode-generator.cc#3959

Bug: v8:11360, v8:8330
Change-Id: If497f2b6a77dc28e4ade4ef78d901299f4e37593
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2652495Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Joyee Cheung <joyee@igalia.com>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72411}

src/interpreter/bytecode-generator.cc

@@ -4135,12 +4135,47 @@ void BytecodeGenerator::VisitCompoundAssignment(CompoundAssignment* expr) {
                              lhs_data.super_property_args().Truncate(3));
       break;
     }
-    case PRIVATE_METHOD:
-    case PRIVATE_GETTER_ONLY:
-    case PRIVATE_SETTER_ONLY:
+    case PRIVATE_METHOD: {
+      // The property access is invalid, but if the brand check fails too, we
+      // need to return the error from the brand check.
+      Property* property = lhs_data.expr()->AsProperty();
+      Register object = VisitForRegisterValue(property->obj());
+      BuildPrivateBrandCheck(property, object,
+                             MessageTemplate::kInvalidPrivateMemberRead);
+      BuildInvalidPropertyAccess(MessageTemplate::kInvalidPrivateMethodWrite,
+                                 lhs_data.expr()->AsProperty());
+      break;
+    }
+    case PRIVATE_GETTER_ONLY: {
+      // The property access is invalid, but if the brand check fails too, we
+      // need to return the error from the brand check.
+      Property* property = lhs_data.expr()->AsProperty();
+      Register object = VisitForRegisterValue(property->obj());
+      BuildPrivateBrandCheck(property, object,
+                             MessageTemplate::kInvalidPrivateMemberRead);
+      BuildInvalidPropertyAccess(MessageTemplate::kInvalidPrivateSetterAccess,
+                                 lhs_data.expr()->AsProperty());
+
+      break;
+    }
+    case PRIVATE_SETTER_ONLY: {
+      // The property access is invalid, but if the brand check fails too, we
+      // need to return the error from the brand check.
+      Property* property = lhs_data.expr()->AsProperty();
+      Register object = VisitForRegisterValue(property->obj());
+      BuildPrivateBrandCheck(property, object,
+                             MessageTemplate::kInvalidPrivateMemberRead);
+      BuildInvalidPropertyAccess(MessageTemplate::kInvalidPrivateGetterAccess,
+                                 lhs_data.expr()->AsProperty());
+      break;
+    }
     case PRIVATE_GETTER_AND_SETTER: {
-      // ({ #foo: name } = obj) is currently syntactically invalid.
-      UNREACHABLE();
+      Property* property = lhs_data.expr()->AsProperty();
+      Register object = VisitForRegisterValue(property->obj());
+      Register key = VisitForRegisterValue(property->key());
+      BuildPrivateBrandCheck(property, object,
+                             MessageTemplate::kInvalidPrivateMemberRead);
+      BuildPrivateGetterAccess(object, key);
       break;
     }
   }

test/mjsunit/regress/regress-v8-11360.js

+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(function TestCompoundAssignmentToPrivateField() {
+  class C {
+    #foo = 1;
+    m() {
+      return this.#foo += 1;
+    }
+  }
+
+  assertEquals(2, (new C()).m());
+})();
+
+(function TestCompoundAssignmentToPrivateFieldWithOnlyGetter() {
+  class C {
+    get #foo() { return 1; }
+    m() {
+      return this.#foo += 1;
+    }
+  }
+
+  assertThrows(() => { (new C()).m(); });
+})();
+
+(function TestCompoundAssignmentToPrivateFieldWithOnlySetter() {
+  class C {
+    set #foo(a) { }
+    m() {
+      return this.#foo += 1;
+    }
+  }
+
+  assertThrows(() => { (new C()).m(); });
+})();
+
+(function TestCompoundAssignmentToPrivateFieldWithGetterAndSetter() {
+  class C {
+    get #foo() { return 1; }
+    set #foo(a) { }
+    m() {
+      return this.#foo += 1;
+    }
+  }
+
+  assertEquals(2, (new C()).m());
+})();
+
+(function TestCompoundAssignmentToPrivateMethod() {
+  class C {
+    m() {
+      return this.#pm += 1;
+    }
+    #pm() {}
+  }
+
+  assertThrows(() => { (new O()).m(); });
+})();
+
+(function TestCompoundAssignmentToStaticPrivateField() {
+  class C {
+    static #foo = 1;
+    m() {
+      return C.#foo += 1;
+    }
+  }
+
+  assertEquals(2, (new C()).m());
+})();
+
+(function TestCompoundAssignmentToStaticPrivateFieldWithOnlyGetter() {
+  class C {
+    static get #foo() { return 1; }
+    m() {
+      return C.#foo += 1;
+    }
+  }
+
+  assertThrows(() => { (new C()).m(); });
+})();
+
+(function TestCompoundAssignmentToStaticPrivateFieldWithOnlySetter() {
+  class C {
+    static set #foo(a) { }
+    m() {
+      return C.#foo += 1;
+    }
+  }
+
+  assertThrows(() => { (new C()).m(); });
+})();
+
+(function TestCompoundAssignmentToStaticPrivateFieldWithGetterAndSetter() {
+  class C {
+    static get #foo() { return 1; }
+    static set #foo(a) { }
+    m() {
+      return C.#foo += 1;
+    }
+  }
+
+  assertEquals(2, (new C()).m());
+})();
+
+(function TestCompoundAssignmentToStaticPrivateMethod() {
+  class C {
+    m() {
+      return C.#pm += 1;
+    }
+    static #pm() {}
+  }
+
+  assertThrows(() => { (new O()).m(); });
+})();
+
+// The following tests test the above cases w/ brand check failures.
+
+(function TestBrandCheck_CompoundAssignmentToPrivateField() {
+  class C {
+    #foo = 1;
+    m() {
+      return this.#foo += 1;
+    }
+  }
+
+  assertThrows(() => { C.prototype.m.call({}); }, TypeError,
+               /Cannot read private member/);
+
+  // It's the same error we get from this case:
+  class C2 {
+    #foo = 1;
+    m() {
+      return this.#foo;
+    }
+  }
+
+  assertThrows(() => { C2.prototype.m.call({}); }, TypeError,
+               /Cannot read private member/);
+})();
+
+(function TestBrandCheck_CompoundAssignmentToPrivateFieldWithOnlyGetter() {
+  class C {
+    get #foo() { return 1; }
+    m() {
+      return this.#foo += 1;
+    }
+  }
+
+  assertThrows(() => { C.prototype.m.call({}); }, TypeError,
+               /Object must be an instance of class/);
+
+  // It's the same error we get from this case:
+  class C2 {
+    get #foo() { return 1; }
+    m() {
+      return this.#foo;
+    }
+  }
+
+  assertThrows(() => { C2.prototype.m.call({}); }, TypeError,
+               /Object must be an instance of class/);
+})();
+
+(function TestBrandCheck_CompoundAssignmentToPrivateFieldWithOnlySetter() {
+  class C {
+    set #foo(a) { }
+    m() {
+      return this.#foo += 1;
+    }
+  }
+
+  assertThrows(() => { C.prototype.m.call({}); }, TypeError,
+               /Object must be an instance of class/);
+})();
+
+(function TestBrandCheck_CompoundAssignmentToPrivateFieldWithGetterAndSetter() {
+  class C {
+    get #foo() { return 1; }
+    set #foo(a) { }
+    m() {
+      return this.#foo += 1;
+    }
+  }
+
+  assertThrows(() => { C.prototype.m.call({}); }, TypeError,
+               /Object must be an instance of class/);
+
+  // It's the same error we get from this case:
+  class C2 {
+    get #foo() { return 1; }
+    set #foo(a) { }
+    m() {
+      return this.#foo;
+    }
+  }
+
+  assertThrows(() => { C2.prototype.m.call({}); }, TypeError,
+               /Object must be an instance of class/);
+})();
+
+(function TestBrandCheck_CompoundAssignmentToPrivateMethod() {
+  class C {
+    m() {
+      return this.#pm += 1;
+    }
+    #pm() {}
+  }
+
+  assertThrows(() => { C.prototype.m.call({}); }, TypeError,
+               /Object must be an instance of class/);
+})();