Revert of [regexp] Create property on result for each named capture (patchset #9 id:160001 of https://codereview.chromium.org/2630233003/ )
Reason for revert:
Some heap tests are broken leading to failures on nosnap builds:
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20nosnap%20-%20debug/builds/3677
Reverting again until tests are fixed to keep bots green.
Original issue's description:
> [regexp] Store named captures on the regexp result
>
> This implements storing named captures on the regexp result object.
> For instance, /(?<a>.)/u.exec("b") will return a result such that:
>
> result.group.a // "b"
>
> https://tc39.github.io/proposal-regexp-named-groups/
>
> BUG=v8:5437
>
> Review-Url: https://codereview.chromium.org/2630233003
> Cr-Original-Original-Commit-Position: refs/heads/master@{#42532}
> Committed: https://chromium.googlesource.com/v8/v8/+/70000946eb2a9155679528702a766219a1fcf154
> Review-Url: https://codereview.chromium.org/2630233003
> Cr-Original-Commit-Position: refs/heads/master@{#42570}
> Committed: https://chromium.googlesource.com/v8/v8/+/ee94fa11ed63477393c6534c352ebac6e502396c
> Review-Url: https://codereview.chromium.org/2630233003
> Cr-Commit-Position: refs/heads/master@{#42676}
> Committed: https://chromium.googlesource.com/v8/v8/+/8bf52534f6bf86821a1589dcbcb7335052c1f94f
TBR=yangguo@chromium.org,littledan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5437
Review-Url: https://codereview.chromium.org/2654233002
Cr-Commit-Position: refs/heads/master@{#42681}
diff --git a/src/builtins/builtins-regexp.cc b/src/builtins/builtins-regexp.cc
index df9bc00..eba57aa 100644
--- a/src/builtins/builtins-regexp.cc
+++ b/src/builtins/builtins-regexp.cc
@@ -33,9 +33,8 @@
void StoreLastIndex(Node* context, Node* regexp, Node* value,
bool is_fastpath);
- Node* ConstructNewResultFromMatchInfo(Node* const context, Node* const regexp,
- Node* const match_info,
- Node* const string);
+ Node* ConstructNewResultFromMatchInfo(Node* context, Node* match_info,
+ Node* string);
Node* RegExpPrototypeExecBodyWithoutResult(Node* const context,
Node* const regexp,
@@ -142,10 +141,10 @@
}
}
-Node* RegExpBuiltinsAssembler::ConstructNewResultFromMatchInfo(
- Node* const context, Node* const regexp, Node* const match_info,
- Node* const string) {
- Label named_captures(this), out(this);
+Node* RegExpBuiltinsAssembler::ConstructNewResultFromMatchInfo(Node* context,
+ Node* match_info,
+ Node* string) {
+ Label out(this);
Node* const num_indices = SmiUntag(LoadFixedArrayElement(
match_info, RegExpMatchInfo::kNumberOfCapturesIndex));
@@ -165,8 +164,7 @@
StoreFixedArrayElement(result_elements, 0, first, SKIP_WRITE_BARRIER);
- // If no captures exist we can skip named capture handling as well.
- GotoIf(SmiEqual(num_results, SmiConstant(1)), &out);
+ GotoIf(SmiEqual(num_results, SmiConstant(Smi::FromInt(1))), &out);
// Store all remaining captures.
Node* const limit = IntPtrAdd(
@@ -189,7 +187,7 @@
Node* const start = LoadFixedArrayElement(match_info, from_cursor);
Label next_iter(this);
- GotoIf(SmiEqual(start, SmiConstant(-1)), &next_iter);
+ GotoIf(SmiEqual(start, SmiConstant(Smi::FromInt(-1))), &next_iter);
Node* const from_cursor_plus1 = IntPtrAdd(from_cursor, IntPtrConstant(1));
Node* const end = LoadFixedArrayElement(match_info, from_cursor_plus1);
@@ -201,83 +199,7 @@
Bind(&next_iter);
var_from_cursor.Bind(IntPtrAdd(from_cursor, IntPtrConstant(2)));
var_to_cursor.Bind(IntPtrAdd(to_cursor, IntPtrConstant(1)));
- Branch(UintPtrLessThan(var_from_cursor.value(), limit), &loop,
- &named_captures);
- }
-
- Bind(&named_captures);
- {
- // We reach this point only if captures exist, implying that this is an
- // IRREGEXP JSRegExp.
-
- CSA_ASSERT(this, HasInstanceType(regexp, JS_REGEXP_TYPE));
- CSA_ASSERT(this, SmiGreaterThan(num_results, SmiConstant(1)));
-
- // Preparations for named capture properties. Exit early if the result does
- // not have any named captures to minimize performance impact.
-
- Node* const data = LoadObjectField(regexp, JSRegExp::kDataOffset);
- CSA_ASSERT(this, SmiEqual(LoadFixedArrayElement(data, JSRegExp::kTagIndex),
- SmiConstant(JSRegExp::IRREGEXP)));
-
- // The names fixed array associates names at even indices with a capture
- // index at odd indices.
- Node* const names =
- LoadFixedArrayElement(data, JSRegExp::kIrregexpCaptureNameMapIndex);
- GotoIf(SmiEqual(names, SmiConstant(0)), &out);
-
- // 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.
-
- Node* const native_context = LoadNativeContext(context);
- Node* const map = LoadContextElement(
- native_context, Context::SLOW_OBJECT_WITH_NULL_PROTOTYPE_MAP);
- Node* const properties =
- 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()->group_string());
- CallRuntime(Runtime::kCreateDataProperty, context, result, name,
- group_object);
- }
-
- // One or more named captures exist, add a property for each one.
-
- CSA_ASSERT(this, HasInstanceType(names, FIXED_ARRAY_TYPE));
- Node* const names_length = LoadAndUntagFixedArrayBaseLength(names);
- CSA_ASSERT(this, IntPtrGreaterThan(names_length, IntPtrConstant(0)));
-
- Variable var_i(this, MachineType::PointerRepresentation());
- var_i.Bind(IntPtrConstant(0));
-
- Variable* vars[] = {&var_i};
- const int vars_count = sizeof(vars) / sizeof(vars[0]);
- Label loop(this, vars_count, vars);
-
- Goto(&loop);
- Bind(&loop);
- {
- Node* const i = var_i.value();
- Node* const i_plus_1 = IntPtrAdd(i, IntPtrConstant(1));
- Node* const i_plus_2 = IntPtrAdd(i_plus_1, IntPtrConstant(1));
-
- Node* const name = LoadFixedArrayElement(names, i);
- Node* const index = LoadFixedArrayElement(names, i_plus_1);
- Node* const capture =
- LoadFixedArrayElement(result_elements, SmiUntag(index));
-
- CallRuntime(Runtime::kCreateDataProperty, context, group_object, name,
- capture);
-
- var_i.Bind(i_plus_2);
- Branch(IntPtrGreaterThanOrEqual(var_i.value(), names_length), &out,
- &loop);
- }
+ Branch(UintPtrLessThan(var_from_cursor.value(), limit), &loop, &out);
}
Bind(&out);
@@ -430,7 +352,7 @@
{
Node* const match_indices = indices_or_null;
Node* const result =
- ConstructNewResultFromMatchInfo(context, regexp, match_indices, string);
+ ConstructNewResultFromMatchInfo(context, match_indices, string);
var_result.Bind(result);
Goto(&out);
}
@@ -2588,7 +2510,7 @@
Bind(&if_matched);
{
Node* result =
- ConstructNewResultFromMatchInfo(context, regexp, match_indices, string);
+ ConstructNewResultFromMatchInfo(context, match_indices, string);
Return(result);
}
}
diff --git a/src/heap-symbols.h b/src/heap-symbols.h
index 1d381a3..4b55b50 100644
--- a/src/heap-symbols.h
+++ b/src/heap-symbols.h
@@ -88,7 +88,6 @@
V(get_string, "get") \
V(get_space_string, "get ") \
V(global_string, "global") \
- V(group_string, "group") \
V(has_string, "has") \
V(hour_string, "hour") \
V(ignoreCase_string, "ignoreCase") \
diff --git a/test/mjsunit/harmony/regexp-named-captures.js b/test/mjsunit/harmony/regexp-named-captures.js
index 4404bd4..ced8e4b 100644
--- a/test/mjsunit/harmony/regexp-named-captures.js
+++ b/test/mjsunit/harmony/regexp-named-captures.js
@@ -18,15 +18,15 @@
assertThrows("/\\k<a>(?<ab>a)/u"); // Invalid reference.
// Fallback behavior in non-unicode mode.
-assertThrows("/(?<>a)/", SyntaxError);
-assertThrows("/(?<aa)/", SyntaxError);
-assertThrows("/(?<42a>a)/", SyntaxError);
-assertThrows("/(?<:a>a)/", SyntaxError);
-assertThrows("/(?<a:>a)/", SyntaxError);
-assertThrows("/(?<a>a)(?<a>a)/", SyntaxError);
-assertThrows("/(?<a>a)(?<b>b)(?<a>a)/", SyntaxError);
-assertThrows("/(?<a>a)\\k<ab>/", SyntaxError);
-assertThrows("/(?<ab>a)\\k<a>/", SyntaxError);
+assertThrows("/(?<>a)/");
+assertThrows("/(?<aa)/");
+assertThrows("/(?<42a>a)/");
+assertThrows("/(?<:a>a)/");
+assertThrows("/(?<a:>a)/");
+assertThrows("/(?<a>a)(?<a>a)/");
+assertThrows("/(?<a>a)(?<b>b)(?<a>a)/");
+assertThrows("/(?<a>a)\\k<ab>/");
+assertThrows("/(?<ab>a)\\k<a>/");
assertEquals(["k<a>"], "xxxk<a>xxx".match(/\k<a>/));
assertEquals(["k<a"], "xxxk<a>xxx".match(/\k<a/));
@@ -74,25 +74,3 @@
// Reference before group.
assertEquals(["bab", "b"], "bab".match(/\k<a>(?<a>b)\w\k<a>/u));
assertEquals(["bab", "b", "a"], "bab".match(/(?<b>b)\k<a>(?<a>a)\k<b>/u));
-
-// Reference properties.
-assertEquals("a", /(?<a>a)(?<b>b)\k<a>/u.exec("aba").group.a);
-assertEquals("b", /(?<a>a)(?<b>b)\k<a>/u.exec("aba").group.b);
-assertEquals(undefined, /(?<a>a)(?<b>b)\k<a>/u.exec("aba").group.c);
-assertEquals(undefined, /(?<a>a)(?<b>b)\k<a>|(?<c>c)/u.exec("aba").group.c);
-
-// Unicode names.
-assertEquals("a", /(?<π>a)/u.exec("bab").group.π);
-assertEquals("a", /(?<\u{03C0}>a)/u.exec("bab").group.\u03C0);
-assertEquals("a", /(?<$>a)/u.exec("bab").group.$);
-assertEquals("a", /(?<_>a)/u.exec("bab").group._);
-assertEquals("a", /(?<$𐒤>a)/u.exec("bab").group.$𐒤);
-assertEquals("a", /(?<_\u200C>a)/u.exec("bab").group._\u200C);
-assertEquals("a", /(?<_\u200D>a)/u.exec("bab").group._\u200D);
-assertEquals("a", /(?<ಠ_ಠ>a)/u.exec("bab").group.ಠ_ಠ);
-assertThrows('/(?<❤>a)/u', SyntaxError);
-assertThrows('/(?<𐒤>a)/u', SyntaxError); // ID_Continue but not ID_Start.
-
-// The '__proto__' property on the groups object.
-assertEquals(undefined, /(?<a>.)/u.exec("a").group.__proto__);
-assertEquals("a", /(?<__proto__>a)/u.exec("a").group.__proto__);
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index 31e118b..9f5d73e 100644
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -156,9 +156,6 @@
# desugaring regexp property class relies on ICU.
'harmony/regexp-property-*': [PASS, ['no_i18n == True', FAIL]],
- # noi18n build cannot parse characters in supplementary plane.
- 'harmony/regexp-named-captures': [PASS, ['no_i18n == True', FAIL]],
-
# Allocates a large array buffer, which TSAN sometimes cannot handle.
'regress/regress-599717': [PASS, ['tsan', SKIP]],
