When reordering constructor initializers, use correct types for temp vars.

When reordering constructor initializers, use correct types for temp vars.

In strong mode, when a call to a super-initializer is reordered, we
can use the static type of the super-initializer arguments to set the
types of the temporary variables that we use to do the reordering.
This is desirable because it might help avoid unnecessary casts.

In non-strong mode, we use `dynamic` for the temporary variables, to
replicate Dart 1.0 behavior.

R=scheglov@google.com

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

[Paul Berry, 2017-08-07 20:32:21]

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

[Paul Berry, 2017-08-07 20:32:22]

I only need a review from one of you.

[scheglov, 2017-08-07 20:38:34]

LGTM

[Paul Berry, 2017-08-07 20:55:44]

Description was changed from

==========
When reordering constructor initializers, use correct types for temp vars.

In strong mode, when a call to a super-initializer is reordered, we
can use the static type of the super-initializer arguments to set the
types of the temporary variables that we use to do the reordering.
This is desirable because it might help avoid unnecessary casts.

In non-strong mode, we use `dynamic` for the temporary variables, to
replicate Dart 1.0 behavior.
==========

to

==========
When reordering constructor initializers, use correct types for temp vars.

In strong mode, when a call to a super-initializer is reordered, we
can use the static type of the super-initializer arguments to set the
types of the temporary variables that we use to do the reordering.
This is desirable because it might help avoid unnecessary casts.

In non-strong mode, we use `dynamic` for the temporary variables, to
replicate Dart 1.0 behavior.

R=scheglov@google.com

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

[Paul Berry, 2017-08-07 20:55:45]

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

[ahe, 2017-08-08 13:31:18]

lgtm

I thought strong mode didn't support moving super, and it's a compile-time error
if it isn't last.

https://codereview.chromium.org/2993193002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart (right):

https://codereview.chromium.org/2993193002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart:426: var type =
typeInferrer.typeSchemaEnvironment.strongMode
Missing type on variable.

[Paul Berry, 2017-08-08 16:40:05]

On 2017/08/08 13:31:18, ahe wrote:
> lgtm
> 
> I thought strong mode didn't support moving super, and it's a compile-time
error
> if it isn't last.

Whoops, you're right.  Thanks for pointing it out--I had forgotten.

I think I'm going to leave the changes in place since they will lead to
(slightly) better behavior in the error-recovered case.  But it's nice to know
that production code won't have to rely on this :)

[Paul Berry, 2017-08-08 16:42:25]

https://codereview.chromium.org/2993193002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart (right):

https://codereview.chromium.org/2993193002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart:426: var type =
typeInferrer.typeSchemaEnvironment.strongMode
On 2017/08/08 13:31:18, ahe wrote:
> Missing type on variable.

IMHO this is ok--type inference will infer DartType, and anyone familiar with
the code will know this is a DartType as well.

[ahe, 2017-08-08 17:35:44]

https://codereview.chromium.org/2993193002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart (right):

https://codereview.chromium.org/2993193002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart:426: var type =
typeInferrer.typeSchemaEnvironment.strongMode
On 2017/08/08 16:42:25, Paul Berry wrote:
> On 2017/08/08 13:31:18, ahe wrote:
> > Missing type on variable.
> 
> IMHO this is ok--type inference will infer DartType, and anyone familiar with
> the code will know this is a DartType as well.

I wouldn't have complained if hadn't used a type on line 429, but in this case,
the lack of type caused me to stumble when reading this. I had to scan two lines
ahead to figure out that this was probably DartType (based on const
DynamicType).

[ahe, 2017-08-08 17:36:01]

On 2017/08/08 16:40:05, Paul Berry wrote:
> On 2017/08/08 13:31:18, ahe wrote:
> > lgtm
> > 
> > I thought strong mode didn't support moving super, and it's a compile-time
> error
> > if it isn't last.
> 
> Whoops, you're right.  Thanks for pointing it out--I had forgotten.
> 
> I think I'm going to leave the changes in place since they will lead to
> (slightly) better behavior in the error-recovered case.  But it's nice to know
> that production code won't have to rely on this :)

I hoped you'd say that :-D