Infer VariableSet types.

Infer VariableSet types.

R=paulberry@google.com
BUG=

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

[scheglov, 2017-05-04 18:41:58]

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

https://codereview.chromium.org/2855323004/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart:908: return
inferrer.inferVariableSet(typeNeeded, declaredType, value);
We don't need "typeContext" in inferrer, should we still pass it in for
consistency?

[Paul Berry, 2017-05-04 19:31:03]

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

https://codereview.chromium.org/2855323004/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart:907: // TODO(scheglov)
Should we use the declared type, or the promoted type?
I believe the declared type is better, since the purpose of the promoted type is
to capture type information that is known about the value stored in the
variable.  Since a VariableSet obliterates the value stored in the variable, the
way we type infer it shouldn't be affected by what was previously stored in it.

https://codereview.chromium.org/2855323004/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart:908: return
inferrer.inferVariableSet(typeNeeded, declaredType, value);
On 2017/05/04 18:41:58, scheglov wrote:
> We don't need "typeContext" in inferrer, should we still pass it in for
> consistency?

I would prefer for us to pass it in for consistency with inferBoolLiteral,
inferDoubleLiteral, etc.

https://codereview.chromium.org/2855323004/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/2855323004/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart:317: return
typeNeeded ? valueType : null;
This seems unnecessary to me.  The convention is that all of the infer...()
functions always return `null` if `typeNeeded` is `false`, so we should be able
to just do:

return valueType;

[scheglov, 2017-05-04 19:41:01]

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

https://codereview.chromium.org/2855323004/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart:907: // TODO(scheglov)
Should we use the declared type, or the promoted type?
On 2017/05/04 19:31:03, Paul Berry wrote:
> I believe the declared type is better, since the purpose of the promoted type
is
> to capture type information that is known about the value stored in the
> variable.  Since a VariableSet obliterates the value stored in the variable,
the
> way we type infer it shouldn't be affected by what was previously stored in
it.

Acknowledged.

https://codereview.chromium.org/2855323004/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart:908: return
inferrer.inferVariableSet(typeNeeded, declaredType, value);
On 2017/05/04 19:31:03, Paul Berry wrote:
> On 2017/05/04 18:41:58, scheglov wrote:
> > We don't need "typeContext" in inferrer, should we still pass it in for
> > consistency?
> 
> I would prefer for us to pass it in for consistency with inferBoolLiteral,
> inferDoubleLiteral, etc.  

Done.

https://codereview.chromium.org/2855323004/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/2855323004/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart:317: return
typeNeeded ? valueType : null;
On 2017/05/04 19:31:03, Paul Berry wrote:
> This seems unnecessary to me.  The convention is that all of the infer...()
> functions always return `null` if `typeNeeded` is `false`, so we should be
able
> to just do:
> 
> return valueType;

Done.

[Paul Berry, 2017-05-04 19:54:03]

lgtm

[scheglov, 2017-05-04 20:37:05]

Description was changed from

==========
Infer VariableSet types.

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

to

==========
Infer VariableSet types.

R=paulberry@google.com
BUG=

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

[scheglov, 2017-05-04 20:37:06]

Committed patchset #2 (id:20001) manually as
a3db98c5defc7ed39ac0a3ce229642a838dfe1ae (presubmit successful).