Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(135)

Issue 580123002: Revert "RegExp: Add support for the ES6-proposed sticky flag" (Closed)

Created:
6 years, 3 months ago by rossberg
Modified:
6 years, 3 months ago
Reviewers:
Erik Corry, Yang
CC:
v8-dev, Erik Corry Chromium.org
Project:
v8
Visibility:
Public.

Description

Revert "RegExp: Add support for the ES6-proposed sticky flag" Causes a flaky failure on buildbots. Here is the (deterministic) repro step (thanks to Michael Stanton): first go to flag-definitions.h and set this to false. DEFINE_BOOL(enable_sse4_1, false, "enable use of SSE4.1 instructions if available") Run the following and it should fail: tools/run-tests.py --arch=ia32 --mode=release cctest/test-api/Regress2107 R=yangguo@chromium.org BUG= Committed: https://code.google.com/p/v8/source/detail?r=24045

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -276 lines) Patch
M src/bootstrapper.cc View 3 chunks +0 lines, -16 lines 0 comments Download
M src/compilation-cache.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/flag-definitions.h View 2 chunks +0 lines, -2 lines 0 comments Download
M src/heap/heap.h View 1 chunk +0 lines, -2 lines 0 comments Download
M src/jsregexp.h View 1 chunk +1 line, -1 line 0 comments Download
M src/jsregexp.cc View 5 chunks +5 lines, -11 lines 0 comments Download
M src/objects.h View 2 chunks +1 line, -8 lines 0 comments Download
M src/regexp.js View 8 chunks +10 lines, -21 lines 0 comments Download
M src/runtime.h View 1 chunk +1 line, -1 line 0 comments Download
M src/runtime.cc View 4 chunks +3 lines, -15 lines 0 comments Download
M test/cctest/test-regexp.cc View 1 chunk +1 line, -1 line 0 comments Download
D test/mjsunit/harmony/regexp-sticky.js View 1 chunk +0 lines, -132 lines 0 comments Download
D test/mjsunit/regexp-not-sticky-yet.js View 1 chunk +0 lines, -65 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
rossberg
6 years, 3 months ago (2014-09-18 14:45:14 UTC) #1
Yang
On 2014/09/18 14:45:14, rossberg wrote: lgtm.
6 years, 3 months ago (2014-09-18 14:45:29 UTC) #2
rossberg
Committed patchset #1 manually as 24045 (tree was closed).
6 years, 3 months ago (2014-09-18 14:53:16 UTC) #3
Sven Panne
On 2014/09/18 14:53:16, rossberg wrote: > Committed patchset #1 manually as 24045 (tree was closed). ...
6 years, 3 months ago (2014-09-19 06:35:48 UTC) #4
Erik Corry
6 years, 3 months ago (2014-09-19 06:52:21 UTC) #6
Message was sent while issue was closed.
My bet is on https://codereview.chromium.org/576093002/ since it changes the
idle notification heuristics.

The test is congenitally flaky, since it makes calls to a heuristic guidance
method, idleNotification, and expects predictable results.  Change the
heuristics and it breaks, but that doesn't mean it wasn't broken to start with.

Powered by Google App Engine
This is Rietveld 408576698