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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 613673002: Support count operations on super named properties. (Closed)

Created:
6 years, 2 months ago by Dmitry Lomov (no reviews)
Modified:
6 years, 2 months ago
CC:
v8-dev
Project:
v8
Visibility:
Public.

Description

Support count operations on super named properties. R=ishell@chromium.org BUG=v8:3330 LOG=N Committed: https://code.google.com/p/v8/source/detail?r=24290

Patch Set 1 #

Patch Set 2 : Platform ports #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+198 lines, -47 lines) Patch
M src/arm/full-codegen-arm.cc View 1 8 chunks +38 lines, -11 lines 0 comments Download
M src/arm64/full-codegen-arm64.cc View 1 8 chunks +37 lines, -11 lines 0 comments Download
M src/full-codegen.h View 1 chunk +1 line, -1 line 0 comments Download
M src/ia32/full-codegen-ia32.cc View 8 chunks +36 lines, -11 lines 0 comments Download
M src/x64/full-codegen-x64.cc View 1 8 chunks +36 lines, -11 lines 0 comments Download
M test/mjsunit/harmony/super.js View 1 chunk +50 lines, -2 lines 2 comments Download

Messages

Total messages: 9 (3 generated)
Dmitry Lomov (no reviews)
PTAL, now supporting super.x++ and friends.
6 years, 2 months ago (2014-09-29 13:10:41 UTC) #4
Igor Sheludko
lgtm
6 years, 2 months ago (2014-09-29 13:51:20 UTC) #5
Dmitry Lomov (no reviews)
Committed patchset #2 (id:20001) manually as 24290 (presubmit successful).
6 years, 2 months ago (2014-09-29 13:56:44 UTC) #6
arv (Not doing code reviews)
LGTM https://codereview.chromium.org/613673002/diff/20001/test/mjsunit/harmony/super.js File test/mjsunit/harmony/super.js (right): https://codereview.chromium.org/613673002/diff/20001/test/mjsunit/harmony/super.js#newcode252 test/mjsunit/harmony/super.js:252: super.x++; Maybe wrap these in assertEquals to also ...
6 years, 2 months ago (2014-09-29 14:50:29 UTC) #7
Dmitry Lomov (no reviews)
https://codereview.chromium.org/613673002/diff/20001/test/mjsunit/harmony/super.js File test/mjsunit/harmony/super.js (right): https://codereview.chromium.org/613673002/diff/20001/test/mjsunit/harmony/super.js#newcode252 test/mjsunit/harmony/super.js:252: super.x++; On 2014/09/29 14:50:29, arv wrote: > Maybe wrap ...
6 years, 2 months ago (2014-09-29 14:54:46 UTC) #8
arv (Not doing code reviews)
6 years, 2 months ago (2014-09-29 15:00:12 UTC) #9
Message was sent while issue was closed. 
On 2014/09/29 14:54:46, Dmitry Lomov (chromium) wrote:
>
https://codereview.chromium.org/613673002/diff/20001/test/mjsunit/harmony/sup...
> File test/mjsunit/harmony/super.js (right):
> 
>
https://codereview.chromium.org/613673002/diff/20001/test/mjsunit/harmony/sup...
> test/mjsunit/harmony/super.js:252: super.x++;
> On 2014/09/29 14:50:29, arv wrote:
> > Maybe wrap these in assertEquals to also assert the return value?
> 
> No, these count operations without use of return values is a special case in
> codegen, and I am testing precisely that.

Oh. That is not possible to grok from looking at the test. Maybe add a comment?

Powered by Google App Engine
This is Rietveld 408576698