Element-model refactoring.

Element-model refactoring.

Introduces ExecutableElement, MemberElement (with subclass MethodElement) and LocalElement (with subclasses LocalVariableElement, LocalParameterElement, and LocalFunctionElement).

Look in elements/elements.dart for main changes.

BUG=
R=floitsch@google.com

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

[Johnni Winther, 2014-07-15 11:43:02]

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (left):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:808: abstract
class ClosureContainer implements Element {
Renamed to [MemberElement] and moved down to [ExecutableElement] and
[LocalElement].

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:938: abstract
class FieldParameterElement extends ParameterElement {
Renamed to [InitializingFormalElement] to match spec terminology.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:980: abstract
class FormalElement extends Element
Other descarded name candidates: 'ParameterishElement', 'ParameteroidElement',
'FunctionSignatureElement'.

[floitsch, 2014-07-15 14:19:39]

LGTM with a few questions/comments.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/deferred_load.dart (right):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:34: MemberElement,
Needed?

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (left):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:808: abstract
class ClosureContainer implements Element {
On 2014/07/15 11:43:01, Johnni Winther wrote:
> Renamed to [MemberElement] and moved down to [ExecutableElement] and
> [LocalElement].

Acknowledged.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:294: /// `true`
is the element is a member of a class, that is a static or instance
s/is/if.

reads too much like "class that is ...".
In this case "i.e." might work better.

Alternatively:
`true` if the element is a (static or instance) member of a class.

Members are constructors, methods and fields.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:298: /// `true`
is the element is a nonstatic member of a class, that is a
ditto.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:903: /// The
type defined by this typedef with the type variables as its type
New line before.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:926: /// The
outermost member that contains this element.
When is this not `this`?

I assume only nested functions/closures have memberContext != this.

If yes, state it.
If there are other cases, provide examples.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:930: /// A
top-level or static field or method, or a constructor.
No instance members?

Does that mean that a MemberElement might return false for both `isClassMember`
and `isInstanceMember`? (for a top-level function).

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:933: ///
context.
How does "ExecutableElement" work with "FieldElement" ?

The ExecutableElement has comment "is an element that holds code". But fields
don't always hold code.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:948: //
TODO(johnniwinther): Should [Local] have `isAssignable` or `type`?
Nit: Move TODO below dartdoc.
This way the important first line is on top.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:951: ///
Several [Element] can define local variable, for instance parameter, local
Several [Element]s can define a local variable and thus implement "Local".
Examples of locals are instance parameters, local variables, and local
functions. [[Are there others?. If not many, list them all.]]
For non-element, ...


It feels strange to say "can define a local variable" and then list "local
variables" as examples.
Replace "can define a local variable" with "can define a local entity" ?

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:961: /// The
exetuable context is the [ExecutableElement] in which this variable
executable

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:973: /// If the
function signature comes from a typedef or an inline function-typed
what's an inline function-typed parameter?

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:974: ///
parameter, the its parameters are not real parameters in that they can take
... then its ...

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:980: abstract
class FormalElement extends Element
On 2014/07/15 11:43:01, Johnni Winther wrote:
> Other descarded name candidates: 'ParameterishElement', 'ParameteroidElement',
> 'FunctionSignatureElement'.

I would be ok with ParameterishElement

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:981: implements
FunctionTypedElement, TypedElement, AstElement {
indent +2

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1385: *
[:A(this.field):].
nit: `A(this.field)` and ///

[Johnni Winther, 2014-07-18 10:31:50]

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/deferred_load.dart (right):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:34: MemberElement,
On 2014/07/15 14:19:38, floitsch wrote:
> Needed?

Not yet.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:294: /// `true`
is the element is a member of a class, that is a static or instance
On 2014/07/15 14:19:38, floitsch wrote:
> s/is/if.
> 
> reads too much like "class that is ...".
> In this case "i.e." might work better.
> 
> Alternatively:
> `true` if the element is a (static or instance) member of a class.
> 
> Members are constructors, methods and fields.

Done.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:298: /// `true`
is the element is a nonstatic member of a class, that is a
On 2014/07/15 14:19:38, floitsch wrote:
> ditto.

Done. Constructors are not instance members!

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:903: /// The
type defined by this typedef with the type variables as its type
On 2014/07/15 14:19:38, floitsch wrote:
> New line before.

Done.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:933: ///
context.
On 2014/07/15 14:19:38, floitsch wrote:
> How does "ExecutableElement" work with "FieldElement" ?
> 
> The ExecutableElement has comment "is an element that holds code". But fields
> don't always hold code.

Updated the comment on [ExecutableElement] to state that variables (can) hold
code through their initializer.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:951: ///
Several [Element] can define local variable, for instance parameter, local
On 2014/07/15 14:19:38, floitsch wrote:
> Several [Element]s can define a local variable and thus implement "Local".
> Examples of locals are instance parameters, local variables, and local
> functions. [[Are there others?. If not many, list them all.]]
> For non-element, ...
> 
> 
> It feels strange to say "can define a local variable" and then list "local
> variables" as examples.
> Replace "can define a local variable" with "can define a local entity" ?

Done.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:961: /// The
exetuable context is the [ExecutableElement] in which this variable
On 2014/07/15 14:19:38, floitsch wrote:
> executable

Done.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:973: /// If the
function signature comes from a typedef or an inline function-typed
On 2014/07/15 14:19:38, floitsch wrote:
> what's an inline function-typed parameter?

Added an example.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:974: ///
parameter, the its parameters are not real parameters in that they can take
On 2014/07/15 14:19:38, floitsch wrote:
> ... then its ...

Done.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:980: abstract
class FormalElement extends Element
On 2014/07/15 14:19:38, floitsch wrote:
> On 2014/07/15 11:43:01, Johnni Winther wrote:
> > Other descarded name candidates: 'ParameterishElement',
'ParameteroidElement',
> > 'FunctionSignatureElement'.
> 
> I would be ok with ParameterishElement

I'll stick with [FormalElement] for new. We can change if there is concensus.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:981: implements
FunctionTypedElement, TypedElement, AstElement {
On 2014/07/15 14:19:38, floitsch wrote:
> indent +2

Done.

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/392873002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1385: *
[:A(this.field):].
On 2014/07/15 14:19:38, floitsch wrote:
> nit: `A(this.field)` and ///

Done.

[Johnni Winther, 2014-07-18 10:38:54]

Committed patchset #3 manually as r38381 (presubmit successful).

[floitsch, 2014-07-18 10:44:02]

Still LGTM.
better. thanks.

https://codereview.chromium.org/392873002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/392873002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:295: ///
Members are constructors, methods and fields.
Maybe one new line before (but not necessary).

https://codereview.chromium.org/392873002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:962: ///
non-element locals, like `this` and boxes, specialized [Local] class are
classes

[Johnni Winther, 2014-07-20 12:03:25]

https://codereview.chromium.org/392873002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/392873002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:295: ///
Members are constructors, methods and fields.
On 2014/07/18 10:44:02, floitsch wrote:
> Maybe one new line before (but not necessary).

Done in https://codereview.chromium.org/409473002

https://codereview.chromium.org/392873002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:962: ///
non-element locals, like `this` and boxes, specialized [Local] class are
On 2014/07/18 10:44:02, floitsch wrote:
> classes

Done in https://codereview.chromium.org/409473002