Infer types for parameters even in the presence of InvocationMirror.delegate.

Infer types for parameters even in the presence of InvocationMirror.delegate.

R=kasperl@google.com

Committed: https://code.google.com/p/dart/source/detail?r=29431

[ngeoffray, 2013-10-28 21:24:10]

Looks like it is *that* simple...

[kasperl, 2013-10-29 08:35:27]

LGTM!

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
File
sdk/lib/_internal/compiler/implementation/inferrer/simple_types_inferrer.dart
(right):

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/simple_types_inferrer.dart:796:
} if (node.isPropertyAccess
if -> else if

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:488:
bool canReachAll = compiler.enabledInvokeOn
Move this code to the selector? Maybe it could be something like:

  Selector selectorToUse = typedSelector.computeEffectiveSelector(compiler);

That way you don't have to compute the mask either.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:492: //
the untyped selector (through noSuchMethod's `InvocationMirror`
InvocationMirror -> Invocation.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:492: //
the untyped selector (through noSuchMethod's `InvocationMirror`
InvocationMirror -> Invocation.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:516: //
`InvocationMirror.delegate`.
InvocationMirror -> Invocation.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:516: //
`InvocationMirror.delegate`.
So the return type for those is covered because the typed targets contain the
NSM handler? Maybe update the comment to make that clearer.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3371:
compiler.enqueuer.codegen.registerSelectorUse(selector.asUntyped);
Do we have to do this at all if invokeOn isn't registered?

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:1546: bool
canReachAll = compiler.enabledInvokeOn
Could we move this code to the selector?

[ngeoffray, 2013-10-29 09:19:27]

Thanks Kasper.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
File
sdk/lib/_internal/compiler/implementation/inferrer/simple_types_inferrer.dart
(right):

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/simple_types_inferrer.dart:796:
} if (node.isPropertyAccess
On 2013/10/29 08:35:27, kasperl wrote:
> if -> else if

Done.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:488:
bool canReachAll = compiler.enabledInvokeOn
On 2013/10/29 08:35:27, kasperl wrote:
> Move this code to the selector? Maybe it could be something like:
> 
>   Selector selectorToUse = typedSelector.computeEffectiveSelector(compiler);
> 
> That way you don't have to compute the mask either.

Done.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:492: //
the untyped selector (through noSuchMethod's `InvocationMirror`
On 2013/10/29 08:35:27, kasperl wrote:
> InvocationMirror -> Invocation.

Done.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:492: //
the untyped selector (through noSuchMethod's `InvocationMirror`
On 2013/10/29 08:35:27, kasperl wrote:
> InvocationMirror -> Invocation.

Done.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:516: //
`InvocationMirror.delegate`.
On 2013/10/29 08:35:27, kasperl wrote:
> InvocationMirror -> Invocation.

Done.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:516: //
`InvocationMirror.delegate`.
On 2013/10/29 08:35:27, kasperl wrote:
> So the return type for those is covered because the typed targets contain the
> NSM handler? Maybe update the comment to make that clearer.

Done.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3371:
compiler.enqueuer.codegen.registerSelectorUse(selector.asUntyped);
On 2013/10/29 08:35:27, kasperl wrote:
> Do we have to do this at all if invokeOn isn't registered?

No, nice catch. Done.

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/49813002/diff/130001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:1546: bool
canReachAll = compiler.enabledInvokeOn
On 2013/10/29 08:35:27, kasperl wrote:
> Could we move this code to the selector?

Done.

[ngeoffray, 2013-10-29 10:22:13]

Committed patchset #3 manually as r29431 (presubmit successful).