Kernel: Do not try to use return value of []=

Kernel: Do not try to use return value of []=

Fixes #28302

R=ahe@google.com, vegorov@google.com

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

[kustermann, 2017-02-01 13:55:40]

kustermann@google.com changed reviewers:
+ ahe@google.com, vegorov@google.com

[Vyacheslav Egorov (Google), 2017-02-01 14:00:47]

LGTM

[ahe, 2017-02-01 14:02:39]

https://codereview.chromium.org/2668893003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/frontend_accessors.dart (right):

https://codereview.chromium.org/2668893003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/frontend_accessors.dart:30: _makeWrite(value,
false), new NullLiteral(), type));
I think this hits more cases than you expect, so I'm not sure this is the right
fix.

Try running:

dart pkg/testing/bin/testing.dart --config=pkg/front_end/test/fasta/testing.json
compile

I think you'll see that you're changing some expectation files.

Do note that I haven't automated running these tests yet, and they're not up to
date, so you need to run the tests with and without your changes.

[kustermann, 2017-02-01 14:23:42]

https://codereview.chromium.org/2668893003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/frontend_accessors.dart (right):

https://codereview.chromium.org/2668893003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/frontend_accessors.dart:30: _makeWrite(value,
false), new NullLiteral(), type));
> I think this hits more cases than you expect, so I'm not sure this is the
right
> fix.

The result of _makeWrite() is used, namely in a ConditionalExpression. So it
cannot be a void context. The VM will crash, because the _makeWrite() results in
caling []=() and the return value *is used*.

So why is this not a correct change?

There might be more such things, but I'm not claiming that I found all of them.


> Try running:
> 
> dart pkg/testing/bin/testing.dart
--config=pkg/front_end/test/fasta/testing.json
> compile


The same tests fail with and without this CL, namely:


12 failed:
compile/parser/parser_suite: Fail
compile/rasta/bad_constructor_redirection: Pass
compile/rasta/bad_explicit_super_constructor: Pass
compile/rasta/for_loop: Pass
compile/rasta/issue_000053: Pass
compile/rasta/issue_000068: Pass
compile/rasta/issue_000070: Pass
compile/rasta/malformed_function_type: Pass
compile/rasta/switch_execution_case_t02: Pass
compile/rasta/switch_fall_through: Pass
compile/rasta/try_label: Pass
compile/scanner/scanner_suite: Fail


Though it's quite difficult finding out if they fail the same way, because the
output of running this command is *huge*.

[ahe, 2017-02-01 14:33:16]

lgtm

https://codereview.chromium.org/2668893003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/frontend_accessors.dart (right):

https://codereview.chromium.org/2668893003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/frontend_accessors.dart:30: _makeWrite(value,
false), new NullLiteral(), type));
On 2017/02/01 14:23:42, kustermann wrote:
> So why is this not a correct change?

I was wrong. Thank's for clarifying.

> Though it's quite difficult finding out if they fail the same way, because the
> output of running this command is *huge*.

Yes. I'm working on that right now.

[kustermann, 2017-02-01 16:46:48]

Thanks, Peter.

[kustermann, 2017-02-01 16:47:24]

Description was changed from

==========
Kernel: Do not try to use return value of []=

Fixes #28302
==========

to

==========
Kernel: Do not try to use return value of []=

Fixes #28302

R=ahe@google.com, vegorov@google.com

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

[kustermann, 2017-02-01 16:47:25]

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