frog: use type annotation in map and list literals

frog: use type annotation in map and list literals
BUG= http://code.google.com/p/dart/issues/detail?id=220 http://code.google.com/p/dart/issues/detail?id=221

Committed: https://code.google.com/p/dart/source/detail?r=2186

[Jennifer Messerly, 2011-12-08 02:13:26]

http://codereview.chromium.org/8849001/diff/2001/frog/gen.dart
File frog/gen.dart (right):

http://codereview.chromium.org/8849001/diff/2001/frog/gen.dart#newcode2159
frog/gen.dart:2159: var arg = type === null ? visitValue(item) :
visitTypedValue(item, type);
I'd move this check into visitTypedValue. Especially since you do the same thing
later :)

Also style nit: we aren't using ===, !== anywhere else for null comparions. Any
reason to change?

http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart
File frog/parser.dart (right):

http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart#newcode1459
frog/parser.dart:1459: if (genericType.typeArguments.length != 1) {
probably worth pulling "genericType.typeArguments" into a temp since it's used 5
times as an rvalue.

http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart#newcode1460
frog/parser.dart:1460: _error('a map literal takes one type argument specfying
the value type',
I hope we follow up with the spec here. This current rule doesn't seem very
future proofed. I can see either: 1. the Map type allows constant keys in
literals, or 2. other collections get literals and also need the <...> syntax,
but what if they need more than one type arg?

Maybe add a TODO here?

http://codereview.chromium.org/8849001/diff/2001/tests/language/language.status
File tests/language/language.status (right):

http://codereview.chromium.org/8849001/diff/2001/tests/language/language.stat...
tests/language/language.status:258: ListLiteral3Test: Fail # vm and frog throw
different exceptions
What are the different exceptions?

[Siggi Cherem (dart-lang), 2011-12-08 23:02:15]

http://codereview.chromium.org/8849001/diff/2001/frog/gen.dart
File frog/gen.dart (right):

http://codereview.chromium.org/8849001/diff/2001/frog/gen.dart#newcode2159
frog/gen.dart:2159: var arg = type === null ? visitValue(item) :
visitTypedValue(item, type);
On 2011/12/08 02:13:26, John Messerly wrote:
> I'd move this check into visitTypedValue. Especially since you do the same
thing
> later :)
> 
Done

> Also style nit: we aren't using ===, !== anywhere else for null comparions.
Any
> reason to change?

No reason - seems that our code needs some cleanup (there are still some left
behind)... addressing in a follow up CL.

http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart
File frog/parser.dart (right):

http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart#newcode1459
frog/parser.dart:1459: if (genericType.typeArguments.length != 1) {
On 2011/12/08 02:13:26, John Messerly wrote:
> probably worth pulling "genericType.typeArguments" into a temp since it's used
5
> times as an rvalue.

Done.

http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart#newcode1460
frog/parser.dart:1460: _error('a map literal takes one type argument specfying
the value type',
On 2011/12/08 02:13:26, John Messerly wrote:
> I hope we follow up with the spec here. This current rule doesn't seem very
> future proofed. I can see either: 1. the Map type allows constant keys in
> literals, or 2. other collections get literals and also need the <...> syntax,
> but what if they need more than one type arg?
> 
> Maybe add a TODO here?

Added TODO. Good news is that the spec actually has an explicit note to revisit
this.

http://codereview.chromium.org/8849001/diff/2001/tests/language/language.status
File tests/language/language.status (right):

http://codereview.chromium.org/8849001/diff/2001/tests/language/language.stat...
tests/language/language.status:258: ListLiteral3Test: Fail # vm and frog throw
different exceptions
On 2011/12/08 02:13:26, John Messerly wrote:
> What are the different exceptions?

vm: UnsupportedOperationException
frog: IllegalAccessException

turns out that we both do the same for map (IllegalAccessException), but not for
lists.

The vm seems inconsistent to me, so I filed this bug:
http://code.google.com/p/dart/issues/detail?id=766

[Jennifer Messerly, 2011-12-08 23:14:33]

On 2011/12/08 23:02:15, sigmund wrote:
> http://codereview.chromium.org/8849001/diff/2001/frog/gen.dart
> File frog/gen.dart (right):
> 
> http://codereview.chromium.org/8849001/diff/2001/frog/gen.dart#newcode2159
> frog/gen.dart:2159: var arg = type === null ? visitValue(item) :
> visitTypedValue(item, type);
> On 2011/12/08 02:13:26, John Messerly wrote:
> > I'd move this check into visitTypedValue. Especially since you do the same
> thing
> > later :)
> > 
> Done
> 
> > Also style nit: we aren't using ===, !== anywhere else for null comparions.
> Any
> > reason to change?
> 
> No reason - seems that our code needs some cleanup (there are still some left
> behind)... addressing in a follow up CL.
> 
> http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart
> File frog/parser.dart (right):
> 
> http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart#newcode1459
> frog/parser.dart:1459: if (genericType.typeArguments.length != 1) {
> On 2011/12/08 02:13:26, John Messerly wrote:
> > probably worth pulling "genericType.typeArguments" into a temp since it's
used
> 5
> > times as an rvalue.
> 
> Done.
> 
> http://codereview.chromium.org/8849001/diff/2001/frog/parser.dart#newcode1460
> frog/parser.dart:1460: _error('a map literal takes one type argument specfying
> the value type',
> On 2011/12/08 02:13:26, John Messerly wrote:
> > I hope we follow up with the spec here. This current rule doesn't seem very
> > future proofed. I can see either: 1. the Map type allows constant keys in
> > literals, or 2. other collections get literals and also need the <...>
syntax,
> > but what if they need more than one type arg?
> > 
> > Maybe add a TODO here?
> 
> Added TODO. Good news is that the spec actually has an explicit note to
revisit
> this.
> 
>
http://codereview.chromium.org/8849001/diff/2001/tests/language/language.status
> File tests/language/language.status (right):
> 
>
http://codereview.chromium.org/8849001/diff/2001/tests/language/language.stat...
> tests/language/language.status:258: ListLiteral3Test: Fail # vm and frog throw
> different exceptions
> On 2011/12/08 02:13:26, John Messerly wrote:
> > What are the different exceptions?
> 
> vm: UnsupportedOperationException
> frog: IllegalAccessException
> 
> turns out that we both do the same for map (IllegalAccessException), but not
for
> lists.
> 
> The vm seems inconsistent to me, so I filed this bug:
> http://code.google.com/p/dart/issues/detail?id=766

lgtm. thanks!