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

Issue 2206573002: Move NoSideEffectToString to C++ (Closed)

Created:
4 years, 4 months ago by jgruber
Modified:
4 years, 4 months ago
Reviewers:
Yang
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Move NoSideEffectToString to C++ BUG= Committed: https://crrev.com/d48170dbf57bb70b5be89ed13873434b94f52b49 Cr-Commit-Position: refs/heads/master@{#38289}

Patch Set 1 #

Patch Set 2 : Tweaks #

Patch Set 3 : Tweaks #

Patch Set 4 : Remove IsErrorObject DCHECK #

Total comments: 4

Patch Set 5 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+346 lines, -237 lines) Patch
M src/api.cc View 1 chunk +2 lines, -7 lines 0 comments Download
M src/bootstrapper.cc View 1 2 chunks +19 lines, -5 lines 0 comments Download
M src/builtins/builtins.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/builtins/builtins-error.cc View 2 chunks +35 lines, -4 lines 0 comments Download
M src/contexts.h View 2 chunks +1 line, -3 lines 0 comments Download
M src/factory.cc View 1 chunk +8 lines, -14 lines 0 comments Download
M src/heap-symbols.h View 1 chunk +0 lines, -1 line 0 comments Download
M src/isolate.cc View 1 chunk +2 lines, -1 line 0 comments Download
M src/js/harmony-simd.js View 1 2 3 1 chunk +0 lines, -13 lines 0 comments Download
M src/js/messages.js View 3 chunks +1 line, -139 lines 0 comments Download
M src/messages.h View 1 2 1 chunk +7 lines, -1 line 0 comments Download
M src/messages.cc View 5 chunks +47 lines, -28 lines 0 comments Download
M src/objects.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/objects.cc View 1 2 3 4 1 chunk +148 lines, -0 lines 0 comments Download
M src/runtime/runtime.h View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M src/runtime/runtime-internal.cc View 1 2 3 1 chunk +0 lines, -13 lines 0 comments Download
M test/cctest/cctest.gyp View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
A test/cctest/test-object.cc View 1 2 3 4 1 chunk +71 lines, -0 lines 0 comments Download
D test/mjsunit/regress/regress-crbug-492526.js View 1 2 3 1 chunk +0 lines, -7 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 28 (21 generated)
jgruber
4 years, 4 months ago (2016-08-03 07:36:31 UTC) #16
Yang
looking good https://codereview.chromium.org/2206573002/diff/60001/src/objects.cc File src/objects.cc (right): https://codereview.chromium.org/2206573002/diff/60001/src/objects.cc#newcode278 src/objects.cc:278: return isolate->factory()->null_string(); Can we defer to Object::ToString ...
4 years, 4 months ago (2016-08-03 09:13:18 UTC) #17
jgruber
PTAL https://codereview.chromium.org/2206573002/diff/60001/src/objects.cc File src/objects.cc (right): https://codereview.chromium.org/2206573002/diff/60001/src/objects.cc#newcode278 src/objects.cc:278: return isolate->factory()->null_string(); On 2016/08/03 09:13:18, Yang wrote: > ...
4 years, 4 months ago (2016-08-03 11:17:15 UTC) #22
Yang
lgtm.
4 years, 4 months ago (2016-08-03 12:13:12 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2206573002/80001
4 years, 4 months ago (2016-08-03 12:16:25 UTC) #25
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 4 months ago (2016-08-03 12:20:44 UTC) #26
commit-bot: I haz the power
4 years, 4 months ago (2016-08-03 12:22:29 UTC) #28
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/d48170dbf57bb70b5be89ed13873434b94f52b49
Cr-Commit-Position: refs/heads/master@{#38289}

Powered by Google App Engine
This is Rietveld 408576698