Fix hints reported by analyzer.

Fix hints reported by analyzer.

BUG=https://github.com/dart-lang/sdk/issues/28454
R=johnniwinther@google.com

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

[ahe, 2017-01-20 16:54:16]

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

[Siggi Cherem (dart-lang), 2017-01-20 18:12:32]

Thanks Peter! This is great.

Most changes are great, there are a couple small cases where the hints might be
onto something wrong with the code (in constants and serialization). Johnni is
very familiar with that part of the code.

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/compil...
File pkg/compiler/lib/src/compile_time_constants.dart (left):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:1297:
Elements.addForwardingElementArgumentsToList<AstConstant>(constructor,
oh my, this actually seems to be a bit broken. I think the last argument here
should not be compileConstant, but a function that wraps it into an AstConstant.

Johnni - do you agree?

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/js_bac...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/js_bac...
pkg/compiler/lib/src/js_backend/backend.dart:902: mask.canHit(element as
MemberElement, selector, _closedWorld));
may be we can change the type of [elements] above in 887 and 889 to be
Set<MemberElement> instead, , and [interceptedMembers] to Map<String,
Set<MemberElement>>?

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/serial...
File pkg/compiler/lib/src/serialization/serialization_util.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/serial...
pkg/compiler/lib/src/serialization/serialization_util.dart:55:
decoder.getString(Key.NAME);
mmm... I'm guessing we should delete these 3 lines? (the deserializeName is
doing the same thing). Johhni can confirm

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:600:
astAdapter.getDartType(_targetFunction.returnType);
you can delete the entire line here

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/ssa/ke...
File pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/ssa/ke...
pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:958: return new
VariableConstantExpression(element as VariableElement);
To continue following the general dart2js style here:
  VariableElement variable = element;
  return new VariableConstantExpression(variable);

[Johnni Winther, 2017-01-23 09:31:55]

lgtm

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/compil...
File pkg/compiler/lib/src/compile_time_constants.dart (left):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:1297:
Elements.addForwardingElementArgumentsToList<AstConstant>(constructor,
On 2017/01/20 18:12:32, Siggi Cherem (dart-lang) wrote:
> oh my, this actually seems to be a bit broken. I think the last argument here
> should not be compileConstant, but a function that wraps it into an
AstConstant.
> 
> Johnni - do you agree?

It should. Weird why this hasn't failed at runtime...

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/js_bac...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/js_bac...
pkg/compiler/lib/src/js_backend/backend.dart:902: mask.canHit(element as
MemberElement, selector, _closedWorld));
On 2017/01/20 18:12:32, Siggi Cherem (dart-lang) wrote:
> may be we can change the type of [elements] above in 887 and 889 to be
> Set<MemberElement> instead, , and [interceptedMembers] to Map<String,
> Set<MemberElement>>?

I'll do that in a later CL. This is ok for now.

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/serial...
File pkg/compiler/lib/src/serialization/serialization_util.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/serial...
pkg/compiler/lib/src/serialization/serialization_util.dart:55:
decoder.getString(Key.NAME);
On 2017/01/20 18:12:32, Siggi Cherem (dart-lang) wrote:
> mmm... I'm guessing we should delete these 3 lines? (the deserializeName is
> doing the same thing). Johhni can confirm

Yes, delete.

[ahe, 2017-01-23 09:45:23]

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/compil...
File pkg/compiler/lib/src/compile_time_constants.dart (left):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:1297:
Elements.addForwardingElementArgumentsToList<AstConstant>(constructor,
On 2017/01/23 09:31:55, Johnni Winther wrote:
> On 2017/01/20 18:12:32, Siggi Cherem (dart-lang) wrote:
> > oh my, this actually seems to be a bit broken. I think the last argument
here
> > should not be compileConstant, but a function that wraps it into an
> AstConstant.
> > 
> > Johnni - do you agree?
> 
> It should. Weird why this hasn't failed at runtime...

What should I do?

[Johnni Winther, 2017-01-23 10:13:43]

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/compil...
File pkg/compiler/lib/src/compile_time_constants.dart (left):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:1297:
Elements.addForwardingElementArgumentsToList<AstConstant>(constructor,
On 2017/01/23 09:45:22, ahe wrote:
> On 2017/01/23 09:31:55, Johnni Winther wrote:
> > On 2017/01/20 18:12:32, Siggi Cherem (dart-lang) wrote:
> > > oh my, this actually seems to be a bit broken. I think the last argument
> here
> > > should not be compileConstant, but a function that wraps it into an
> > AstConstant.
> > > 
> > > Johnni - do you agree?
> > 
> > It should. Weird why this hasn't failed at runtime...
> 
> What should I do?

Just remove the type argument to avoid the hint. I'll look into it.

[ahe, 2017-01-23 10:58:37]

Thank you, Siggi and Johnni!

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/serial...
File pkg/compiler/lib/src/serialization/serialization_util.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/serial...
pkg/compiler/lib/src/serialization/serialization_util.dart:55:
decoder.getString(Key.NAME);
On 2017/01/23 09:31:55, Johnni Winther wrote:
> On 2017/01/20 18:12:32, Siggi Cherem (dart-lang) wrote:
> > mmm... I'm guessing we should delete these 3 lines? (the deserializeName is
> > doing the same thing). Johhni can confirm
> 
> Yes, delete.

Done.

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:600:
astAdapter.getDartType(_targetFunction.returnType);
On 2017/01/20 18:12:32, Siggi Cherem (dart-lang) wrote:
> you can delete the entire line here

Done.

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/ssa/ke...
File pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart (right):

https://codereview.chromium.org/2647043002/diff/1/pkg/compiler/lib/src/ssa/ke...
pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:958: return new
VariableConstantExpression(element as VariableElement);
On 2017/01/20 18:12:32, Siggi Cherem (dart-lang) wrote:
> To continue following the general dart2js style here:
>   VariableElement variable = element;
>   return new VariableConstantExpression(variable);

I think there's a bug in the analyzer. It's as if it ignores types of local
variables.

[ahe, 2017-01-23 11:05:34]

Description was changed from

==========
Fix hints reported by analyzer.

BUG=https://github.com/dart-lang/sdk/issues/28454
==========

to

==========
Fix hints reported by analyzer.

BUG=https://github.com/dart-lang/sdk/issues/28454
R=johnniwinther@google.com

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

[ahe, 2017-01-23 11:05:35]

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