Issue 10831035: Refactor conversion to native.

Issue 10831035: Refactor conversion to native. (Closed)

Created:
8 years, 4 months ago by Anton Muhin

Modified:
8 years, 4 months ago

Reviewers:
podivilov

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Refactor conversion to native. generator.py should be only aware of details, but shouldn't emit the code directly. Committed: https://code.google.com/p/dart/source/detail?r=11172

Patch Set 3 : #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+29 lines, -57 lines)			Patch
	M	lib/dom/scripts/generator.py	View	1 2	5 chunks	+14 lines, -51 lines	0 comments	Download
	M	lib/dom/scripts/systemnative.py	View	1 2	1 chunk	+15 lines, -6 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Anton Muhin

And another real review (not submitted). In this case I really prefer the suggested approach ...

8 years, 4 months ago (2012-07-26 18:33:49 UTC) #1

Anton Muhin

And even more radical variant (and I like it most :): https://chromiumcodereview.appspot.com/10831036/

8 years, 4 months ago (2012-07-26 18:53:05 UTC) #2

podivilov

https://chromiumcodereview.appspot.com/10831035/diff/1/lib/dom/scripts/generator.py File lib/dom/scripts/generator.py (left): https://chromiumcodereview.appspot.com/10831035/diff/1/lib/dom/scripts/generator.py#oldcode491 lib/dom/scripts/generator.py:491: emitter.Emit( I believe we had an agreement that the ...

8 years, 4 months ago (2012-08-21 12:11:31 UTC) #3

podivilov

lgtm https://chromiumcodereview.appspot.com/10831035/diff/4002/lib/dom/scripts/systemnative.py File lib/dom/scripts/systemnative.py (right): https://chromiumcodereview.appspot.com/10831035/diff/4002/lib/dom/scripts/systemnative.py#newcode833 lib/dom/scripts/systemnative.py:833: argument_expression, type, cls, function = self._TypeInfo(argument.type.id).to_native_info( consider returning ...

8 years, 4 months ago (2012-08-22 15:21:04 UTC) #5

Anton Muhin

8 years, 4 months ago (2012-08-22 16:38:51 UTC) #6

https://chromiumcodereview.appspot.com/10831035/diff/4002/lib/dom/scripts/sys...
File lib/dom/scripts/systemnative.py (right):

https://chromiumcodereview.appspot.com/10831035/diff/4002/lib/dom/scripts/sys...
lib/dom/scripts/systemnative.py:833: argument_expression, type, cls, function =
self._TypeInfo(argument.type.id).to_native_info(
On 2012/08/22 15:21:05, podivilov wrote:
> consider returning 'cls::function' to simplify to_native_info 'interface'.

I'd rather not duplicate this logic over here, let's see how it goes in the
future.

https://chromiumcodereview.appspot.com/10831035/diff/4002/lib/dom/scripts/sys...
lib/dom/scripts/systemnative.py:848: NO=start_index + i)
On 2012/08/22 15:21:05, podivilov wrote:
> nit: INDEX=...

Done.

Expand Messages | Collapse Messages