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

Issue 12255033: Fix f.apply() optimization when declared arguments are mutated. (Closed)

Created:
7 years, 10 months ago by Michael Starzinger
Modified:
7 years, 10 months ago
Reviewers:
Toon Verwaest
CC:
v8-dev
Visibility:
Public.

Description

Fix f.apply() optimization when declared arguments are mutated. R=verwaest@chromium.org BUG=v8:2539 TEST=mjsunit/regress/regress-2539 Committed: http://code.google.com/p/v8/source/detail?r=13673

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed comments by Toon Verwaest. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -26 lines) Patch
M src/hydrogen.cc View 1 2 chunks +14 lines, -11 lines 0 comments Download
M test/mjsunit/compiler/inline-function-apply.js View 1 chunk +1 line, -2 lines 0 comments Download
A + test/mjsunit/regress/regress-2539.js View 1 chunk +18 lines, -13 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Michael Starzinger
7 years, 10 months ago (2013-02-14 14:37:52 UTC) #1
Toon Verwaest
lgtm with nit. https://codereview.chromium.org/12255033/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): https://codereview.chromium.org/12255033/diff/1/src/hydrogen.cc#newcode8011 src/hydrogen.cc:8011: int parameter_count = arguments_values->length(); Please rename ...
7 years, 10 months ago (2013-02-14 14:58:20 UTC) #2
Michael Starzinger
7 years, 10 months ago (2013-02-14 15:12:01 UTC) #3
Addressed comments. Landing.

https://codereview.chromium.org/12255033/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/12255033/diff/1/src/hydrogen.cc#newcode8011
src/hydrogen.cc:8011: int parameter_count = arguments_values->length();
On 2013/02/14 14:58:20, Toon Verwaest wrote:
> Please rename this to arguments_count, given that it's the number of
> argument_values.

Done.

Powered by Google App Engine
This is Rietveld 408576698