inserted the full snapshot. previous one was not complete

This behaviour is correct per the spec.

But this code worked in all previous versions.

I can't understand how can you rely just on spec making all existing code wrong in a moment.

http://crrev.com/80b1b2a45bbd9bf3d08e4e6516acfaaa8f438213

Cc littledan@chromium.org

Thanks commenter #5.

This was actually a change for ES5 spec compliance, not even ES2015. The specification says that if you *fail* to find a match, then do a write to lastIndex, and further, if that write fails, then throw. We only recently shipped the second part, but both parts have been required for years.

The relevant question that I'm wondering about is, does this break many websites? If it does, then let's think about making some sort of fixup (to V8, but ideally to the specification also) so that websites don't break. However, apart from web compatibility, it's hard for me to understand the motivation for using frozen RegExps. I'd recommend not freezing it.

We are using SAp Web IDE. I've got my work blocked due to this error after browser upgrade. After debugging the source code I found this check. I'm not sure why they use freeze, I guess it's somehow connected with security reasons. They probably want regex not to be overwritten probably as it's stored in the global variable.

I tested Edge, Firefox and Safari Tech Preview, and none of them seemed to throw for this simple test case that Chrome 51 throws for, which it sounds like you're talking about:

x = /a/
Object.freeze(x)
"b".match(x)

I agree with the culprit patch https://chromium.googlesource.com/v8/v8/+/80b1b2a45bbd9bf3d08e4e6516acfaaa8f438213%5E%21 which made this change purposely, because the spec called for it.

For web compatibility, maybe we should revert the change for now, backporting to 52 to minimize the period of breakage, and collect statistics on how often this error would be thrown. If it turns out to be necessary, we could standardize suppressing this error. It is counterintuitive that the failure path writes to lastIndex even for non-global RegExps, but I worry about the web compatibility risks of changing that as well.

Cross-posted to https://github.com/tc39/ecma262/issues/625 for spec discussion

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/34880eb3dcf7492d44c0a3b45b6c888189f2c3c3

commit 34880eb3dcf7492d44c0a3b45b6c888189f2c3c3
Author: littledan <littledan@chromium.org>
Date: Thu Jun 30 14:36:12 2016

Revert of Put RegExp js code in strict mode (patchset #2 id:20001 of https://codereview.chromium.org/1776883005/ )

Reason for revert:
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 

Original issue's description:
> 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}

TBR=yangguo@chromium.org,adamk@chromium.org

Review-Url: https://codereview.chromium.org/2112713003
Cr-Commit-Position: refs/heads/master@{#37449}

[modify] https://crrev.com/34880eb3dcf7492d44c0a3b45b6c888189f2c3c3/src/js/regexp.js
[delete] https://crrev.com/de369129d272c9cbd2a6282f4097a5d0ce264e85/test/mjsunit/regexp-lastIndex.js
[modify] https://crrev.com/34880eb3dcf7492d44c0a3b45b6c888189f2c3c3/test/test262/test262.status
[modify] https://crrev.com/34880eb3dcf7492d44c0a3b45b6c888189f2c3c3/test/webkit/fast/regex/lastIndex-expected.txt

Reverting the blamed patch should address this issue. I'd like to backport the merge to 52 to minimize the period of breakage, but I am not sure if it is worth backporting to 51 at this point.

Looks like this didn't make the 53 branch; should we merge it there?

Your change meets the bar and is auto-approved for M52 (branch: 2743)

Your change meets the bar and is auto-approved for M53 (branch: 2785)

Tested this on Win7/64 bit - Version 53.0.2785.0 canary (64-bit)

I still see the same error:
Uncaught TypeError: Cannot assign to read only property 'lastIndex' of object '[object RegExp]'(…)

Please find the attached screen shots.

littledan@, can you please take a look ?

	M53.PNG 95.9 KB View Download

	M49.PNG 143 KB View Download

	Firefox 47.0.PNG 21.4 KB View Download

pucchakayala, I am working on backporting the patch to M53. I hope to have it updated within a week. Sorry about the delay.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/476af19c7e85aed3d4ec9d4bd8713eb5c881250f

commit 476af19c7e85aed3d4ec9d4bd8713eb5c881250f
Author: Dan Ehrenberg <littledan@chromium.org>
Date: Fri Jul 01 21:16:45 2016

Version 5.3.332.6 (cherry-pick)

Merged 34880eb3dcf7492d44c0a3b45b6c888189f2c3c3

Revert of Put RegExp js code in strict mode (patchset #2 id:20001 of https://codereview.chromium.org/1776883005/ )

BUG= chromium:624318 
LOG=N
R=adamk@chromium.org

Review URL: https://codereview.chromium.org/2119943002 .

Cr-Commit-Position: refs/branch-heads/5.3@{#8}
Cr-Branched-From: 820a23aade5e74a92d794e05a0c2b3597f0da4b5-refs/heads/5.3.332@{#2}
Cr-Branched-From: 37538cb2c1b4d75c41af386cb4fedbe5566f5608-refs/heads/master@{#37308}

[modify] https://crrev.com/476af19c7e85aed3d4ec9d4bd8713eb5c881250f/include/v8-version.h
[modify] https://crrev.com/476af19c7e85aed3d4ec9d4bd8713eb5c881250f/src/js/regexp.js
[delete] https://crrev.com/b02bd634a5c551888033dc80925d82b33fb9da59/test/mjsunit/regexp-lastIndex.js
[modify] https://crrev.com/476af19c7e85aed3d4ec9d4bd8713eb5c881250f/test/test262/test262.status
[modify] https://crrev.com/476af19c7e85aed3d4ec9d4bd8713eb5c881250f/test/webkit/fast/regex/lastIndex-expected.txt

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/b63de17426eb45352d03f263f5bb2271de003a44

commit b63de17426eb45352d03f263f5bb2271de003a44
Author: Dan Ehrenberg <littledan@chromium.org>
Date: Fri Jul 01 21:31:42 2016

Version 5.2.361.34 (cherry-pick)

Merged 34880eb3dcf7492d44c0a3b45b6c888189f2c3c3

Revert of Put RegExp js code in strict mode (patchset #2 id:20001 of https://codereview.chromium.org/1776883005/ )

BUG= chromium:624318 
LOG=N
R=adamk@chromium.org

Review URL: https://codereview.chromium.org/2118973002 .

Cr-Commit-Position: refs/branch-heads/5.2@{#40}
Cr-Branched-From: 2cd36d6d0439ddfbe84cd90e112dced85084ec95-refs/heads/5.2.361@{#1}
Cr-Branched-From: 3fef34e02388e07d46067c516320f1ff12304c8e-refs/heads/master@{#36332}

[modify] https://crrev.com/b63de17426eb45352d03f263f5bb2271de003a44/include/v8-version.h
[modify] https://crrev.com/b63de17426eb45352d03f263f5bb2271de003a44/src/js/regexp.js
[delete] https://crrev.com/0c09c4eb9709d19b7b23b26f9589a6ed5180da0c/test/mjsunit/regexp-lastIndex.js
[modify] https://crrev.com/b63de17426eb45352d03f263f5bb2271de003a44/test/test262/test262.status
[modify] https://crrev.com/b63de17426eb45352d03f263f5bb2271de003a44/test/webkit/fast/regex/lastIndex-expected.txt

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e5c6b86202dd919622439a28abd0ee86ffa30213

commit e5c6b86202dd919622439a28abd0ee86ffa30213
Author: littledan <littledan@chromium.org>
Date: Thu Jul 07 06:19:07 2016

Version 5.2.361.36 (cherry-pick)

Remove duplicate key from status file

R=machenbach@chromium.org
BUG= chromium:624318 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2124943003
Cr-Commit-Position: refs/branch-heads/5.2@{#42}
Cr-Branched-From: 2cd36d6d0439ddfbe84cd90e112dced85084ec95-refs/heads/5.2.361@{#1}
Cr-Branched-From: 3fef34e02388e07d46067c516320f1ff12304c8e-refs/heads/master@{#36332}

[modify] https://crrev.com/e5c6b86202dd919622439a28abd0ee86ffa30213/include/v8-version.h
[modify] https://crrev.com/e5c6b86202dd919622439a28abd0ee86ffa30213/test/test262/test262.status

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6b7430fab1cac0b8b9d6a45b5fdd1ca252921ed7

commit 6b7430fab1cac0b8b9d6a45b5fdd1ca252921ed7
Author: littledan <littledan@chromium.org>
Date: Thu Sep 15 19:20:17 2016

Reland of Put RegExp js code in strict mode (patchset #2 id:20001 of https://codereview.chromium.or… (patchset #2 id:20001 of https://codereview.chromium.org/2112713003/ )

Reason for revert:
With fixes for frozen RegExps in https://codereview.chromium.org/2339443002 , it should be web-compatible to put RegExps in strict mode again, per spec.

Original issue's description:
> Revert of Put RegExp js code in strict mode (patchset #2 id:20001 of https://codereview.chromium.org/1776883005/ )
>
> Reason for revert:
> 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 
>
> Original issue's description:
> > 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}
>
> TBR=yangguo@chromium.org,adamk@chromium.org
>
> Committed: https://crrev.com/34880eb3dcf7492d44c0a3b45b6c888189f2c3c3
> Cr-Commit-Position: refs/heads/master@{#37449}

TBR=adamk@chromium.org,yangguo@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= chromium:624318 

Review-Url: https://codereview.chromium.org/2344773002
Cr-Commit-Position: refs/heads/master@{#39456}

[modify] https://crrev.com/6b7430fab1cac0b8b9d6a45b5fdd1ca252921ed7/src/js/regexp.js
[add] https://crrev.com/6b7430fab1cac0b8b9d6a45b5fdd1ca252921ed7/test/mjsunit/regexp-lastIndex.js
[modify] https://crrev.com/6b7430fab1cac0b8b9d6a45b5fdd1ca252921ed7/test/test262/test262.status
[modify] https://crrev.com/6b7430fab1cac0b8b9d6a45b5fdd1ca252921ed7/test/webkit/fast/regex/lastIndex-expected.txt

Merged onto Node.js v6.x (V8 5.1) via https://github.com/nodejs/node/pull/8673.