Handle for-loop with simple break in compile_from_dill_test

Handle for-loop with simple break in compile_from_dill_test

R=sigmund@google.com

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

[Johnni Winther, 2017-06-22 11:55:49]

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

[Johnni Winther, 2017-06-23 12:25:38]

PTAL

[Siggi Cherem (dart-lang), 2017-06-23 19:34:00]

lgtm after comment re-inlining is addressed

https://codereview.chromium.org/2950303002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2950303002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:292: // Find the root
node for the current member.
I know this will go away when you do it eagerly, but any reason why not fetch
the root node from `currentMember` (I guess that will require having access to
that global member-ir map here)?

https://codereview.chromium.org/2950303002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:314:
_members.removeLast();
do we need to clear the _jumpTargetMap when we enter/leave? or have a stack of
them (like we have a stack of members)?

[Johnni Winther, 2017-06-26 09:04:18]

https://codereview.chromium.org/2950303002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2950303002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:292: // Find the root
node for the current member.
On 2017/06/23 19:34:00, Siggi Cherem (dart-lang) wrote:
> I know this will go away when you do it eagerly, but any reason why not fetch
> the root node from `currentMember` (I guess that will require having access to
> that global member-ir map here)?

Your guess is right.

https://codereview.chromium.org/2950303002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:314:
_members.removeLast();
On 2017/06/23 19:34:00, Siggi Cherem (dart-lang) wrote:
> do we need to clear the _jumpTargetMap when we enter/leave? or have a stack of
> them (like we have a stack of members)?

Yes, something like it. I think we need to split inlined member from
KernelToLocalsMap.

[Johnni Winther, 2017-06-26 09:10:36]

Description was changed from

==========
Handle for-loop with simple break in compile_from_dill_test
==========

to

==========
Handle for-loop with simple break in compile_from_dill_test

R=sigmund@google.com

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

[Johnni Winther, 2017-06-26 09:10:37]

Committed patchset #1 (id:1) manually as
14dda164821f06d82d9a4537d0d025d735f044f1 (presubmit successful).

[Siggi Cherem (dart-lang), 2017-06-26 16:44:14]

thanks

https://codereview.chromium.org/2950303002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2950303002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:314:
_members.removeLast();
On 2017/06/26 09:04:18, Johnni Winther wrote:
> On 2017/06/23 19:34:00, Siggi Cherem (dart-lang) wrote:
> > do we need to clear the _jumpTargetMap when we enter/leave? or have a stack
of
> > them (like we have a stack of members)?
> 
> Yes, something like it. I think we need to split inlined member from
> KernelToLocalsMap.

Can you add a note about it here? (maybe TODO(redemption)) - just want to make
sure we don't accidentally hit this when we start implementing inlining and
forget about it.