Add optional FunctionType.positionalParameterNames and use them to resynthesize function-type forma…

Add optional FunctionType.positionalParameterNames and use them to resynthesize function-type formal parameters in Analyzer.

This will be also used as an alternative approach to support of
typedefs in Analyzer. The previous one was reverted in
e431e93e872d9a1c97a5177ebb09d5416f1d659a, because it caused problems
during storing parameters of generic Function(s).


R=ahe@google.com, brianwilkerson@google.com, sigmund@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/66663f7fd6c088ee087f98f3543b80dfea600c4a

[Brian Wilkerson, 2017-07-28 19:56:27]

lgtm

[Siggi Cherem (dart-lang), 2017-07-31 17:43:08]

lgtm

https://codereview.chromium.org/2985293002/diff/1/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2985293002/diff/1/pkg/kernel/lib/ast.dart#new...
pkg/kernel/lib/ast.dart:4096: /// not `null`.
it might be a bit confusing if positionalParameters.length and
positionalParameterNames.length have different length (even if 0), is there a
reason why we can't use `null` as a default instead?

https://codereview.chromium.org/2985293002/diff/1/pkg/kernel/lib/ast.dart#new...
pkg/kernel/lib/ast.dart:4097: @informative
General question about what we should do about @informative fields: I know we
want to support having .dill files with and without this data (so we can exclude
it when it is not needed). What are your thoughts on how to exclude it:
 - compute it in fasta always, then omit it during serialization
 - make fasta smarter to optionally produce the data (e.g. use a compiler-option
to ensure they are built), then serialize in the same way regardless.

[scheglov, 2017-07-31 20:35:30]

https://codereview.chromium.org/2985293002/diff/1/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2985293002/diff/1/pkg/kernel/lib/ast.dart#new...
pkg/kernel/lib/ast.dart:4096: /// not `null`.
On 2017/07/31 17:43:08, Siggi Cherem (dart-lang) wrote:
> it might be a bit confusing if positionalParameters.length and
> positionalParameterNames.length have different length (even if 0), is there a
> reason why we can't use `null` as a default instead?
> 

In flatbuffers every type has default value. For lists default value is an empty
list. This makes life easier - we don't need separate flags to mark null values
- always just length and values.

https://codereview.chromium.org/2985293002/diff/1/pkg/kernel/lib/ast.dart#new...
pkg/kernel/lib/ast.dart:4097: @informative
On 2017/07/31 17:43:07, Siggi Cherem (dart-lang) wrote:
> General question about what we should do about @informative fields: I know we
> want to support having .dill files with and without this data (so we can
exclude
> it when it is not needed). What are your thoughts on how to exclude it:
>  - compute it in fasta always, then omit it during serialization
>  - make fasta smarter to optionally produce the data (e.g. use a
compiler-option
> to ensure they are built), then serialize in the same way regardless.

Well, ultimately any computation, which results we don't use, is useless and
should be prevented.
So, I think we should stop computing these values where it is cheaper to skip
than to compute.

Nothing prevents us from doing this at any point in time.

[scheglov, 2017-07-31 21:09:13]

Description was changed from

==========
Add optional FunctionType.positionalParameterNames and use them to resynthesize
function-type formal parameters in Analyzer.

This will be also used as an alternative approach to support of
typedefs in Analyzer. The previous one was reverted in
e431e93e872d9a1c97a5177ebb09d5416f1d659a, because it caused problems
during storing parameters of generic Function(s).


R=ahe@google.com, brianwilkerson@google.com, sigmund@google.com
BUG=
==========

to

==========
Add optional FunctionType.positionalParameterNames and use them to resynthesize
function-type formal parameters in Analyzer.

This will be also used as an alternative approach to support of
typedefs in Analyzer. The previous one was reverted in
e431e93e872d9a1c97a5177ebb09d5416f1d659a, because it caused problems
during storing parameters of generic Function(s).


R=ahe@google.com, brianwilkerson@google.com, sigmund@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/66663f7fd6c088ee087f98f3543b80dfea600c4a
==========

[scheglov, 2017-07-31 21:09:14]

Committed patchset #1 (id:1) manually as
66663f7fd6c088ee087f98f3543b80dfea600c4a (presubmit successful).

[Siggi Cherem (dart-lang), 2017-07-31 22:09:29]

Peter/Kevin - I'd like a second opinion from one of you on the changes here. I
just forgot to include this question on my first reply.

Konstantin has proposed two different ways to represent parameters name
information to reconstitute typedefs in the element model, it would be good to
hear your thoughts.

This CL has the second approach. The first approach was to store the extra
information directly in the Typedef nodes (see
https://codereview.chromium.org/2990783002/diff/40001/pkg/kernel/lib/ast.dart)

Hopefully you agree with the approach, otherwise we can always go back and amend
it again.