[regexp] Add 'groups' property to JSRegExpResult objects

See https://github.com/tc39/proposal-regexp-named-groups/pull/40.

The spec is being changed to always create a 'groups' property on
regexp result objects. Its value is undefined if no named captures
exist, and the object containing named captures otherwise.

Bug: v8:7192, v8:5437
Change-Id: I1fb00ffc186c7effd84b5692dcbed420581855c3
Reviewed-on: https://chromium-review.googlesource.com/829137Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50154}

src/bootstrapper.cc

@@ -4832,9 +4832,9 @@ bool Genesis::InstallNatives(GlobalContextType context_type) {
   }
 
   // Create a constructor for RegExp results (a variant of Array that
-  // predefines the two properties index and match).
+  // predefines the properties index, input, and groups).
   {
-    // RegExpResult initial map.
+    // JSRegExpResult initial map.
 
     // Find global.Array.prototype to inherit from.
     Handle<JSFunction> array_constructor(native_context()->array_function());
@@ -4843,16 +4843,20 @@ bool Genesis::InstallNatives(GlobalContextType context_type) {
 
     // Add initial map.
     Handle<Map> initial_map = factory()->NewMap(
-        JS_ARRAY_TYPE, JSRegExpResult::kSize, TERMINAL_FAST_ELEMENTS_KIND, 2);
+        JS_ARRAY_TYPE, JSRegExpResult::kSize, TERMINAL_FAST_ELEMENTS_KIND,
+        JSRegExpResult::kInObjectPropertyCount);
     initial_map->SetConstructor(*array_constructor);
 
     // Set prototype on map.
     initial_map->set_has_non_instance_prototype(false);
     Map::SetPrototype(initial_map, array_prototype);
 
-    // Update map with length accessor from Array and add "index" and "input".
-    Map::EnsureDescriptorSlack(initial_map, 3);
+    // Update map with length accessor from Array and add "index", "input" and
+    // "groups".
+    Map::EnsureDescriptorSlack(initial_map,
+                               JSRegExpResult::kInObjectPropertyCount + 1);
 
+    // length descriptor.
     {
       JSFunction* array_function = native_context()->array_function();
       Handle<DescriptorArray> array_descriptors(
@@ -4866,6 +4870,8 @@ bool Genesis::InstallNatives(GlobalContextType context_type) {
           array_descriptors->GetDetails(old).attributes());
       initial_map->AppendDescriptor(&d);
     }
+
+    // index descriptor.
     {
       Descriptor d = Descriptor::DataField(factory()->index_string(),
                                            JSRegExpResult::kIndexIndex, NONE,
@@ -4873,6 +4879,7 @@ bool Genesis::InstallNatives(GlobalContextType context_type) {
       initial_map->AppendDescriptor(&d);
     }
 
+    // input descriptor.
     {
       Descriptor d = Descriptor::DataField(factory()->input_string(),
                                            JSRegExpResult::kInputIndex, NONE,
@@ -4880,6 +4887,14 @@ bool Genesis::InstallNatives(GlobalContextType context_type) {
       initial_map->AppendDescriptor(&d);
     }
 
+    // groups descriptor.
+    {
+      Descriptor d = Descriptor::DataField(factory()->groups_string(),
+                                           JSRegExpResult::kGroupsIndex, NONE,
+                                           Representation::Tagged());
+      initial_map->AppendDescriptor(&d);
+    }
+
     native_context()->set_regexp_result_map(*initial_map);
   }
 

src/builtins/builtins-regexp-gen.cc

@@ -62,15 +62,15 @@ Node* RegExpBuiltinsAssembler::AllocateRegExpResult(Node* context, Node* length,
       LoadContextElement(native_context, Context::REGEXP_RESULT_MAP_INDEX);
   StoreMapNoWriteBarrier(result, map);
 
-  Node* const empty_array = EmptyFixedArrayConstant();
-  DCHECK(Heap::RootIsImmortalImmovable(Heap::kEmptyFixedArrayRootIndex));
   StoreObjectFieldNoWriteBarrier(result, JSArray::kPropertiesOrHashOffset,
-                                 empty_array);
+                                 EmptyFixedArrayConstant());
   StoreObjectFieldNoWriteBarrier(result, JSArray::kElementsOffset, elements);
   StoreObjectFieldNoWriteBarrier(result, JSArray::kLengthOffset, length);
 
   StoreObjectFieldNoWriteBarrier(result, JSRegExpResult::kIndexOffset, index);
-  StoreObjectField(result, JSRegExpResult::kInputOffset, input);
+  StoreObjectFieldNoWriteBarrier(result, JSRegExpResult::kInputOffset, input);
+  StoreObjectFieldNoWriteBarrier(result, JSRegExpResult::kGroupsOffset,
+                                 UndefinedConstant());
 
   // Initialize the elements.
 
@@ -223,8 +223,6 @@ Node* RegExpBuiltinsAssembler::ConstructNewResultFromMatchInfo(
     // Allocate a new object to store the named capture properties.
     // TODO(jgruber): Could be optimized by adding the object map to the heap
     // root list.
-    // TODO(jgruber): Replace CreateDataProperty runtime calls once we have
-    // equivalent functionality in CSA.
 
     Node* const native_context = LoadNativeContext(context);
     Node* const map = LoadContextElement(
@@ -233,14 +231,7 @@ Node* RegExpBuiltinsAssembler::ConstructNewResultFromMatchInfo(
         AllocateNameDictionary(NameDictionary::kInitialCapacity);
 
     Node* const group_object = AllocateJSObjectFromMap(map, properties);
-
-    // Store it on the result as a 'group' property.
-
-    {
-      Node* const name = HeapConstant(isolate()->factory()->groups_string());
-      CallRuntime(Runtime::kCreateDataProperty, context, result, name,
-                  group_object);
-    }
+    StoreObjectField(result, JSRegExpResult::kGroupsOffset, group_object);
 
     // One or more named captures exist, add a property for each one.
 
@@ -267,6 +258,9 @@ Node* RegExpBuiltinsAssembler::ConstructNewResultFromMatchInfo(
       Node* const capture =
           LoadFixedArrayElement(result_elements, SmiUntag(index));
 
+      // TODO(jgruber): Calling into runtime to create each property is slow.
+      // Either we should create properties entirely in CSA (should be doable),
+      // or only call runtime once and loop there.
       CallRuntime(Runtime::kCreateDataProperty, context, group_object, name,
                   capture);
 

src/objects/js-regexp.h

@@ -144,13 +144,20 @@ DEFINE_OPERATORS_FOR_FLAGS(JSRegExp::Flags)
 // After creation the result must be treated as a JSArray in all regards.
 class JSRegExpResult : public JSArray {
  public:
-  // Offsets of object fields.
-  static const int kIndexOffset = JSArray::kSize;
-  static const int kInputOffset = kIndexOffset + kPointerSize;
-  static const int kSize = kInputOffset + kPointerSize;
+#define REG_EXP_RESULT_FIELDS(V) \
+  V(kIndexOffset, kPointerSize)  \
+  V(kInputOffset, kPointerSize)  \
+  V(kGroupsOffset, kPointerSize) \
+  V(kSize, 0)
+
+  DEFINE_FIELD_OFFSET_CONSTANTS(JSArray::kSize, REG_EXP_RESULT_FIELDS)
+#undef REG_EXP_RESULT_FIELDS
+
   // Indices of in-object properties.
   static const int kIndexIndex = 0;
   static const int kInputIndex = 1;
+  static const int kGroupsIndex = 2;
+  static const int kInObjectPropertyCount = 3;
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSRegExpResult);

test/mjsunit/harmony/regexp-named-captures.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-regexp-named-captures
+// Flags: --harmony-regexp-named-captures --allow-natives-syntax
 
 // Malformed named captures.
 assertThrows("/(?<>a)/u", SyntaxError);  // Empty name.
@@ -418,3 +418,124 @@ function toSlowMode(re) {
   assertEquals("cd", "abcd".replace(re, "$<fth>"));
   assertEquals("cd", "abcd".replace(re, "$<$1>"));
 }
+
+// Tests for 'groups' semantics on the regexp result object.
+// https://crbug.com/v8/7192
+
+{
+  const re = /./;
+  const result = re.exec("a");
+  assertTrue(%SpeciesProtector());
+  assertEquals(result.__proto__, Array.prototype);
+  assertTrue(result.hasOwnProperty('groups'));
+  assertArrayEquals(["a"], result);
+  assertEquals(0, result.index);
+  assertEquals(undefined, result.groups);
+
+  Array.prototype.groups = { a: "b" };
+  assertTrue(%SpeciesProtector());
+  assertEquals("$<a>", "a".replace(re, "$<a>"));
+  Array.prototype.groups = undefined;
+}
+
+{
+  const re = toSlowMode(/./);
+  const result = re.exec("a");
+  assertTrue(%SpeciesProtector());
+  assertEquals(result.__proto__, Array.prototype);
+  assertTrue(result.hasOwnProperty('groups'));
+  assertArrayEquals(["a"], result);
+  assertEquals(0, result.index);
+  assertEquals(undefined, result.groups);
+
+  Array.prototype.groups = { a: "b" };
+  assertTrue(%SpeciesProtector());
+  assertEquals("$<a>", "a".replace(re, "$<a>"));
+  Array.prototype.groups = undefined;
+}
+
+{
+  const re = /(?<a>a).|(?<x>x)/;
+  const result = re.exec("ab");
+  assertTrue(%SpeciesProtector());
+  assertEquals(result.__proto__, Array.prototype);
+  assertTrue(result.hasOwnProperty('groups'));
+  assertArrayEquals(["ab", "a", undefined], result);
+  assertEquals(0, result.index);
+  assertEquals({a: "a", x: undefined}, result.groups);
+
+  // a is a matched named capture, b is an unmatched named capture, and z
+  // is not a named capture.
+  Array.prototype.groups = { a: "b", x: "y", z: "z" };
+  assertTrue(%SpeciesProtector());
+  assertEquals("a", "ab".replace(re, "$<a>"));
+  assertEquals("", "ab".replace(re, "$<x>"));
+  assertEquals("", "ab".replace(re, "$<z>"));
+  Array.prototype.groups = undefined;
+}
+
+{
+  const re = toSlowMode(/(?<a>a).|(?<x>x)/);
+  const result = re.exec("ab");
+  assertTrue(%SpeciesProtector());
+  assertEquals(result.__proto__, Array.prototype);
+  assertTrue(result.hasOwnProperty('groups'));
+  assertArrayEquals(["ab", "a", undefined], result);
+  assertEquals(0, result.index);
+  assertEquals({a: "a", x: undefined}, result.groups);
+
+  // a is a matched named capture, b is an unmatched named capture, and z
+  // is not a named capture.
+  Array.prototype.groups = { a: "b", x: "y", z: "z" };
+  assertTrue(%SpeciesProtector());
+  assertEquals("a", "ab".replace(re, "$<a>"));
+  assertEquals("", "ab".replace(re, "$<x>"));
+  assertEquals("", "ab".replace(re, "$<z>"));
+  Array.prototype.groups = undefined;
+}
+
+{
+  class FakeRegExp extends RegExp {
+    exec(subject) {
+      const fake_result = [ "ab", "a" ];
+      fake_result.index = 0;
+      // groups is not set, triggering prototype lookup.
+      return fake_result;
+    }
+  };
+
+  const re = new FakeRegExp();
+  const result = re.exec("ab");
+  assertTrue(%SpeciesProtector());
+  assertEquals(result.__proto__, Array.prototype);
+  assertFalse(result.hasOwnProperty('groups'));
+
+  Array.prototype.groups = { a: "b" };
+  Array.prototype.groups.__proto__.b = "c";
+  assertTrue(%SpeciesProtector());
+  assertEquals("b", "ab".replace(re, "$<a>"));
+  assertEquals("c", "ab".replace(re, "$<b>"));
+  Array.prototype.groups = undefined;
+}
+
+{
+  class FakeRegExp extends RegExp {
+    exec(subject) {
+      const fake_result = [ "ab", "a" ];
+      fake_result.index = 0;
+      fake_result.groups = { a: "b" };
+      fake_result.groups.__proto__.b = "c";
+      return fake_result;
+    }
+  };
+
+  const re = new FakeRegExp();
+  const result = re.exec("ab");
+  assertTrue(%SpeciesProtector());
+  assertEquals(result.__proto__, Array.prototype);
+  assertTrue(result.hasOwnProperty('groups'));
+  assertEquals({ a: "b" }, result.groups);
+
+  assertEquals("b", "ab".replace(re, "$<a>"));
+  assertEquals("c", "ab".replace(re, "$<b>"));
+}