Commit d4574d18 authored by Joshua Litt's avatar Joshua Litt Committed by Commit Bot

Reland "[regexp] Clone match info for match indices."

This reverts commit d7793c06.

Reason for revert: This cl *will* cause regexp regressions. We are trying to gauge the real world impact.

Original change's description:
> Revert "[regexp] Clone match info for match indices."
>
> This reverts commit dfd9ceb9.
>
> Reason for revert: Regressions https://chromeperf.appspot.com/group_report?rev=64356 https://crbug.com/1015749
>
> Original change's description:
> > [regexp] Clone match info for match indices.
> >
> > The current behavior for generating match indices simply stashes a
> > pointer to the match info and then constructs the indices lazily.
> > However, it turns out the match info object used to create the result
> > object is the regexp_last_match_info living on native context, and thus
> > it can change between the creation of the result object and the generation
> > of indices. This cl clones the match info which will be safer.
> >
> > Bug: v8:9548
> > Change-Id: Ia6f26f88fbc22fd09671bf4c579d39a1510b552d
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864585
> > Commit-Queue: Joshua Litt <joshualitt@chromium.org>
> > Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#64356}
>
> TBR=jgruber@chromium.org,joshualitt@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: v8:9548, chromium:1015749
> Change-Id: I9c30b8fb459cf2aa89d920bf061614441250844d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1870236
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#64407}

TBR=jgruber@chromium.org,joshualitt@chromium.org


Bug: v8:9548, chromium:1015749
Change-Id: I151511307e3d8752fdbde4b8247514031b141b08
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1879587Reviewed-by: 's avatarJoshua Litt <joshualitt@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Commit-Queue: Joshua Litt <joshualitt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64587}
parent 2cc980d8
...@@ -84,6 +84,14 @@ TNode<JSRegExpResult> RegExpBuiltinsAssembler::AllocateRegExpResult( ...@@ -84,6 +84,14 @@ TNode<JSRegExpResult> RegExpBuiltinsAssembler::AllocateRegExpResult(
length, SmiConstant(JSArray::kMaxFastArrayLength))); length, SmiConstant(JSArray::kMaxFastArrayLength)));
CSA_ASSERT(this, SmiGreaterThan(length, SmiConstant(0))); CSA_ASSERT(this, SmiGreaterThan(length, SmiConstant(0)));
// Clone the match info so we can stash a pointer to it. This is necessary
// because it may point to the 'regexp_last_match_info' on native context and
// thus could change.
// TODO(v8:9870): Now that we clone the last match info, we can make its size
// static.
TNode<FixedArrayBase> cloned_match_info =
CloneFixedArray(match_info, ExtractFixedArrayFlag::kFixedArrays);
// Allocate. // Allocate.
const ElementsKind elements_kind = PACKED_ELEMENTS; const ElementsKind elements_kind = PACKED_ELEMENTS;
...@@ -117,10 +125,10 @@ TNode<JSRegExpResult> RegExpBuiltinsAssembler::AllocateRegExpResult( ...@@ -117,10 +125,10 @@ TNode<JSRegExpResult> RegExpBuiltinsAssembler::AllocateRegExpResult(
StoreObjectFieldNoWriteBarrier(result, JSRegExpResult::kNamesOffset, StoreObjectFieldNoWriteBarrier(result, JSRegExpResult::kNamesOffset,
undefined_value); undefined_value);
// Stash match_info in order to build JSRegExpResultIndices lazily when the // Stash cloned_match_info in order to build JSRegExpResultIndices lazily when
// 'indices' property is accessed. // the 'indices' property is accessed.
StoreObjectField(result, JSRegExpResult::kCachedIndicesOrMatchInfoOffset, StoreObjectField(result, JSRegExpResult::kCachedIndicesOrMatchInfoOffset,
match_info); cloned_match_info);
// Finish elements initialization. // Finish elements initialization.
......
...@@ -111,3 +111,12 @@ ...@@ -111,3 +111,12 @@
gc(); gc();
assertEquals(m.indices, [[0, 9]]); assertEquals(m.indices, [[0, 9]]);
} }
// Match between matches.
{
const re = /a+(?<A>zz)?(?<B>ii)?/;
const m = re.exec("xaaazzii");
assertTrue(/b+(?<C>cccc)?/.test("llllllbbbbbbcccc"));
assertEquals(m.indices, [[1, 8], [4, 6], [6, 8]]);
assertEquals(m.indices.groups, {'A': [4, 6], 'B': [6, 8]});
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment