Issue 9558005: Implement literal maps.

Issue 9558005: Implement literal maps. (Closed)

Created:
8 years, 9 months ago by ahe

Modified:
8 years, 9 months ago

Reviewers:
floitsch, ngeoffray

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

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

Visibility:
Public.

More Reviews

Description

Implement literal maps. Committed: https://code.google.com/p/dart/source/detail?r=5032

Patch Set 2 : Address review comments #

Created: 8 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+45 lines, -25 lines)			Patch
M	dart/frog/leg/lib/js_helper.dart	View	1	2 chunks	+18 lines, -2 lines	0 comments	Download
M	dart/frog/leg/resolver.dart	View	1	1 chunk	+2 lines, -2 lines	0 comments	Download
M	dart/frog/leg/ssa/builder.dart	View	1	2 chunks	+20 lines, -2 lines	0 comments	Download
M	dart/frog/leg/ssa/closure.dart	View	1	1 chunk	+0 lines, -4 lines	0 comments	Download
M	dart/frog/leg/tree/nodes.dart	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	dart/frog/tests/leg_only/src/ToStringTest.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	dart/tests/co19/co19-leg.status	View	1	1 chunk	+0 lines, -5 lines	0 comments	Download
M	dart/tests/corelib/corelib-leg.status	View	1	2 chunks	+0 lines, -4 lines	0 comments	Download
M	dart/tests/language/language-leg.status	View	1	5 chunks	+2 lines, -5 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

ngeoffray

LGTM! https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/lib/js_helper.dart File dart/frog/leg/lib/js_helper.dart (right): https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/lib/js_helper.dart#newcode1306 dart/frog/leg/lib/js_helper.dart:1306: var iterator = keyValuePairs.iterator(); var -> Iterator ? ...

8 years, 9 months ago (2012-03-01 08:36:09 UTC) #2

floitsch

dbc. https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/lib/js_helper.dart File dart/frog/leg/lib/js_helper.dart (right): https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/lib/js_helper.dart#newcode1311 dart/frog/leg/lib/js_helper.dart:1311: // TODO(ahe): Don't use UNINTERCEPTED when optimizer is ...

8 years, 9 months ago (2012-03-01 10:24:07 UTC) #3

ahe

8 years, 9 months ago (2012-03-06 15:34:12 UTC) #4

Hi Nicolas and Florian,

Thank you for taking a look.

Cheers,
Peter

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/lib/js_he...
File dart/frog/leg/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/lib/js_he...
dart/frog/leg/lib/js_helper.dart:1306: var iterator = keyValuePairs.iterator();
On 2012/03/01 08:36:09, ngeoffray wrote:
> var -> Iterator ?

Done.

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/lib/js_he...
dart/frog/leg/lib/js_helper.dart:1309: var key = iterator.next();
On 2012/03/01 08:36:09, ngeoffray wrote:
> var key -> String key?

Done.

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/lib/js_he...
dart/frog/leg/lib/js_helper.dart:1311: // TODO(ahe): Don't use UNINTERCEPTED
when optimizer is smarter.
On 2012/03/01 10:24:07, floitsch wrote:
> Is this necessary or just a premature optimization?

Premature optimization to make the code readable. I'll try to do without it.

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/ssa/build...
File dart/frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/ssa/build...
dart/frog/leg/ssa/builder.dart:2252: push(new
HStatic(interceptors.getMapMaker()));
On 2012/03/01 08:36:09, ngeoffray wrote:
> Note that you don't really need to push, you could just call 'add'. Using
'add'
> forces you to introduce a new local variable though.
> 
> No strong preference, I just wanted to make sure you knew you could call
'add'.

Done.

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/ssa/build...
dart/frog/leg/ssa/builder.dart:2253: push(new
HInvokeStatic(Selector.INVOCATION_1, <HInstruction>[pop(), pop()]));
On 2012/03/01 08:36:09, ngeoffray wrote:
> Line too long.

Done.

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/tree/node...
File dart/frog/leg/tree/nodes.dart (right):

https://chromiumcodereview.appspot.com/9558005/diff/1/dart/frog/leg/tree/node...
dart/frog/leg/tree/nodes.dart:1275: bool isConst() => false; // TODO(ahe): Store
constness.
On 2012/03/01 08:36:09, ngeoffray wrote:
> Where is this used?

When I create the list of key/value pairs.

Expand Messages | Collapse Messages