Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(44)

Issue 1307943007: [es6] Fix computed property names in nested literals (Closed)

Created:
5 years, 2 months ago by adamk
Modified:
5 years, 2 months ago
CC:
rossberg, v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[es6] Fix computed property names in nested literals Make ObjectLiteral::is_simple() false for literals containing computed property names, which causes IsCompileTimeValue() to return false and thus force code to be generated for setting up such properties. This mirrors the handling of '__proto__' in literals. BUG=v8:4387 LOG=y Committed: https://crrev.com/233d62f8e306e56d5ed0597b008c205700e744bd Cr-Commit-Position: refs/heads/master@{#30362}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -0 lines) Patch
M src/ast.cc View 1 chunk +1 line, -0 lines 0 comments Download
M test/mjsunit/es6/computed-property-names.js View 1 chunk +56 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (7 generated)
adamk
5 years, 2 months ago (2015-08-24 19:44:33 UTC) #2
caitp (gmail)
On 2015/08/24 19:44:33, adamk wrote: Looks pretty good to me
5 years, 2 months ago (2015-08-24 19:53:29 UTC) #3
adamk
Does it look good enough to say the magic words? Rietveld is pretty picky :)
5 years, 2 months ago (2015-08-24 20:20:41 UTC) #4
caitp (gmail)
On 2015/08/24 20:20:41, adamk wrote: > Does it look good enough to say the magic ...
5 years, 2 months ago (2015-08-24 21:13:06 UTC) #5
adamk
Ah, thanks. I think I could actually land now (since I'm an owner), but I'll ...
5 years, 2 months ago (2015-08-24 21:47:41 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1307943007/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1307943007/1
5 years, 2 months ago (2015-08-25 18:36:41 UTC) #9
commit-bot: I haz the power
No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even if an ...
5 years, 2 months ago (2015-08-25 18:36:43 UTC) #11
adamk
+littledan
5 years, 2 months ago (2015-08-25 18:39:20 UTC) #13
adamk
and re-adding rossberg, the race is on!
5 years, 2 months ago (2015-08-25 20:09:49 UTC) #15
Dan Ehrenberg
lgtm Looks pretty straightforward!
5 years, 2 months ago (2015-08-25 20:27:17 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1307943007/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1307943007/1
5 years, 2 months ago (2015-08-25 20:41:28 UTC) #18
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 2 months ago (2015-08-25 21:10:40 UTC) #19
commit-bot: I haz the power
5 years, 2 months ago (2015-08-25 21:10:55 UTC) #20
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/233d62f8e306e56d5ed0597b008c205700e744bd
Cr-Commit-Position: refs/heads/master@{#30362}

Powered by Google App Engine
This is Rietveld 408576698