Setup getter and setter elements.

Setup getter and setter elements.

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

[ahe, 2012-01-24 16:01:34]

https://chromiumcodereview.appspot.com/9117033/diff/1/dart/frog/leg/frog_leg....
File dart/frog/leg/frog_leg.dart (right):

https://chromiumcodereview.appspot.com/9117033/diff/1/dart/frog/leg/frog_leg....
dart/frog/leg/frog_leg.dart:99: if (token === null && element !== null) {
This is way too complicated. I have made a better version that uses this
newfangled technology called "recursion".

[ngeoffray, 2012-01-25 08:36:05]

LGTM!

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/elemen...
File dart/frog/leg/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/elemen...
dart/frog/leg/elements/elements.dart:142: ? cachedNode.getBeginToken() :
variables.position();
This will return different values in time. Is that what we want?

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/emitte...
File dart/frog/leg/emitter.dart (right):

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/emitte...
dart/frog/leg/emitter.dart:70: element: member);
Because that's the only place it's needed, and because we know that if we're
here, the node is available, I would prefer doing:
node: element.parseNode(compiler, compiler).

I am not convinced about the robustness of the 'position' method in Element
(what if someone starts cancelling on the element before it got parsed, or
resolved?). Therefore I would not provide the possibility of passing it an
element in the API.

[ahe, 2012-01-25 08:56:24]

Thank you, Nicolas.

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/elemen...
File dart/frog/leg/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/elemen...
dart/frog/leg/elements/elements.dart:142: ? cachedNode.getBeginToken() :
variables.position();
On 2012/01/25 08:36:05, ngeoffray wrote:
> This will return different values in time. Is that what we want?

You're right.

I'll add a todo to record the Token instead of the SourceString. It isn't good
enough to try to extract the information from the name, as this might be a byte
offset, not a character offset.

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/emitte...
File dart/frog/leg/emitter.dart (right):

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/emitte...
dart/frog/leg/emitter.dart:70: element: member);
On 2012/01/25 08:36:05, ngeoffray wrote:
> Because that's the only place it's needed, and because we know that if we're
> here, the node is available, I would prefer doing:
> node: element.parseNode(compiler, compiler).
> 
> I am not convinced about the robustness of the 'position' method in Element
> (what if someone starts cancelling on the element before it got parsed, or
> resolved?). Therefore I would not provide the possibility of passing it an
> element in the API.

I tried calling parseNode here, but noticed that I got a different error
message. This is what happened: parseFunction doesn't handle "get" or "set",
yet.

Moving forward, it would seem that we don't actually know that the node is
available at this point.

Regardless, it is a sound principle to keep error handling as simple as
possible. Invoking the parser just to report an error message does not seem like
a great idea to me anyways (but it was the first thing I thought of doing :-).

[ngeoffray, 2012-01-25 09:03:59]

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/emitte...
File dart/frog/leg/emitter.dart (right):

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/emitte...
dart/frog/leg/emitter.dart:70: element: member);
On 2012/01/25 08:56:24, ahe wrote:
> On 2012/01/25 08:36:05, ngeoffray wrote:
> > Because that's the only place it's needed, and because we know that if we're
> > here, the node is available, I would prefer doing:
> > node: element.parseNode(compiler, compiler).
> > 
> > I am not convinced about the robustness of the 'position' method in Element
> > (what if someone starts cancelling on the element before it got parsed, or
> > resolved?). Therefore I would not provide the possibility of passing it an
> > element in the API.
> 
> I tried calling parseNode here, but noticed that I got a different error
> message. This is what happened: parseFunction doesn't handle "get" or "set",
> yet.
> 
> Moving forward, it would seem that we don't actually know that the node is
> available at this point.

Yeah, and I guess that's a bug: we're not putting in the work queue the
getter/setter.

> 
> Regardless, it is a sound principle to keep error handling as simple as
> possible. Invoking the parser just to report an error message does not seem
like
> a great idea to me anyways (but it was the first thing I thought of doing :-).

I agree. Then we have to realize we must give up on giving a location if
cachedNode is null (which you do not check against in the 'position' methods).
You're just lucky that this is backed by a PartialElement that knows its initial
location :)

[ahe, 2012-01-25 09:07:14]

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/emitte...
File dart/frog/leg/emitter.dart (right):

https://chromiumcodereview.appspot.com/9117033/diff/4001/dart/frog/leg/emitte...
dart/frog/leg/emitter.dart:70: element: member);
On 2012/01/25 09:03:59, ngeoffray wrote:
> On 2012/01/25 08:56:24, ahe wrote:
> > On 2012/01/25 08:36:05, ngeoffray wrote:
> > > Because that's the only place it's needed, and because we know that if
we're
> > > here, the node is available, I would prefer doing:
> > > node: element.parseNode(compiler, compiler).
> > > 
> > > I am not convinced about the robustness of the 'position' method in
Element
> > > (what if someone starts cancelling on the element before it got parsed, or
> > > resolved?). Therefore I would not provide the possibility of passing it an
> > > element in the API.
> > 
> > I tried calling parseNode here, but noticed that I got a different error
> > message. This is what happened: parseFunction doesn't handle "get" or "set",
> > yet.
> > 
> > Moving forward, it would seem that we don't actually know that the node is
> > available at this point.
> 
> Yeah, and I guess that's a bug: we're not putting in the work queue the
> getter/setter.
> 
> > 
> > Regardless, it is a sound principle to keep error handling as simple as
> > possible. Invoking the parser just to report an error message does not seem
> like
> > a great idea to me anyways (but it was the first thing I thought of doing
:-).
> 
> I agree. Then we have to realize we must give up on giving a location if
> cachedNode is null (which you do not check against in the 'position' methods).
> You're just lucky that this is backed by a PartialElement that knows its
initial
> location :)

I'm not sure what situation you have in mind here. But let's talk more about
this when I arrive at the office.