Always store GlobalTypeInferenceElementData on the member context

Always store GlobalTypeInferenceElementData on the member context

R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/63f74d855f3ea59a97381880bf05d716b7955909

[Johnni Winther, 2017-06-29 16:51:10]

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

[Johnni Winther, 2017-06-29 17:00:28]

https://codereview.chromium.org/2961313002/diff/1/pkg/compiler/lib/src/inferr...
File pkg/compiler/lib/src/inferrer/inferrer_engine.dart (left):

https://codereview.chromium.org/2961313002/diff/1/pkg/compiler/lib/src/inferr...
pkg/compiler/lib/src/inferrer/inferrer_engine.dart:121:
GlobalTypeInferenceElementData _dataOf(AstElement element) => inTreeData
It turned out that we never (after type inference) read the data stored on a
local function. 

After investigating this, it turned out that the addition of [_dataOf] in
https://codereview.chromium.org/2468303003 actually unintentionally introduced
this (local functions share the TreeElements with their member context).

Surprisingly, this didn't show any regression back then (as far as i know) -
though I suspect the inference might have become slower; taking more iterations
to reach the same fixed-point by starting from scratch instead of using the data
collected in the local function data.

[Siggi Cherem (dart-lang), 2017-06-29 21:40:09]

lgtm, just a question at the very end...

https://codereview.chromium.org/2961313002/diff/1/pkg/compiler/lib/src/inferr...
File pkg/compiler/lib/src/inferrer/builder.dart (right):

https://codereview.chromium.org/2961313002/diff/1/pkg/compiler/lib/src/inferr...
pkg/compiler/lib/src/inferrer/builder.dart:78: final
GlobalTypeInferenceElementData inTreeData;
rename to memberData like you did for the map in the other file? or simply
`data`? (since the builder is per-member already?)

https://codereview.chromium.org/2961313002/diff/1/tests/compiler/dart2js/infe...
File tests/compiler/dart2js/inference/inference_test_helper.dart (right):

https://codereview.chromium.org/2961313002/diff/1/tests/compiler/dart2js/infe...
tests/compiler/dart2js/inference/inference_test_helper.dart:48: elementResult =
results.resultOfMember(localFunction.memberContext);
memberContext or callMethod?

[Johnni Winther, 2017-06-30 08:13:46]

Description was changed from

==========
Always store GlobalTypeInferenceElementData on the member context
==========

to

==========
Always store GlobalTypeInferenceElementData on the member context

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/63f74d855f3ea59a97381880bf05d716b7955909
==========

[Johnni Winther, 2017-06-30 08:13:48]

Committed patchset #2 (id:20001) manually as
63f74d855f3ea59a97381880bf05d716b7955909 (presubmit successful).

[Johnni Winther, 2017-06-30 08:14:50]

https://codereview.chromium.org/2961313002/diff/1/pkg/compiler/lib/src/inferr...
File pkg/compiler/lib/src/inferrer/builder.dart (right):

https://codereview.chromium.org/2961313002/diff/1/pkg/compiler/lib/src/inferr...
pkg/compiler/lib/src/inferrer/builder.dart:78: final
GlobalTypeInferenceElementData inTreeData;
On 2017/06/29 21:40:09, Siggi Cherem (dart-lang) wrote:
> rename to memberData like you did for the map in the other file? or simply
> `data`? (since the builder is per-member already?)

Renamed to [memberData]. The builder is created for members _and_ local
functions.

https://codereview.chromium.org/2961313002/diff/1/tests/compiler/dart2js/infe...
File tests/compiler/dart2js/inference/inference_test_helper.dart (right):

https://codereview.chromium.org/2961313002/diff/1/tests/compiler/dart2js/infe...
tests/compiler/dart2js/inference/inference_test_helper.dart:48: elementResult =
results.resultOfMember(localFunction.memberContext);
On 2017/06/29 21:40:09, Siggi Cherem (dart-lang) wrote:
> memberContext or callMethod?

They amount to the same but like using .callMethod to align it with to kernel
based solution (when it arrives)