Introduce FieldParameterElement.

Introduce FieldParameterElement.

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

[ngeoffray, 2012-03-15 10:06:01]

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/elements/el...
File frog/leg/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/elements/el...
frog/leg/elements/elements.dart:369: [Node node])
I believe the node should not be optional here.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1431: ElementKind.VARIABLE_LIST, enclosingElement);
Maybe add a helper method for creating the variables element. It is now being
created 3 times: lines 1430, 1455 and 1468.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1436: FieldParameterElement visitSend(Send node) {
Please add a comment that the only valid Send you could find in a parameter list
is a field parameter: this.x.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1456: ElementKind.VARIABLE_LIST, enclosingElement);
4 space

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1458: fieldElement, variables, enclosingElement, node:
node);
ditto

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1469: ElementKind.VARIABLE_LIST, enclosingElement);
Why do you need to create a new element? fieldParameterElement looks good enough
here.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:283: // locals.
It looks like you should have an if/else here:
if (element.kind == PARAMETER) {
} else {
  // field parameter.
}

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:802: localsHandler.updateLocal(parameter, argument);
ditto for the if/else. The field parameter element should not be referenced.

[floitsch, 2012-03-15 14:11:04]

PTAL.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/elements/el...
File frog/leg/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/elements/el...
frog/leg/elements/elements.dart:369: [Node node])
On 2012/03/15 10:06:01, ngeoffray wrote:
> I believe the node should not be optional here.

Done.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1431: ElementKind.VARIABLE_LIST, enclosingElement);
On 2012/03/15 10:06:01, ngeoffray wrote:
> Maybe add a helper method for creating the variables element. It is now being
> created 3 times: lines 1430, 1455 and 1468.

Only created twice now, and once as a VariableElement and once as a
FieldParameterElement. Code share would be minimal.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1436: FieldParameterElement visitSend(Send node) {
On 2012/03/15 10:06:01, ngeoffray wrote:
> Please add a comment that the only valid Send you could find in a parameter
list
> is a field parameter: this.x.

Done.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1456: ElementKind.VARIABLE_LIST, enclosingElement);
On 2012/03/15 10:06:01, ngeoffray wrote:
> 4 space

Done.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1458: fieldElement, variables, enclosingElement, node:
node);
On 2012/03/15 10:06:01, ngeoffray wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/resolver.da...
frog/leg/resolver.dart:1469: ElementKind.VARIABLE_LIST, enclosingElement);
On 2012/03/15 10:06:01, ngeoffray wrote:
> Why do you need to create a new element? fieldParameterElement looks good
enough
> here.

Done.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:283: // locals.
On 2012/03/15 10:06:01, ngeoffray wrote:
> It looks like you should have an if/else here:
> if (element.kind == PARAMETER) {
> } else {
>   // field parameter.
> }

Done.

https://chromiumcodereview.appspot.com/9699033/diff/1016/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:802: localsHandler.updateLocal(parameter, argument);
On 2012/03/15 10:06:01, ngeoffray wrote:
> ditto for the if/else. The field parameter element should not be referenced.

Done.

[ngeoffray, 2012-03-15 15:16:24]

LGTM!

https://chromiumcodereview.appspot.com/9699033/diff/3014/frog/leg/compile_tim...
File frog/leg/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/9699033/diff/3014/frog/leg/compile_tim...
frog/leg/compile_time_constants.dart:924: Map<Element, Constant>
constructorDefinitions,
constructorDefinitions -> parameterValues ?

https://chromiumcodereview.appspot.com/9699033/diff/3014/frog/leg/compile_tim...
frog/leg/compile_time_constants.dart:1005: constructorDefinitions, fieldValues);
This is all happening in the same function, so you should not need to pass them
all as parameters.

[floitsch, 2012-03-15 15:37:24]

https://chromiumcodereview.appspot.com/9699033/diff/3014/frog/leg/compile_tim...
File frog/leg/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/9699033/diff/3014/frog/leg/compile_tim...
frog/leg/compile_time_constants.dart:924: Map<Element, Constant>
constructorDefinitions,
On 2012/03/15 15:16:25, ngeoffray wrote:
> constructorDefinitions -> parameterValues ?

I want to rename some elements in this file anyways. I will see if this makes
sense then.

https://chromiumcodereview.appspot.com/9699033/diff/3014/frog/leg/compile_tim...
frog/leg/compile_time_constants.dart:1005: constructorDefinitions, fieldValues);
On 2012/03/15 15:16:25, ngeoffray wrote:
> This is all happening in the same function, so you should not need to pass
them
> all as parameters.

I think I will need separate arguments when implementing super calls. Maybe I'm
wrong in which case I will change it then.