Support for NamedExpression(s) as constructor initializer arguments.

Support for NamedExpression(s) as constructor initializer arguments.

R=paulberry@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/5082e6f8f23b645bed560254b5649b80279cf5c8

[Paul Berry, 2016-05-05 21:47:02]

https://codereview.chromium.org/1952663006/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/idl.dart (right):

https://codereview.chromium.org/1952663006/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/idl.dart:1040: String get name;
It seems like an unfortunate inconsistency that we only use this mechanism for
named arguments to "super." and "this." constructor initializers, and for all
other kinds of named arguments, the expressions are serialized without reference
to their name, and the names are stored somewhere associated with the call site.
 (For example, with UnlinkedConstOperation.invokeConstructor, the names are
stored in UnlinkedConst.strings before the expressions for the constant values).

How about if, instead of storing the names here, we add an "argumentNames" field
(of type List<String>) to UnlinkedConstructorInitializer?

So for example, when serializing `class C { C() : super(0, x: 1, y: 2); }`,
UnlinkedConstructorInitializer.argumentNames would be `['x', 'y']`.

[scheglov, 2016-05-05 21:56:58]

https://codereview.chromium.org/1952663006/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/idl.dart (right):

https://codereview.chromium.org/1952663006/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/idl.dart:1040: String get name;
On 2016/05/05 21:47:02, Paul Berry wrote:
> It seems like an unfortunate inconsistency that we only use this mechanism for
> named arguments to "super." and "this." constructor initializers, and for all
> other kinds of named arguments, the expressions are serialized without
reference
> to their name, and the names are stored somewhere associated with the call
site.
>  (For example, with UnlinkedConstOperation.invokeConstructor, the names are
> stored in UnlinkedConst.strings before the expressions for the constant
values).
> 
> How about if, instead of storing the names here, we add an "argumentNames"
field
> (of type List<String>) to UnlinkedConstructorInitializer?
> 
> So for example, when serializing `class C { C() : super(0, x: 1, y: 2); }`,
> UnlinkedConstructorInitializer.argumentNames would be `['x', 'y']`.

Well, I actually started to implement it this way. But this makes implementation
more complex and less clear than adding "name" to UnlinkedConst. Instead of just
consistently serializing any Expression we would need to support two parallel
data structures - for simple expressions and optional names.

I see benefits of storing in-expression named arguments in "strings", but in
this case IMHO we can use simpler approach.

[Paul Berry, 2016-05-05 22:04:28]

lgtm

https://codereview.chromium.org/1952663006/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/idl.dart (right):

https://codereview.chromium.org/1952663006/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/idl.dart:1040: String get name;
On 2016/05/05 21:56:58, scheglov wrote:
> On 2016/05/05 21:47:02, Paul Berry wrote:
> > It seems like an unfortunate inconsistency that we only use this mechanism
for
> > named arguments to "super." and "this." constructor initializers, and for
all
> > other kinds of named arguments, the expressions are serialized without
> reference
> > to their name, and the names are stored somewhere associated with the call
> site.
> >  (For example, with UnlinkedConstOperation.invokeConstructor, the names are
> > stored in UnlinkedConst.strings before the expressions for the constant
> values).
> > 
> > How about if, instead of storing the names here, we add an "argumentNames"
> field
> > (of type List<String>) to UnlinkedConstructorInitializer?
> > 
> > So for example, when serializing `class C { C() : super(0, x: 1, y: 2); }`,
> > UnlinkedConstructorInitializer.argumentNames would be `['x', 'y']`.
> 
> Well, I actually started to implement it this way. But this makes
implementation
> more complex and less clear than adding "name" to UnlinkedConst. Instead of
just
> consistently serializing any Expression we would need to support two parallel
> data structures - for simple expressions and optional names.
> 
> I see benefits of storing in-expression named arguments in "strings", but in
> this case IMHO we can use simpler approach.

Hmm, It's not clear to me where the additional source of complexity comes in.

Since this is blocking the DDC team, let's land it as is.  I'll try to find time
to experiment with the alternative implementation and see if it becomes clear to
me why it's more complex.

[scheglov, 2016-05-05 22:13:25]

Description was changed from

==========
Support for NamedExpression(s) as constructor initializer arguments.

R=paulberry@google.com
BUG=
==========

to

==========
Support for NamedExpression(s) as constructor initializer arguments.

R=paulberry@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/5082e6f8f23b645bed560254b5649b80279cf5c8
==========

[scheglov, 2016-05-05 22:13:28]

Committed patchset #1 (id:1) manually as
5082e6f8f23b645bed560254b5649b80279cf5c8 (presubmit successful).