Chromium Code Reviews
Help | Chromium Project | Sign in
(1026)

Issue 11358214: Remove 'type' and 'arguments' properties from Error object. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by Yang
Modified:
1 year, 5 months ago
Reviewers:
Sven Panne
CC:
v8-dev_googlegroups.com
Visibility:
Public.

Description

Remove 'type' and 'arguments' properties from Error object.

R=svenpanne@chromium.org
BUG=v8:2397
Committed: https://code.google.com/p/v8/source/detail?r=12956

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -54 lines) Lint Patch
M src/messages.js View 3 chunks +0 lines, -5 lines 1 comment ? errors Download
M test/cctest/test-parsing.cc View 4 chunks +35 lines, -10 lines 1 comment ? errors Download
M test/mjsunit/array-reduce.js View 6 chunks +8 lines, -8 lines 0 comments ? errors Download
M test/mjsunit/error-constructors.js View 3 chunks +1 line, -14 lines 0 comments ? errors Download
M test/mjsunit/function-call.js View 9 chunks +13 lines, -16 lines 0 comments ? errors Download
M test/mjsunit/regress/regress-1980.js View 1 chunk +1 line, -1 line 0 comments ? errors Download
Trybot results:
Commit:

Messages

Total messages: 2
Yang
PTAL. Most of the change are tests that relied on Error.type.
1 year, 5 months ago #1
Sven Panne
1 year, 5 months ago #2
LGTM with nits

http://codereview.chromium.org/11358214/diff/1/src/messages.js
File src/messages.js (right):

http://codereview.chromium.org/11358214/diff/1/src/messages.js#newcode258
src/messages.js:258: var e = new constructor(FormatMessage(type, args));
Just drop the "var e =", it's cleaner... ;-)

http://codereview.chromium.org/11358214/diff/1/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):

http://codereview.chromium.org/11358214/diff/1/test/cctest/test-parsing.cc#ne...
test/cctest/test-parsing.cc:1095: i::Handle<i::String> message_string;
Remove this line, merging it with the initialization below. In general, we
should avoid declaring any uninitialized variable. (Yes, I know, this one is
initialized via it's default constructor, but in a way it is still
uninitialized.)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434