Issue 10443085: Report syntax errors in natives when building with mksnapshot.

Issue 10443085: Report syntax errors in natives when building with mksnapshot. (Closed)

Created:
8 years, 6 months ago by Erik Corry

Modified:
8 years, 6 months ago

Reviewers:
ulan

CC:
v8-dev

Base URL:
http://v8.googlecode.com/svn/branches/bleeding_edge/

Visibility:
Public.

More Reviews

Description

Report syntax errors in natives when building with mksnapshot. Committed: http://code.google.com/p/v8/source/detail?r=11687

Patch Set 1 #

Total comments: 6

Created: 8 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+84 lines, -4 lines)			Patch
M	include/v8.h	View	2 chunks	+6 lines, -0 lines	0 comments	Download
M	src/api.cc	View	1 chunk	+15 lines, -0 lines	2 comments	Download
M	src/factory.h	View	1 chunk	+1 line, -0 lines	2 comments	Download
M	src/factory.cc	View	3 chunks	+41 lines, -2 lines	0 comments	Download
M	src/isolate.cc	View	1 chunk	+12 lines, -2 lines	0 comments	Download
M	src/mksnapshot.cc	View	1 chunk	+6 lines, -0 lines	0 comments	Download
M	src/parser.h	View	1 chunk	+3 lines, -0 lines	2 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

ulan

http://codereview.chromium.org/10443085/diff/1/src/api.cc File src/api.cc (right): http://codereview.chromium.org/10443085/diff/1/src/api.cc#newcode1674 src/api.cc:1674: exception_(i::Smi::FromInt(0)), Why is exception_ = 0 here, instead of ...

8 years, 6 months ago (2012-05-31 08:48:52 UTC) #2

Erik Corry

8 years, 6 months ago (2012-05-31 12:25:16 UTC) #3

http://codereview.chromium.org/10443085/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/10443085/diff/1/src/api.cc#newcode1674
src/api.cc:1674: exception_(i::Smi::FromInt(0)),
On 2012/05/31 08:48:52, ulan wrote:
> Why is exception_ = 0 here, instead of the hole?
> try_catch.HasCaught() seems to be always true in mksnapshot.cc since it checks
> for the hole.

Well spotted.

It turns out that I can just test the return value for IsEmpty and get rid of
the ugly special TryCatch constructor.

http://codereview.chromium.org/10443085/diff/1/src/factory.h
File src/factory.h (right):

http://codereview.chromium.org/10443085/diff/1/src/factory.h#newcode341
src/factory.h:341: Handle<String> EmergencyNewError(const char* type,
Handle<JSArray> args);
On 2012/05/31 08:48:52, ulan wrote:
> Consider NewEmergencyError().

The error is normal, it's just happening at a bad moment, so I prefer the
current name.

http://codereview.chromium.org/10443085/diff/1/src/parser.h
File src/parser.h (right):

http://codereview.chromium.org/10443085/diff/1/src/parser.h#newcode452
src/parser.h:452: void EmergencyReportMessageAt(MessageLocation* location,
On 2012/05/31 08:48:52, ulan wrote:
> Dead code.

Done.

Expand Messages | Collapse Messages