Fix factories in Frog to correspond to the new syntax.

Fix factories in Frog to correspond to the new syntax.
(Note: this is designed to work against corelib changes in http://codereview.chromium.org/8931011/, but it also works against current checked in corelib) I kept old factories limping along in Frog. We can remove them soon

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

[Jennifer Messerly, 2011-12-15 02:14:33]

regis, zundel let me know how you'd like to coordinate checking this in

[sra1, 2011-12-15 02:38:16]

DBC

http://codereview.chromium.org/8952006/diff/1/frog/parser.dart
File frog/parser.dart (right):

http://codereview.chromium.org/8952006/diff/1/frog/parser.dart#newcode283
frog/parser.dart:283: world.warning('factory no longer supported, use "default"
instead',
I think Frog warnings need to be consistent with quoting.
I would suggest always using single quotes in the message, e.g.

world.warning("'factory' no longer supported, use 'default' instead",

[ahe, 2011-12-15 07:19:58]

Hi John,

Thank you for taking care of this. The changes to leg LGTM. 

Cheers,
Peter

[zundel, 2011-12-15 14:28:34]

On 2011/12/15 02:14:33, John Messerly wrote:
> regis, zundel let me know how you'd like to coordinate checking this in

The vm code is already checked in. If you want to go ahead and commit, that's
fine. I can commit the corelib changes and dartc changes after my patch passes
review.

[regis, 2011-12-15 17:01:19]

LGTM

You guys can go ahead and submit. The VM supports both old and new syntax.

[jimhug, 2011-12-15 17:05:02]

LGTM!

This is a great step forward for the language (thanks, Gilad!) and I like how
its implemented.  Personally, I like the incremental way this is going in with
support for the deprecated old syntax.  It's fine to wait a bit to rip that out
so that users can get clear messages.  Then it will be a great celebration when
you have the next CL that just rips things out.

FYI - When building AspectJ we used this trick very extensively to handle a
language in rapid flux that also had real enterprise users.  It's worth
considering adding a little more infrastructure to the compiler to make this
easier as we enter the home stretch for dart v1 and should have a similar
situation of wanting to agressively make language changes - but also not break
existing code too soon or without friendly warnings.

http://codereview.chromium.org/8952006/diff/1/frog/parser.dart
File frog/parser.dart (right):

http://codereview.chromium.org/8952006/diff/1/frog/parser.dart#newcode283
frog/parser.dart:283: world.warning('factory no longer supported, use "default"
instead',
SGTM.  I just checked and both Python and JS use single quotes so I think
Stephen's choice is the rightest <smile> one.  This is obviously low-pri, but a
good thing to keep in mind - and someone (me?) should get a wiki setup for
capturing these kinds of conventions.
On 2011/12/15 02:38:17, sra1 wrote:
> I think Frog warnings need to be consistent with quoting.
> I would suggest always using single quotes in the message, e.g.
> 
> world.warning("'factory' no longer supported, use 'default' instead",

http://codereview.chromium.org/8952006/diff/1/frog/parser.dart#newcode433
frog/parser.dart:433: // I left it here for now to support old-style factories
I think we need this "forever" as we still have factory constructors - they just
don't share a name with default implementations anymore (yay!).  However, the
typeParameters part below can go away pretty soon - yay!

http://codereview.chromium.org/8952006/diff/1/frog/scripts/tree_gen.py
File frog/scripts/tree_gen.py (left):

http://codereview.chromium.org/8952006/diff/1/frog/scripts/tree_gen.py#oldcod...
frog/scripts/tree_gen.py:113: 'List<FormalNode> formals, List<ParameterType>
typeParameters,' +
Whoo hoo!  You can't imagine how much joy I get out of seeing this go away...

http://codereview.chromium.org/8952006/diff/1/frog/scripts/tree_gen.py
File frog/scripts/tree_gen.py (right):

http://codereview.chromium.org/8952006/diff/1/frog/scripts/tree_gen.py#newcod...
frog/scripts/tree_gen.py:103: 'NativeType nativeType, DefaultTypeReference
defaultType, '+
This makes me smile!  Thanks for doing the thorough fix and getting the field
names right as well - it would have been very easy to skip this kind of cleanup
but the code feels so much better when you do it right.

[Siggi Cherem (dart-lang), 2011-12-15 18:09:23]

lgtm

[Jennifer Messerly, 2011-12-15 19:28:15]

thanks! I'll upload a new version without corelib changes & land it

http://codereview.chromium.org/8952006/diff/1/frog/parser.dart
File frog/parser.dart (right):

http://codereview.chromium.org/8952006/diff/1/frog/parser.dart#newcode283
frog/parser.dart:283: world.warning('factory no longer supported, use "default"
instead',
On 2011/12/15 17:05:04, jimhug wrote:
> SGTM.  I just checked and both Python and JS use single quotes so I think
> Stephen's choice is the rightest <smile> one.  This is obviously low-pri, but
a
> good thing to keep in mind - and someone (me?) should get a wiki setup for
> capturing these kinds of conventions.
> On 2011/12/15 02:38:17, sra1 wrote:
> > I think Frog warnings need to be consistent with quoting.
> > I would suggest always using single quotes in the message, e.g.
> > 
> > world.warning("'factory' no longer supported, use 'default' instead",
> 

+1 on consistency. I'm trying to be consistent with most of the other messages
in Frog which use double quotes AFAIK. Let's do the mass search/replace at some
point in the future--it seems much lower priority than just about anything else
we could work on.

http://codereview.chromium.org/8952006/diff/1/frog/parser.dart#newcode433
frog/parser.dart:433: // I left it here for now to support old-style factories
On 2011/12/15 17:05:04, jimhug wrote:
> I think we need this "forever" as we still have factory constructors - they
just
> don't share a name with default implementations anymore (yay!).  However, the
> typeParameters part below can go away pretty soon - yay!

Ah, I might be misremembering the history of the code. At one point weren't
factories handled through finishDefinition? Other than the "factory" modifier,
they look just like normal constructors now, AFAIK.