Infer the return types of local functions where appropriate.

Infer the return types of local functions where appropriate.

Note that we do this in order to be consistent with type inference of
function expressions.  See https://codereview.chromium.org/2209293002.

R=sigmund@google.com

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

[Paul Berry, 2017-06-21 22:07:47]

paulberry@google.com changed reviewers:
+ ahe@google.com, sigmund@google.com

[Siggi Cherem (dart-lang), 2017-06-21 22:34:27]

lgtm (but I didn't review the large chunk of the code that I believe was just
moved, if there were changes there, let me know and I'll look in more detail)

https://codereview.chromium.org/2950213002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart (right):

https://codereview.chromium.org/2950213002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart:643: DartType
inferLocalFunction(FunctionNode function, DartType typeContext,
I assume this just moved, but otherwise is the same?

https://codereview.chromium.org/2950213002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart:693: // Let
`[T/S]` denote the type substitution where each `Si` is replaced with
nit: wrap

[Paul Berry, 2017-06-22 17:44:20]

I'm going to go ahead and land this because even though the code that was moved
*was* changed, the changes are small and don't affect the old code path.  If you
have further comments, please let me know and I'll be happy to address them in a
follow-up CL.

https://codereview.chromium.org/2950213002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart (right):

https://codereview.chromium.org/2950213002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart:643: DartType
inferLocalFunction(FunctionNode function, DartType typeContext,
On 2017/06/21 22:34:27, Siggi Cherem (dart-lang) wrote:
> I assume this just moved, but otherwise is the same?

There are two small changes.  See comments below.

https://codereview.chromium.org/2950213002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart:644: bool
typeNeeded, int fileOffset, DartType returnContext, bool isNamed) {
Previously, returnContext was always initially `null`.  Now it is passed in as a
parameter.  In the old code path (KernelFunctionExpression._inferExpression), we
pass in `null`, so the behavior is unchanged.  In the new code path
(KernelFunctionDeclaration._inferStatement), we pass in the explicitly declared
return type, or `null` if there isn't one.  This allows the new code path to
supply the declared return type without having to synthesize 
an ersatz typeContext to contain it.

https://codereview.chromium.org/2950213002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart:693: // Let
`[T/S]` denote the type substitution where each `Si` is replaced with
On 2017/06/21 22:34:27, Siggi Cherem (dart-lang) wrote:
> nit: wrap

Done.

https://codereview.chromium.org/2950213002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart:742: bool
needToSetReturnType = hasImplicitReturnType &&
Previously this was:

bool needToSetReturnType = isExpressionFunction || strongMode;

In the old code path, hasImplicitReturnType is `true` and isNamed is `false`, so
the behavior is unchanged.  In the new code path, isNamed is `true` and
hasImplicitReturnType is determined by the presence or absence of an explicit
return type.  This ensures that (a) we don't try to change the return type if
one was explicitly given, and (b) in spec mode, we don't try to infer the return
type of named functions at all.

[Siggi Cherem (dart-lang), 2017-06-22 18:04:54]

still lgtm!

Thanks Paul for highlighting the two changes.

[Paul Berry, 2017-06-22 19:17:14]

Description was changed from

==========
Infer the return types of local functions where appropriate.

Note that we do this in order to be consistent with type inference of
function expressions.  See https://codereview.chromium.org/2209293002.
==========

to

==========
Infer the return types of local functions where appropriate.

Note that we do this in order to be consistent with type inference of
function expressions.  See https://codereview.chromium.org/2209293002.

R=sigmund@google.com

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

[Paul Berry, 2017-06-22 19:17:15]

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