[regexp] Set static property attributes as in spec proposal

[regexp] Set static property attributes as in spec proposal

'[...] accessor properties who have the attributes { [[Enumerable]]:
false, [[Configurable]]: true } [...]'

BUG=v8:5566
Committed: https://crrev.com/88c5a300c5d291a76cf62c216aeeddf022efea45
Cr-Commit-Position: refs/heads/master@{#40609}

[jgruber, 2016-10-26 07:36:35]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 07:36:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jgruber, 2016-10-26 07:37:15]

jgruber@chromium.org changed reviewers:
+ littledan@chromium.org, yangguo@chromium.org

[jgruber, 2016-10-26 07:37:16]

Let's see whether layout tests need to be updated.

[commit-bot: I haz the power, 2016-10-26 08:07:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 08:07:02]

Dry run: This issue passed the CQ dry run.

[Dan Ehrenberg, 2016-10-26 08:36:42]

lgtm

https://codereview.chromium.org/2452913002/diff/1/test/mjsunit/regress/regres...
File test/mjsunit/regress/regress-5566.js (right):

https://codereview.chromium.org/2452913002/diff/1/test/mjsunit/regress/regres...
test/mjsunit/regress/regress-5566.js:22:
assertFalse(Object.getOwnPropertyDescriptor(RegExp, prop).enumerable, prop);
While you're looking at the property descriptors, it'd be nice to check that
these are getter/setters.

[jgruber, 2016-10-26 09:29:36]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 09:29:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jgruber, 2016-10-26 09:30:43]

https://codereview.chromium.org/2452913002/diff/1/test/mjsunit/regress/regres...
File test/mjsunit/regress/regress-5566.js (right):

https://codereview.chromium.org/2452913002/diff/1/test/mjsunit/regress/regres...
test/mjsunit/regress/regress-5566.js:22:
assertFalse(Object.getOwnPropertyDescriptor(RegExp, prop).enumerable, prop);
On 2016/10/26 08:36:41, Dan Ehrenberg wrote:
> While you're looking at the property descriptors, it'd be nice to check that
> these are getter/setters.

Done, and also changed setters to be undefined instead of the EmptyFunction
builtin.

[jgruber, 2016-10-26 09:34:07]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 09:34:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 09:49:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 09:49:48]

Dry run: Try jobs failed on following builders:
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/11501)
  v8_mac_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng_triggered/bui...)

[Dan Ehrenberg, 2016-10-26 09:59:58]

Could you back out that setter change from this patch? I hadn't noticed it in
the legacy properties spec--that seems like a big deal! It would make setting
the legacy properties throw in strict mode--obscure, to be sure, but making code
that doesn't throw throw could really break websites. I don't know if it's
web-compatible.

The best way to go forward would be to replace kEmptyFunction with a builtin
which increments a UseCounter so we can learn how frequently this setter path is
hit. If it is hit "too frequently" (a subjective measurement), then we should
petition to change the draft spec to use empty functions as setters as well, to
suppress the exception.

[jgruber, 2016-10-26 10:34:42]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 10:34:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jgruber, 2016-10-26 10:37:11]

On 2016/10/26 09:59:58, Dan Ehrenberg wrote:
> Could you back out that setter change from this patch? I hadn't noticed it in
> the legacy properties spec--that seems like a big deal! It would make setting
> the legacy properties throw in strict mode--obscure, to be sure, but making
code
> that doesn't throw throw could really break websites. I don't know if it's
> web-compatible.
> 
> The best way to go forward would be to replace kEmptyFunction with a builtin
> which increments a UseCounter so we can learn how frequently this setter path
is
> hit. If it is hit "too frequently" (a subjective measurement), then we should
> petition to change the draft spec to use empty functions as setters as well,
to
> suppress the exception.

Ok, reverted back to kEmptyFunction for now and we can do the UseCounters in a 
follow-up (not doing them here since it requires touching chromium as well).

[commit-bot: I haz the power, 2016-10-26 10:59:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 10:59:35]

Dry run: This issue passed the CQ dry run.

[Dan Ehrenberg, 2016-10-26 11:14:27]

lgtm

https://codereview.chromium.org/2452913002/diff/60001/test/mjsunit/regress/re...
File test/mjsunit/regress/regress-5566.js (right):

https://codereview.chromium.org/2452913002/diff/60001/test/mjsunit/regress/re...
test/mjsunit/regress/regress-5566.js:20: assertTrue(desc.get !== undefined,
prop);
Since we left it this way on purpose, how about desc.set !== undefined as well?

[jgruber, 2016-10-27 07:48:15]

https://codereview.chromium.org/2452913002/diff/60001/test/mjsunit/regress/re...
File test/mjsunit/regress/regress-5566.js (right):

https://codereview.chromium.org/2452913002/diff/60001/test/mjsunit/regress/re...
test/mjsunit/regress/regress-5566.js:20: assertTrue(desc.get !== undefined,
prop);
On 2016/10/26 11:14:27, Dan Ehrenberg wrote:
> Since we left it this way on purpose, how about desc.set !== undefined as
well?

Done.

[jgruber, 2016-10-27 07:48:19]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-10-27 07:48:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org
Link to the patchset: https://codereview.chromium.org/2452913002/#ps80001
(title: "Check setters as well")

[commit-bot: I haz the power, 2016-10-27 07:48:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 08:25:31]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-17 22:14:57]

Description was changed from

==========
[regexp] Set static property attributes as in spec proposal

'[...] accessor properties who have the attributes { [[Enumerable]]:
false, [[Configurable]]: true } [...]'

BUG=v8:5566
==========

to

==========
[regexp] Set static property attributes as in spec proposal

'[...] accessor properties who have the attributes { [[Enumerable]]:
false, [[Configurable]]: true } [...]'

BUG=v8:5566

Committed: https://crrev.com/88c5a300c5d291a76cf62c216aeeddf022efea45
Cr-Commit-Position: refs/heads/master@{#40609}
==========

[commit-bot: I haz the power, 2016-11-17 22:14:58]

Patchset 5 (id:??) landed as
https://crrev.com/88c5a300c5d291a76cf62c216aeeddf022efea45
Cr-Commit-Position: refs/heads/master@{#40609}