Handle parameters in compile_from_dill_test

Handle parameters in compile_from_dill_test

R=efortuna@google.com, sigmund@google.com

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

[Johnni Winther, 2017-06-07 12:13:04]

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

[Emily Fortuna, 2017-06-07 18:16:42]

efortuna@google.com changed reviewers:
+ efortuna@google.com

[Emily Fortuna, 2017-06-07 18:16:43]

https://codereview.chromium.org/2926863002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/locals_handler.dart (left):

https://codereview.chromium.org/2926863002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/locals_handler.dart:207: if (element is MethodElement)
{
perhaps I'm missing something here -- why do we want to move this code out of
here into SSA where it gets replicated twice (in builder and then
builder_kernel.dart)

[Johnni Winther, 2017-06-07 18:34:45]

https://codereview.chromium.org/2926863002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/locals_handler.dart (left):

https://codereview.chromium.org/2926863002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/locals_handler.dart:207: if (element is MethodElement)
{
On 2017/06/07 18:16:42, Emily Fortuna wrote:
> perhaps I'm missing something here -- why do we want to move this code out of
> here into SSA where it gets replicated twice (in builder and then
> builder_kernel.dart)

If we move this (and other pieces) out of the locals handler it doesn't even
need to know about nodes (ast or ir) or signatures. 

Also this is one of these places were (it already looks like) temporarily
duplicating the code will show how it mixes concerns (like with the
Resolution/CodegenEnqueuer split)

[Emily Fortuna, 2017-06-07 19:21:40]

https://codereview.chromium.org/2926863002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/locals_handler.dart (left):

https://codereview.chromium.org/2926863002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/locals_handler.dart:207: if (element is MethodElement)
{
On 2017/06/07 18:34:45, Johnni Winther wrote:
> On 2017/06/07 18:16:42, Emily Fortuna wrote:
> > perhaps I'm missing something here -- why do we want to move this code out
of
> > here into SSA where it gets replicated twice (in builder and then
> > builder_kernel.dart)
> 
> If we move this (and other pieces) out of the locals handler it doesn't even
> need to know about nodes (ast or ir) or signatures. 
> 
> Also this is one of these places were (it already looks like) temporarily
> duplicating the code will show how it mixes concerns (like with the
> Resolution/CodegenEnqueuer split)

Got it! Thanks for the clarification!

[Emily Fortuna, 2017-06-07 19:21:46]

lgtm

[Johnni Winther, 2017-06-07 19:43:20]

PTAL @ the last patch

[Siggi Cherem (dart-lang), 2017-06-07 22:29:48]

lgtm

[Johnni Winther, 2017-06-08 07:20:59]

Description was changed from

==========
Handle parameters in compile_from_dill_test
==========

to

==========
Handle parameters in compile_from_dill_test

R=efortuna@google.com, sigmund@google.com

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

[Johnni Winther, 2017-06-08 07:21:01]

Committed patchset #3 (id:40001) manually as
412f2cff8d333f3b265094ff95871032fa6232f9 (presubmit successful).