Add support for fields to the new dart backend.

Add support for fields to the new dart backend.

Field initializers are represented similarly to a function with no
parameters.
When they are emitted in backend_ast_emitter.dart `ensureExpression`
wraps the code in an immediately invoked closure if it is not on the
simple form `return e;`.
Ideally this should never happen, but currently happens for cases like eg.
class A {
  static var b = 1;
  var a = b++;
}

main() {
  var a = new A();
  print(a.a);
}

Gets converted to:

class A {
  static var b = 1;
  var a = (() /* new backend */
    {
      var v0;
      v0 = A.b;
      A.b = v0 + 1;
      return v0;
    }
  )();
}

main() /* new backend */
  {
    print(new A().a);
  }

R=kmillikin@google.com

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

[sigurdm, 2014-11-26 12:48:44]

sigurdm@google.com changed reviewers:
+ kmillikin@google.com

[Kevin Millikin (Google), 2014-11-26 13:31:30]

LGTM with some indentation suggestions.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart (right):

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart:153:
definitions.definitions.nodes.first;
Indent.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart:159: if (fieldDefinition
is! ast.SendSet) return null;
Is this the case that there is no initializer?  It needs a comment here, and a
comment for the function describing the conditions which will make it return
null.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart:161: new
DetectClosureVariables(elements);
Indent.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart:168: builder, () {
This needs a 4-space indent.  I prefer

return withBuilder(builder, () {
  // body
});

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
File pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart (right):

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart:555: class FieldDefinition extends
Node
Better:

class FieldDefinition extends Node
    implements InteriorNode, ExecutableDefinition {

In any case, it should be consistent with FunctionDefinition just below.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart:590: acceptPass(Pass pass) =>
pass.rewriteFunctionDefinition(this);
applyPass?

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
File pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart (right):

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart:35: names = new Names();
Isn't this the same as the implementation of visitFunctionDefinition?  I don't
care too much for this class, but it might be nice to share the code.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart:338:
visitFieldDefinition(cps_ir.FieldDefinition f) {
Same here: you could share the code but I don't care too much one way or the
other :)

[sigurdm, 2014-11-26 14:38:44]

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart (right):

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart:153:
definitions.definitions.nodes.first;
On 2014/11/26 13:31:29, kmillikin wrote:
> Indent.

Done.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart:159: if (fieldDefinition
is! ast.SendSet) return null;
On 2014/11/26 13:31:29, kmillikin wrote:
> Is this the case that there is no initializer?  It needs a comment here, and a
> comment for the function describing the conditions which will make it return
> null.

Done.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart:161: new
DetectClosureVariables(elements);
On 2014/11/26 13:31:29, kmillikin wrote:
> Indent.

Done.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_visitor.dart:168: builder, () {
On 2014/11/26 13:31:29, kmillikin wrote:
> This needs a 4-space indent.  I prefer
> 
> return withBuilder(builder, () {
>   // body
> });

Done.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
File pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart (right):

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart:555: class FieldDefinition extends
Node
On 2014/11/26 13:31:30, kmillikin wrote:
> Better:
> 
> class FieldDefinition extends Node
>     implements InteriorNode, ExecutableDefinition {
> 
> In any case, it should be consistent with FunctionDefinition just below.

Done.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart:590: acceptPass(Pass pass) =>
pass.rewriteFunctionDefinition(this);
On 2014/11/26 13:31:29, kmillikin wrote:
> applyPass?

Done.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
File pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart (right):

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart:35: names = new Names();
On 2014/11/26 13:31:30, kmillikin wrote:
> Isn't this the same as the implementation of visitFunctionDefinition?  I don't
> care too much for this class, but it might be nice to share the code.

Done.

https://codereview.chromium.org/759193005/diff/20001/pkg/compiler/lib/src/cps...
pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart:338:
visitFieldDefinition(cps_ir.FieldDefinition f) {
On 2014/11/26 13:31:30, kmillikin wrote:
> Same here: you could share the code but I don't care too much one way or the
> other :)

Done.

[sigurdm, 2014-11-28 08:49:10]

Committed patchset #4 (id:60001) manually as 42024 (presubmit successful).