Issue 11468016: Rename GET_NATIVE_ARGUMENT macro to GET_NON_NULL_NATIVE_ARGUMENT.

Issue 11468016: Rename GET_NATIVE_ARGUMENT macro to GET_NON_NULL_NATIVE_ARGUMENT. (Closed)

Created:
8 years ago by regis

Modified:
8 years ago

Reviewers:
srdjan, siva

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Rename GET_NATIVE_ARGUMENT macro to GET_NON_NULL_NATIVE_ARGUMENT. Introduce new GET_NATIVE_ARGUMENT macro accepting null. Add test. Committed: https://code.google.com/p/dart/source/detail?r=15919

Patch Set 2 : #

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+299 lines, -190 lines)			Patch
M	runtime/lib/array.cc	View	1	4 chunks	+7 lines, -7 lines	0 comments	Download
M	runtime/lib/byte_array.cc	View	1	24 chunks	+98 lines, -97 lines	0 comments	Download
M	runtime/lib/date.cc	View	1	2 chunks	+4 lines, -2 lines	0 comments	Download
M	runtime/lib/double.cc	View	1	15 chunks	+17 lines, -16 lines	0 comments	Download
M	runtime/lib/error.cc	View	1	3 chunks	+6 lines, -5 lines	0 comments	Download
M	runtime/lib/growable_array.cc	View	1	5 chunks	+6 lines, -6 lines	0 comments	Download
M	runtime/lib/integers.cc	View	1	13 chunks	+13 lines, -13 lines	0 comments	Download
M	runtime/lib/isolate.cc	View	1	3 chunks	+6 lines, -6 lines	0 comments	Download
M	runtime/lib/math.cc	View	1	1 chunk	+11 lines, -11 lines	0 comments	Download
M	runtime/lib/mirrors.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	runtime/lib/object.cc	View	1	1 chunk	+3 lines, -2 lines	0 comments	Download
M	runtime/lib/regexp.cc	View	1	2 chunks	+7 lines, -5 lines	0 comments	Download
M	runtime/lib/string.cc	View	1	8 chunks	+10 lines, -10 lines	0 comments	Download
M	runtime/lib/weak_property.cc	View	1	2 chunks	+9 lines, -6 lines	0 comments	Download
M	runtime/vm/code_generator_test.cc	View	1	2 chunks	+50 lines, -1 line	0 comments	Download
M	runtime/vm/native_entry.h	View	1	2 chunks	+17 lines, -2 lines	0 comments	Download
M	runtime/vm/native_entry_test.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	runtime/vm/native_entry_test.cc	View	1	2 chunks	+33 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

regis

The new macro is not optimal. It allocates 2 handles and the resulting handle is ...

8 years ago (2012-12-07 01:26:06 UTC) #1

siva

LGTM with one comment. https://codereview.chromium.org/11468016/diff/1/runtime/vm/native_entry.h File runtime/vm/native_entry.h (right): https://codereview.chromium.org/11468016/diff/1/runtime/vm/native_entry.h#newcode79 runtime/vm/native_entry.h:79: name ^= value; \ Shouldn't ...

8 years ago (2012-12-07 02:28:39 UTC) #2

regis

8 years ago (2012-12-10 17:58:42 UTC) #4

Thanks!

https://codereview.chromium.org/11468016/diff/1/runtime/vm/native_entry.h
File runtime/vm/native_entry.h (right):

https://codereview.chromium.org/11468016/diff/1/runtime/vm/native_entry.h#new...
runtime/vm/native_entry.h:79: name ^= value;                                    
                        \
On 2012/12/07 02:28:40, siva wrote:
> Shouldn't the assignment above be done irrespective of whether it is NULL or
non
> NULL.
> 
> Otherwise the vtable entry in the NULL case would still point
> to  Instance and if the native code did an Is<type> call for whatever reason
it
> would return false.

Done.

https://codereview.chromium.org/11468016/diff/1/runtime/vm/native_entry_test.cc
File runtime/vm/native_entry_test.cc (right):

https://codereview.chromium.org/11468016/diff/1/runtime/vm/native_entry_test....
runtime/vm/native_entry_test.cc:71: GET_NATIVE_ARGUMENT(Integer, argument,
arguments->NativeArgAt(i));
On 2012/12/07 02:28:40, siva wrote:
> EXPECT(argument.IsInteger());
> 
> Does that make sense?

Done.

Expand Messages | Collapse Messages