Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2779033003:
[regexp] Stage named captures (Closed)
Description[regexp] Stage named captures
BUG=v8:5437
Review-Url: https://codereview.chromium.org/2779033003
Cr-Commit-Position: refs/heads/master@{#44331}
Committed: https://chromium.googlesource.com/v8/v8/+/3b716a08044da1073240cd287f35c8517fcf392f
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rebase and address comments #Patch Set 3 : Remove move_object_start implication as well now that it's shipped #Messages
Total messages: 27 (17 generated)
The CQ bit was checked by jgruber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jgruber@chromium.org changed reviewers: + adamk@chromium.org, yangguo@chromium.org
Adam, PTAL. Design doc: https://docs.google.com/document/d/1IpE1HK66uAibbZ8qhlxEQtwdxKftWUN9oJsV8t1S7...
On 2017/03/29 08:21:01, jgruber wrote: > Adam, PTAL. Design doc: > https://docs.google.com/document/d/1IpE1HK66uAibbZ8qhlxEQtwdxKftWUN9oJsV8t1S7... LGTM. Please wait for Adam's green light.
lgtm I guess this doesn't go on our Q2 OKRs then :) https://codereview.chromium.org/2779033003/diff/1/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/2779033003/diff/1/src/flag-definitions.h#newc... src/flag-definitions.h:198: DEFINE_IMPLICATION(es_staging, harmony_regexp_named_captures) This isn't needed. You can also remove the line above. es_staging already implies harmony, which implies everything under "HARMONY_STAGED" below.
On 2017/03/29 17:26:50, adamk wrote: > lgtm > > I guess this doesn't go on our Q2 OKRs then :) > > https://codereview.chromium.org/2779033003/diff/1/src/flag-definitions.h > File src/flag-definitions.h (right): > > https://codereview.chromium.org/2779033003/diff/1/src/flag-definitions.h#newc... > src/flag-definitions.h:198: DEFINE_IMPLICATION(es_staging, > harmony_regexp_named_captures) > This isn't needed. You can also remove the line above. es_staging already > implies harmony, which implies everything under "HARMONY_STAGED" below. Holding off on staging this since the decision was just made to support named references in non-unicode mode [0]. Probably makes sense to implement that first. [0] https://github.com/tc39/proposal-regexp-named-groups/issues/7#issuecomment-29...
On 2017/03/30 06:47:04, jgruber wrote: > On 2017/03/29 17:26:50, adamk wrote: > > lgtm > > > > I guess this doesn't go on our Q2 OKRs then :) > > > > https://codereview.chromium.org/2779033003/diff/1/src/flag-definitions.h > > File src/flag-definitions.h (right): > > > > > https://codereview.chromium.org/2779033003/diff/1/src/flag-definitions.h#newc... > > src/flag-definitions.h:198: DEFINE_IMPLICATION(es_staging, > > harmony_regexp_named_captures) > > This isn't needed. You can also remove the line above. es_staging already > > implies harmony, which implies everything under "HARMONY_STAGED" below. > > Holding off on staging this since the decision was just made to support named > references > in non-unicode mode [0]. Probably makes sense to implement that first. > > [0] > https://github.com/tc39/proposal-regexp-named-groups/issues/7#issuecomment-29... Support in non-unicode mode just landed at https://codereview.chromium.org/2788873002/.
The CQ bit was checked by jgruber@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
The CQ bit was checked by jgruber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2779033003/diff/1/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/2779033003/diff/1/src/flag-definitions.h#newc... src/flag-definitions.h:198: DEFINE_IMPLICATION(es_staging, harmony_regexp_named_captures) On 2017/03/29 17:26:49, adamk wrote: > This isn't needed. You can also remove the line above. es_staging already > implies harmony, which implies everything under "HARMONY_STAGED" below. Done.
The CQ bit was checked by jgruber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by jgruber@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org, adamk@chromium.org Link to the patchset: https://codereview.chromium.org/2779033003/#ps40001 (title: "Remove move_object_start implication as well now that it's shipped")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491212101041350,
"parent_rev": "81a976953d3823bf39bcedbdc8db5a5a771862ce", "commit_rev":
"3b716a08044da1073240cd287f35c8517fcf392f"}
Message was sent while issue was closed.
Description was changed from ========== [regexp] Stage named captures BUG=v8:5437 ========== to ========== [regexp] Stage named captures BUG=v8:5437 Review-Url: https://codereview.chromium.org/2779033003 Cr-Commit-Position: refs/heads/master@{#44331} Committed: https://chromium.googlesource.com/v8/v8/+/3b716a08044da1073240cd287f35c8517fc... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/v8/v8/+/3b716a08044da1073240cd287f35c8517fc... |
