[classes] Fix DCHECK for a case when data property overwrites one accessor

Also add comments and regression tests.

Bug: chromium:904272
Change-Id: I89e8ec537bbdce09fda120cd29d5a5e54e77cf19
Reviewed-on: https://chromium-review.googlesource.com/c/1335556
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57500}

src/objects/literal-objects.cc

@@ -221,28 +221,35 @@ void AddToDictionaryTemplate(Isolate* isolate, Handle<Dictionary> dictionary,
             GetExistingValueIndex(current_pair->getter());
         int existing_setter_index =
             GetExistingValueIndex(current_pair->setter());
+        // At least one of the accessors must already be defined.
+        DCHECK(existing_getter_index >= 0 || existing_setter_index >= 0);
         if (existing_getter_index < key_index &&
             existing_setter_index < key_index) {
-          // Both getter and setter were defined before the computed method,
-          // so overwrite both.
+          // Either both getter and setter were defined before the computed
+          // method or just one of them was defined before while the other one
+          // was not defined yet, so overwrite property to kData.
           PropertyDetails details(kData, DONT_ENUM, PropertyCellType::kNoCell,
                                   enum_order);
           dictionary->DetailsAtPut(isolate, entry, details);
           dictionary->ValueAtPut(entry, value);
 
         } else {
+          // The data property was defined "between" accessors so the one that
+          // was overwritten has to be cleared.
           if (existing_getter_index < key_index) {
-            DCHECK_LT(existing_setter_index, key_index);
-            // Getter was defined before the computed method and then it was
-            // overwritten by the current computed method which in turn was
-            // later overwritten by the setter method. So we clear the getter.
+            DCHECK_LT(key_index, existing_setter_index);
+            // Getter was defined and it was done before the computed method
+            // and then it was overwritten by the current computed method which
+            // in turn was later overwritten by the setter method. So we clear
+            // the getter.
             current_pair->set_getter(*isolate->factory()->null_value());
 
           } else if (existing_setter_index < key_index) {
-            DCHECK_LT(existing_getter_index, key_index);
-            // Setter was defined before the computed method and then it was
-            // overwritten by the current computed method which in turn was
-            // later overwritten by the getter method. So we clear the setter.
+            DCHECK_LT(key_index, existing_getter_index);
+            // Setter was defined and it was done before the computed method
+            // and then it was overwritten by the current computed method which
+            // in turn was later overwritten by the getter method. So we clear
+            // the setter.
             current_pair->set_setter(*isolate->factory()->null_value());
           }
         }

test/mjsunit/es6/classes.js

@@ -1161,3 +1161,95 @@ function testClassRestrictedProperties(C) {
     assertEquals(instance[key], value);
   }
 })();
+
+var b = 'b';
+
+(function TestOverwritingInstanceAccessors() {
+  var C, desc;
+  C = class {
+    [b]() { return 'B'; };
+    get b() { return 'get B'; };
+  };
+  desc = Object.getOwnPropertyDescriptor(C.prototype, 'b');
+  assertFalse(desc.enumerable);
+  assertTrue(desc.configurable);
+  assertEquals('get B', desc.get());
+  assertEquals(undefined, desc.set);
+
+  C = class {
+    [b]() { return 'B'; };
+    set b(v) { return 'set B'; };
+  };
+  desc = Object.getOwnPropertyDescriptor(C.prototype, 'b');
+  assertFalse(desc.enumerable);
+  assertTrue(desc.configurable);
+  assertEquals(undefined, desc.get);
+  assertEquals('set B', desc.set());
+
+  C = class {
+    set b(v) { return 'get B'; };
+    [b]() { return 'B'; };
+    get b() { return 'get B'; };
+  };
+  desc = Object.getOwnPropertyDescriptor(C.prototype, 'b');
+  assertFalse(desc.enumerable);
+  assertTrue(desc.configurable);
+  assertEquals('get B', desc.get());
+  assertEquals(undefined, desc.set);
+
+  C = class {
+    get b() { return 'get B'; };
+    [b]() { return 'B'; };
+    set b(v) { return 'set B'; };
+  };
+  desc = Object.getOwnPropertyDescriptor(C.prototype, 'b');
+  assertFalse(desc.enumerable);
+  assertTrue(desc.configurable);
+  assertEquals(undefined, desc.get);
+  assertEquals('set B', desc.set());
+})();
+
+(function TestOverwritingStaticAccessors() {
+  var C, desc;
+  C = class {
+    static [b]() { return 'B'; };
+    static get b() { return 'get B'; };
+  };
+  desc = Object.getOwnPropertyDescriptor(C, 'b');
+  assertFalse(desc.enumerable);
+  assertTrue(desc.configurable);
+  assertEquals('get B', desc.get());
+  assertEquals(undefined, desc.set);
+
+  C = class {
+    static [b]() { return 'B'; };
+    static set b(v) { return 'set B'; };
+  };
+  desc = Object.getOwnPropertyDescriptor(C, 'b');
+  assertFalse(desc.enumerable);
+  assertTrue(desc.configurable);
+  assertEquals(undefined, desc.get);
+  assertEquals('set B', desc.set());
+
+  C = class {
+    static set b(v) { return 'get B'; };
+    static [b]() { return 'B'; };
+    static get b() { return 'get B'; };
+  };
+  desc = Object.getOwnPropertyDescriptor(C, 'b');
+  assertFalse(desc.enumerable);
+  assertTrue(desc.configurable);
+  assertEquals('get B', desc.get());
+  assertEquals(undefined, desc.set);
+
+  C = class {
+    static get b() { return 'get B'; };
+    static [b]() { return 'B'; };
+    static set b(v) { return 'set B'; };
+  };
+  desc = Object.getOwnPropertyDescriptor(C, 'b');
+  assertFalse(desc.enumerable);
+  assertTrue(desc.configurable);
+  assertEquals(undefined, desc.get);
+  assertEquals('set B', desc.set());
+})();