Issue 113634: Propagate information on whether a non function was called as constructor or not

Issue 113634: Propagate information on whether a non function was called as constructor or not (Closed)

Created:
11 years, 7 months ago by Søren Thygesen Gjesse

Modified:
9 years, 7 months ago

Reviewers:
Christian Plesner Hansen, Mads Ager (chromium)

CC:
v8-dev

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

Visibility:
Public.

Description

Propagate information on whether a non function was called as constructor or not.The Arguments object passed to the callback now has IsConstructCall set accordingly.BUG=http://crbug.com/3285 Committed: http://code.google.com/p/v8/source/detail?r=2020

Patch Set 1 #

Total comments: 8

Patch Set 2 : '' #

Created: 11 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+136 lines, -47 lines)			Patch
M	src/arm/builtins-arm.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/bootstrapper.cc	View		1 chunk	+22 lines, -8 lines	0 comments	Download
M	src/builtins.h	View		2 chunks	+32 lines, -30 lines	0 comments	Download
M	src/builtins.cc	View	1	3 chunks	+26 lines, -5 lines	0 comments	Download
M	src/contexts.h	View	1	2 chunks	+3 lines, -0 lines	0 comments	Download
M	src/execution.h	View	1	1 chunk	+4 lines, -0 lines	0 comments	Download
M	src/execution.cc	View		1 chunk	+18 lines, -0 lines	0 comments	Download
M	src/ia32/builtins-ia32.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/runtime.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	src/runtime.cc	View		1 chunk	+8 lines, -0 lines	0 comments	Download
M	src/runtime.js	View		1 chunk	+12 lines, -0 lines	0 comments	Download
M	test/cctest/test-api.cc	View	1	2 chunks	+8 lines, -2 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Mads Ager (chromium)

LGTM http://codereview.chromium.org/113634/diff/1/8 File src/builtins.cc (right): http://codereview.chromium.org/113634/diff/1/8#newcode398 Line 398: // API that support calls. The object ...

11 years, 7 months ago (2009-05-20 12:23:28 UTC) #2

Christian Plesner Hansen

Lgtm http://codereview.chromium.org/113634/diff/1/8 File src/builtins.cc (right): http://codereview.chromium.org/113634/diff/1/8#newcode407 Line 407: ASSERT(!CalledAsConstructor()); Nit: I'd move the assert to ...

11 years, 7 months ago (2009-05-20 13:00:18 UTC) #3

Søren Thygesen Gjesse

11 years, 7 months ago (2009-05-20 19:29:32 UTC) #5

ARM builtins updated as well.

http://codereview.chromium.org/113634/diff/1/8
File src/builtins.cc (right):

http://codereview.chromium.org/113634/diff/1/8#newcode398
Line 398: // API that support calls. The object can be called as either a
constructor
On 2009/05/20 12:23:28, Mads Ager wrote:
> In all of these comments, I would remove the "that supports calls".

Done.

http://codereview.chromium.org/113634/diff/1/8#newcode407
Line 407: ASSERT(!CalledAsConstructor());
On 2009/05/20 13:00:18, Christian Plesner Hansen wrote:
> Nit: I'd move the assert to the very top of the function, before doing
anything
> else.

Done.

http://codereview.chromium.org/113634/diff/1/9
File src/contexts.h (right):

http://codereview.chromium.org/113634/diff/1/9#newcode94
Line 94: call_as_constructor_delegate) \
On 2009/05/20 12:23:28, Mads Ager wrote:
> Indentation seems slightly off here?

Done.

http://codereview.chromium.org/113634/diff/1/12
File src/execution.h (right):

http://codereview.chromium.org/113634/diff/1/12#newcode133
Line 133: static Handle<Object> GetConstructorDelegate(Handle<Object> object);
On 2009/05/20 12:23:28, Mads Ager wrote:
> Add comment like the one above?

Done.

Expand Messages | Collapse Messages