Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years, 9 months ago by Dan Ehrenberg Modified:
4 years, 9 months ago Reviewers:
Yang CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionPut RegExp js code in strict mode
src/js/regexp.js was one of the few files that was left in sloppy
mode. The ES2017 draft specification requires that writes to
lastIndex throw when the property is non-writable, and test262
tests enforce this behavior. This patch puts that file in strict
mode.
BUG=v8:4504
R=yangguo@chromium.org
LOG=Y
Committed: https://crrev.com/80b1b2a45bbd9bf3d08e4e6516acfaaa8f438213
Cr-Commit-Position: refs/heads/master@{#34801}
Patch Set 1 #Patch Set 2 : Better testing #
Total comments: 1
Created: 4 years, 9 months ago
Messages
Total messages: 25 (10 generated)
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1776883005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1776883005/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/...)
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1776883005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1776883005/20001
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 littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1776883005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1776883005/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/1...)
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1776883005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1776883005/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM. Might be a good idea to run LayoutTests upfront though. https://codereview.chromium.org/1776883005/diff/20001/test/mjsunit/regexp-las... File test/mjsunit/regexp-lastIndex.js (right): https://codereview.chromium.org/1776883005/diff/20001/test/mjsunit/regexp-las... test/mjsunit/regexp-lastIndex.js:7: // set unconditionally. If a set fails, then it acts as if in strict mode I wonder whether it makes sense to discuss these funny rules in TC39 and fix them. This is really surprising to me.
On 2016/03/15 at 06:56:36, yangguo wrote: > LGTM. Might be a good idea to run LayoutTests upfront though. > > https://codereview.chromium.org/1776883005/diff/20001/test/mjsunit/regexp-las... > File test/mjsunit/regexp-lastIndex.js (right): > > https://codereview.chromium.org/1776883005/diff/20001/test/mjsunit/regexp-las... > test/mjsunit/regexp-lastIndex.js:7: // set unconditionally. If a set fails, then it acts as if in strict mode > I wonder whether it makes sense to discuss these funny rules in TC39 and fix them. This is really surprising to me. I ran the LayoutTests and removed an invalid test as a result. Re: discussing the rules at TC39: Good idea. Submitted an issue: https://github.com/tc39/ecma262/issues/477
The CQ bit was checked by littledan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1776883005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1776883005/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Put RegExp js code in strict mode src/js/regexp.js was one of the few files that was left in sloppy mode. The ES2017 draft specification requires that writes to lastIndex throw when the property is non-writable, and test262 tests enforce this behavior. This patch puts that file in strict mode. BUG=v8:4504 R=yangguo@chromium.org LOG=Y ========== to ========== Put RegExp js code in strict mode src/js/regexp.js was one of the few files that was left in sloppy mode. The ES2017 draft specification requires that writes to lastIndex throw when the property is non-writable, and test262 tests enforce this behavior. This patch puts that file in strict mode. BUG=v8:4504 R=yangguo@chromium.org LOG=Y Committed: https://crrev.com/80b1b2a45bbd9bf3d08e4e6516acfaaa8f438213 Cr-Commit-Position: refs/heads/master@{#34801} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/80b1b2a45bbd9bf3d08e4e6516acfaaa8f438213 Cr-Commit-Position: refs/heads/master@{#34801}
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2106803004/ by littledan@chromium.org. The reason for reverting is: Found to break SAP Web IDE, and these semantics are not shipped in any other browser. Revert to legacy semantics while assessing web compatibility. BUG=chromium:624318. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
