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

Issue 347503003: Move invalid string length check to the factory. (Closed)

Created:
6 years, 6 months ago by Yang
Modified:
6 years, 6 months ago
Reviewers:
Igor Sheludko
CC:
v8-dev
Visibility:
Public.

Description

Move invalid string length check to the factory. R=ishell@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=21886

Patch Set 1 #

Total comments: 6

Patch Set 2 : addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -119 lines) Patch
M src/factory.h View 1 chunk +17 lines, -0 lines 0 comments Download
M src/factory.cc View 1 6 chunks +51 lines, -16 lines 0 comments Download
M src/heap.h View 1 chunk +0 lines, -23 lines 0 comments Download
M src/heap.cc View 4 chunks +6 lines, -67 lines 0 comments Download
M src/heap-inl.h View 1 4 chunks +8 lines, -13 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Yang
6 years, 6 months ago (2014-06-18 12:04:50 UTC) #1
Igor Sheludko
lgtm with nits: https://codereview.chromium.org/347503003/diff/1/src/factory.cc File src/factory.cc (right): https://codereview.chromium.org/347503003/diff/1/src/factory.cc#newcode277 src/factory.cc:277: NewRawOneByteString(string.length(), pretenure), string.length() -> length https://codereview.chromium.org/347503003/diff/1/src/factory.cc#newcode286 ...
6 years, 6 months ago (2014-06-18 13:10:13 UTC) #2
Yang
Committed patchset #2 manually as r21886 (tree was closed).
6 years, 6 months ago (2014-06-18 13:26:17 UTC) #3
Yang
6 years, 6 months ago (2014-06-18 13:27:26 UTC) #4
Message was sent while issue was closed.
https://codereview.chromium.org/347503003/diff/1/src/factory.cc
File src/factory.cc (right):

https://codereview.chromium.org/347503003/diff/1/src/factory.cc#newcode277
src/factory.cc:277: NewRawOneByteString(string.length(), pretenure),
On 2014/06/18 13:10:13, Igor Sheludko wrote:
> string.length() -> length

Done.

https://codereview.chromium.org/347503003/diff/1/src/factory.cc#newcode286
src/factory.cc:286: NewRawTwoByteString(string.length(), pretenure),
On 2014/06/18 13:10:13, Igor Sheludko wrote:
> string.length() -> length

Done.

https://codereview.chromium.org/347503003/diff/1/src/heap-inl.h
File src/heap-inl.h (right):

https://codereview.chromium.org/347503003/diff/1/src/heap-inl.h#newcode541
src/heap-inl.h:541: __object__ = __allocation__.ToObjectChecked();              
              \
On 2014/06/18 13:10:13, Igor Sheludko wrote:
> While we are here: what about 
>   if (__allocation__.To(&__object__)) {
> instead of two lines above?

Done.

Powered by Google App Engine
This is Rietveld 408576698