Issue 9921010: Address review comments from: https://chromiumcodereview.appspot.com/9750003.

Issue 9921010: Address review comments from: https://chromiumcodereview.appspot.com/9750003. (Closed)

Created:
8 years, 8 months ago by ngeoffray

Modified:
8 years, 8 months ago

Reviewers:
floitsch, sra1

CC:
reviews_dartlang.org, ahe, kasperl, karlklose, Lasse Reichstein Nielsen

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

Visibility:
Public.

More Reviews

Description

Address review comments from: https://chromiumcodereview.appspot.com/9750003. Committed: https://code.google.com/p/dart/source/detail?r=6026

Patch Set 1 #

Total comments: 2

Created: 8 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+66 lines, -27 lines)			Patch
A +	frog/tests/native/src/NativeNoSuchMethodException2FrogTest.dart	View	2 chunks	+20 lines, -4 lines	0 comments	Download
M	lib/compiler/implementation/lib/js_helper.dart	View	1 chunk	+2 lines, -1 line	0 comments	Download
M	lib/compiler/implementation/lib/native_helper.dart	View	10 chunks	+42 lines, -20 lines	2 comments	Download
M	lib/compiler/implementation/native_emitter.dart	View	2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

sra1

LGTM https://chromiumcodereview.appspot.com/9921010/diff/1/lib/compiler/implementation/lib/native_helper.dart File lib/compiler/implementation/lib/native_helper.dart (right): https://chromiumcodereview.appspot.com/9921010/diff/1/lib/compiler/implementation/lib/native_helper.dart#newcode147 lib/compiler/implementation/lib/native_helper.dart:147: // is a subclass. It is not really ...

8 years, 8 months ago (2012-03-30 00:14:54 UTC) #3

ngeoffray

8 years, 8 months ago (2012-03-30 09:27:11 UTC) #4

Thanks Florian and Stephen,

https://chromiumcodereview.appspot.com/9921010/diff/1/lib/compiler/implementa...
File lib/compiler/implementation/lib/native_helper.dart (right):

https://chromiumcodereview.appspot.com/9921010/diff/1/lib/compiler/implementa...
lib/compiler/implementation/lib/native_helper.dart:147: // is a subclass.
On 2012/03/30 00:14:54, sra1 wrote:
> It is not really calling dynamicBind 'again'.
> It is a fresh call to the dynamically bound method that hits this trampoline,
> and if from a subclass it needs to call dynamicBind to get the correct method
> rather than 'using' this incorrect one.

Could you elaborate? It looks to me that if you have reached this trampoline,
that's because the subclass did not have a chance to patch its own prototype. So
you end up calling the method attached on the Object.prototype, that will in
turn call dynamicBind.

Expand Messages | Collapse Messages