Issue 7206015: Fix issue 1354: Bad function name inference.

Issue 7206015: Fix issue 1354: Bad function name inference. (Closed)

Created:
9 years, 6 months ago by mnaganov (inactive)

Modified:
9 years, 6 months ago

Reviewers:
Kevin Millikin (Chromium), Vitaly Repeshko

CC:
v8-dev

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

Visibility:
Public.

Description

Fix issue 1354: Bad function name inference. R=kmillikin@chromium.org, vitalyr@chromium.org BUG=1354 TEST=test-func-name-inference Committed: http://code.google.com/p/v8/source/detail?r=8383

Patch Set 1 #

Total comments: 6

Created: 9 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+75 lines, -33 lines)			Patch
M	src/func-name-inferrer.h	View	3 chunks	+18 lines, -7 lines	4 comments	Download
M	src/func-name-inferrer.cc	View	2 chunks	+30 lines, -12 lines	2 comments	Download
M	src/parser.cc	View	3 chunks	+13 lines, -2 lines	0 comments	Download
M	src/profile-generator.cc	View	2 chunks	+2 lines, -1 line	0 comments	Download
M	test/cctest/cctest.status	View	1 chunk	+0 lines, -8 lines	0 comments	Download
M	test/cctest/test-func-name-inference.cc	View	1 chunk	+12 lines, -3 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Vitaly Repeshko

LGTM with comments. http://codereview.chromium.org/7206015/diff/1/src/func-name-inferrer.cc File src/func-name-inferrer.cc (right): http://codereview.chromium.org/7206015/diff/1/src/func-name-inferrer.cc#newcode55 src/func-name-inferrer.cc:55: Isolate::Current()->runtime_state(), name->Get(0))) { While we're here, ...

9 years, 6 months ago (2011-06-22 13:21:30 UTC) #3

mnaganov (inactive)

9 years, 6 months ago (2011-06-22 20:21:08 UTC) #4

Thanks!

http://codereview.chromium.org/7206015/diff/1/src/func-name-inferrer.cc
File src/func-name-inferrer.cc (right):

http://codereview.chromium.org/7206015/diff/1/src/func-name-inferrer.cc#newco...
src/func-name-inferrer.cc:55: Isolate::Current()->runtime_state(),
name->Get(0))) {
On 2011/06/22 13:21:30, Vitaly Repeshko wrote:
> While we're here, let's fix the extra TLS loads by passing an isolate to the
> inferrer.

Done.

http://codereview.chromium.org/7206015/diff/1/src/func-name-inferrer.h
File src/func-name-inferrer.h (right):

http://codereview.chromium.org/7206015/diff/1/src/func-name-inferrer.h#newcode79
src/func-name-inferrer.h:79: // Infers a function name and leaves names
collection state.
On 2011/06/22 13:21:30, Vitaly Repeshko wrote:
> The comment should be fixed.

Done.

http://codereview.chromium.org/7206015/diff/1/src/func-name-inferrer.h#newcod...
src/func-name-inferrer.h:113: Handle<String> dot_;
On 2011/06/22 13:21:30, Vitaly Repeshko wrote:
> Function name inference is an integral part of the parser now. It's better to
> move these two symbols to the global list in the heap instead of allocating
them
> as strings here. 

Done.

Expand Messages | Collapse Messages