Fix bug in SsaConstructionFieldTypes: handle mulitple assignments to the same field.

Fix bug in SsaConstructionFieldTypes: handle mulitple assignments to the same field.

R=kasperl@google.com

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

[ngeoffray, 2013-06-06 07:38:12]

This could be one of the reason for bug
https://code.google.com/p/dart/issues/detail?id=11117. I'm investigating another
one.

[kasperl, 2013-06-06 08:15:12]

https://codereview.chromium.org/15725018/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/optimize.dart (right):

https://codereview.chromium.org/15725018/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/optimize.dart:1482:
currentFieldSetters.remove(field);
Can we then get another field type added after this? Should we add a marker
instead of removing it? Need a test with three field sets?

https://codereview.chromium.org/15725018/diff/1/tests/language/multiple_field...
File tests/language/multiple_field_assignment_constructor_test.dart (right):

https://codereview.chromium.org/15725018/diff/1/tests/language/multiple_field...
tests/language/multiple_field_assignment_constructor_test.dart:7: class NoInline
{
DontInline?

https://codereview.chromium.org/15725018/diff/1/tests/language/multiple_field...
tests/language/multiple_field_assignment_constructor_test.dart:17: @NoInline
What's the purpose of this annotation? Are you experimenting with adding a
proper way of disabling inlining for certain elements?

[ngeoffray, 2013-06-06 08:44:00]

https://codereview.chromium.org/15725018/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/optimize.dart (right):

https://codereview.chromium.org/15725018/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/optimize.dart:1482:
currentFieldSetters.remove(field);
On 2013/06/06 08:15:12, kasperl wrote:
> Can we then get another field type added after this? Should we add a marker
> instead of removing it? Need a test with three field sets?

Good catch. Done.

https://codereview.chromium.org/15725018/diff/1/tests/language/multiple_field...
File tests/language/multiple_field_assignment_constructor_test.dart (right):

https://codereview.chromium.org/15725018/diff/1/tests/language/multiple_field...
tests/language/multiple_field_assignment_constructor_test.dart:7: class NoInline
{
On 2013/06/06 08:15:12, kasperl wrote:
> DontInline?

Done.

https://codereview.chromium.org/15725018/diff/1/tests/language/multiple_field...
tests/language/multiple_field_assignment_constructor_test.dart:17: @NoInline
On 2013/06/06 08:15:12, kasperl wrote:
> What's the purpose of this annotation? Are you experimenting with adding a
> proper way of disabling inlining for certain elements?

Yeah, more like a marker that this test depends on not inlining some elements.
So the test does not get lost when we finally add an annotation.

WDYT about adding it to a library, trying to use it more when writing tests and
at some point implementing it?

[kasperl, 2013-06-06 08:45:30]

LGTM!

https://codereview.chromium.org/15725018/diff/1/tests/language/multiple_field...
File tests/language/multiple_field_assignment_constructor_test.dart (right):

https://codereview.chromium.org/15725018/diff/1/tests/language/multiple_field...
tests/language/multiple_field_assignment_constructor_test.dart:17: @NoInline
On 2013/06/06 08:44:00, ngeoffray wrote:
> On 2013/06/06 08:15:12, kasperl wrote:
> > What's the purpose of this annotation? Are you experimenting with adding a
> > proper way of disabling inlining for certain elements?
> 
> Yeah, more like a marker that this test depends on not inlining some elements.
> So the test does not get lost when we finally add an annotation.
> 
> WDYT about adding it to a library, trying to use it more when writing tests
and
> at some point implementing it?

SGTM!

[ngeoffray, 2013-06-06 08:54:06]

Committed patchset #4 manually as r23665 (presubmit successful).