Distinguish between actual closure scopes and non-closure scopes.

Distinguish between actual closure scopes and non-closure scopes.


BUG=
R=johnniwinther@google.com

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

[Emily Fortuna, 2017-07-12 23:37:15]

Patchset #1 (id:1) has been deleted

[Emily Fortuna, 2017-07-12 23:39:55]

efortuna@google.com changed reviewers:
+ johnniwinther@google.com, sra@google.com

[Emily Fortuna, 2017-07-12 23:40:02]

Patchset #1 (id:20001) has been deleted

[sra1, 2017-07-12 23:57:42]

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_model/closure.dart (right):

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure.dart:38: /// [_closureScopeMap].
Say why are they separate. Is it so these can be GC-ed after the closure task
but the others cannot?

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure.dart:40: Map<ir.Node, ClosureScope>
_closureScopeMap = <ir.Node, ClosureScope>{};
Can this be Entity too?

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_model/closure_visitors.dart (right):

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure_visitors.dart:28: Map<Entity, ScopeInfo>
_scopeInfoMap = <Entity, ScopeInfo>{};
final, don't initialize.

[Johnni Winther, 2017-07-13 08:56:14]

lgtm

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_model/closure_visitors.dart (right):

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure_visitors.dart:231: entity =
_kernelToElementMap.getMember(tempNode);
Maybe the visitor should have the member entity as a final field instead. You
always know the member when you create the visitor (in _buildClosureModel)

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure_visitors.dart:233: entity =
_kernelToElementMap.getLocalFunction(tempNode);
Can this ever happen? We always start visiting on an ir.Member and only visit
local functions inside of the member.

[Emily Fortuna, 2017-07-13 21:03:39]

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_model/closure.dart (right):

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure.dart:38: /// [_closureScopeMap].
On 2017/07/12 23:57:41, sra1 wrote:
> Say why are they separate. Is it so these can be GC-ed after the closure task
> but the others cannot?

see comment below.

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure.dart:40: Map<ir.Node, ClosureScope>
_closureScopeMap = <ir.Node, ClosureScope>{};
On 2017/07/12 23:57:41, sra1 wrote:
> Can this be Entity too?

yes, ended up combining with the above, since these don't always get garbage
collected.

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_model/closure_visitors.dart (right):

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure_visitors.dart:28: Map<Entity, ScopeInfo>
_scopeInfoMap = <Entity, ScopeInfo>{};
On 2017/07/12 23:57:41, sra1 wrote:
> final, don't initialize.

Done.

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure_visitors.dart:231: entity =
_kernelToElementMap.getMember(tempNode);
On 2017/07/13 08:56:14, Johnni Winther wrote:
> Maybe the visitor should have the member entity as a final field instead. You
> always know the member when you create the visitor (in _buildClosureModel)

done.

https://codereview.chromium.org/2981723002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_model/closure_visitors.dart:233: entity =
_kernelToElementMap.getLocalFunction(tempNode);
On 2017/07/13 08:56:14, Johnni Winther wrote:
> Can this ever happen? We always start visiting on an ir.Member and only visit
> local functions inside of the member.

You're right it won't. Took it out.

[Emily Fortuna, 2017-07-13 21:06:07]

Description was changed from

==========
Distinguish between actual closure scopes and non-closure scopes.


BUG=
==========

to

==========
Distinguish between actual closure scopes and non-closure scopes.


BUG=
R=johnniwinther@google.com

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

[Emily Fortuna, 2017-07-13 21:06:08]

Committed patchset #2 (id:60001) manually as
6264923aa1308351ab409ead46a749f3c835c3c6 (presubmit successful).