Remove BlockExpression from the Kernel language.

Remove BlockExpression from the Kernel language.

BlockExpression allowed statements to appear in an expression context.
It is unsafe for its original use case, allowing 'yield' to appear as an
expression, because the VM's yield implementation does not work when
there are live expression intermediate values.

Eliminate it by changing the async ExpressionLifter to return a
transformed expression and emit statements into a given list as a side
effect.  This requires implementing the transformation for all
statements to account for how they may contain a nested expression.

BUG=
R=asgerf@google.com

Committed: https://github.com/dart-lang/kernel/commit/698af311ca03ca9691e593bbcfca47cb1c69936d

[Kevin Millikin (Google), 2016-10-31 12:29:31]

kmillikin@google.com changed reviewers:
+ asgerf@google.com, vegorov@google.com

[Kevin Millikin (Google), 2016-10-31 12:29:32]

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/async.dart
File lib/transformations/async.dart (right):

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/async.d...
lib/transformations/async.dart:427: arguments.positional[0] =
expr.operand.accept(this)..parent = arguments;
This was a bug: the translated operand should occur before the continuation
point, so it must be translated after because the statements are accumulated in
reverse order.

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
File lib/transformations/continuation.dart (right):

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:37: node.transformChildren(new
RecursiveContinuationRewriter(helper));
super.visitFunctionNode(node) is node.transformChildren(this) which isn't what
we want (it will try to translate a nested synchronous function using the wrong
rewriter).

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:286: return null;
The statement translation functions always return null and add statements to the
current list as a side effect.  It is weird to make it a transformation instead
of just a visit.  I'll clean that up later.

[asgerf, 2016-11-01 10:44:45]

LGTM!

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
File lib/transformations/continuation.dart (right):

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:284: TreeNode
visitInvalidStatement(InvalidStatemnt stmt) {
InvalidStatemnt -> InvalidStatement

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:405: for (int i = 0; i <
stmt.updates.length; ++i) {
Did you mean to use "i < length" here? (since we have the length variable)

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:431: stmt.condition = null;  // No
condition in a for loop is the same as true.
Long line.

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:483: List<Statment> updates =
<Statement>[];
Statment -> Statement

[Kevin Millikin (Google), 2016-11-01 13:11:31]

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
File lib/transformations/continuation.dart (right):

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:284: TreeNode
visitInvalidStatement(InvalidStatemnt stmt) {
On 2016/11/01 10:44:45, asgerf wrote:
> InvalidStatemnt -> InvalidStatement

Done.

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:405: for (int i = 0; i <
stmt.updates.length; ++i) {
On 2016/11/01 10:44:45, asgerf wrote:
> Did you mean to use "i < length" here? (since we have the length variable)

Yes.  Done.

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:431: stmt.condition = null;  // No
condition in a for loop is the same as true.
On 2016/11/01 10:44:45, asgerf wrote:
> Long line.

I just ran the whole file through the formatter.

https://codereview.chromium.org/2460373002/diff/1/lib/transformations/continu...
lib/transformations/continuation.dart:483: List<Statment> updates =
<Statement>[];
On 2016/11/01 10:44:45, asgerf wrote:
> Statment -> Statement

Done.

[Kevin Millikin (Google), 2016-11-01 13:13:34]

Description was changed from

==========
Remove BlockExpression from the Kernel language.

BlockExpression allowed statements to appear in an expression context.
It is unsafe for its original use case, allowing 'yield' to appear as an
expression, because the VM's yield implementation does not work when
there are live expression intermediate values.

Eliminate it by changing the async ExpressionLifter to return a
transformed expression and emit statements into a given list as a side
effect.  This requires implementing the transformation for all
statements to account for how they may contain a nested expression.

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

to

==========
Remove BlockExpression from the Kernel language.

BlockExpression allowed statements to appear in an expression context.
It is unsafe for its original use case, allowing 'yield' to appear as an
expression, because the VM's yield implementation does not work when
there are live expression intermediate values.

Eliminate it by changing the async ExpressionLifter to return a
transformed expression and emit statements into a given list as a side
effect.  This requires implementing the transformation for all
statements to account for how they may contain a nested expression.

BUG=
R=asgerf@google.com

Committed:
https://github.com/dart-lang/kernel/commit/698af311ca03ca9691e593bbcfca47cb1c...
==========

[Kevin Millikin (Google), 2016-11-01 13:13:35]

Committed patchset #2 (id:20001) manually as
698af311ca03ca9691e593bbcfca47cb1c69936d (presubmit successful).