Repeated prefixes and parser fixes.

Repeated prefixes and parser fixes.

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

[ngeoffray, 2012-03-10 22:36:51]

LGTM!

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

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:270: listener.cancel('duplicate definition
$element', token: element.position());
line too long

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:270: listener.cancel('duplicate definition
$element', token: element.position());
definition 'of' ?

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:349: Token position() =>
findMyName(variables.position());
Why don't you check cachedNode first before going into slow case?

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:417: if (getter !== null &&
getter.enclosingElement === enclosingElement) {
I don't understand the second check.

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:420: return setter.position();
setter could be null

[ahe, 2012-03-10 23:07:18]

Hi Nicolas,

Thank you for your comments. I think I need to add a few more comments to the
code.

Cheers,
Peter

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

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:349: Token position() =>
findMyName(variables.position());
On 2012/03/10 22:36:51, ngeoffray wrote:
> Why don't you check cachedNode first before going into slow case?

Wouldn't be correct for function typed parameters.

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:417: if (getter !== null &&
getter.enclosingElement === enclosingElement) {
On 2012/03/10 22:36:51, ngeoffray wrote:
> I don't understand the second check.

I'll add a comment explaining this.

The getter and setter may be defined in two different compilation units.
However, we know that one of them is non-null and defined in the same
compilation unit as the abstract element.

We need to make sure that the position returned is relative to the compilation
unit of the abstract element.

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:420: return setter.position();
On 2012/03/10 22:36:51, ngeoffray wrote:
> setter could be null

No. See my comment above.

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/scanner/p...
File dart/frog/leg/scanner/partial_parser.dart (right):

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/scanner/p...
dart/frog/leg/scanner/partial_parser.dart:23: if (value === '=' &&
token.next.stringValue === '{') {
I should add a comment explaining this covers this case:

class Foo {
  var map;
  Foo() : map = {};
}

[ngeoffray, 2012-03-10 23:11:27]

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

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:349: Token position() =>
findMyName(variables.position());
On 2012/03/10 23:07:18, ahe wrote:
> On 2012/03/10 22:36:51, ngeoffray wrote:
> > Why don't you check cachedNode first before going into slow case?
> 
> Wouldn't be correct for function typed parameters.

Could you please add that as a comment?

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:417: if (getter !== null &&
getter.enclosingElement === enclosingElement) {
On 2012/03/10 23:07:18, ahe wrote:
> On 2012/03/10 22:36:51, ngeoffray wrote:
> > I don't understand the second check.
> 
> I'll add a comment explaining this.
> 
> The getter and setter may be defined in two different compilation units.
> However, we know that one of them is non-null and defined in the same
> compilation unit as the abstract element.
> 
> We need to make sure that the position returned is relative to the compilation
> unit of the abstract element.

I see, thanks for the comment!

[ahe, 2012-03-11 13:01:50]

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

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:270: listener.cancel('duplicate definition
$element', token: element.position());
On 2012/03/10 22:36:51, ngeoffray wrote:
> line too long

Done.

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:270: listener.cancel('duplicate definition
$element', token: element.position());
On 2012/03/10 22:36:51, ngeoffray wrote:
> definition 'of' ?

Debug code.

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:349: Token position() =>
findMyName(variables.position());
On 2012/03/10 23:11:27, ngeoffray wrote:
> On 2012/03/10 23:07:18, ahe wrote:
> > On 2012/03/10 22:36:51, ngeoffray wrote:
> > > Why don't you check cachedNode first before going into slow case?
> > 
> > Wouldn't be correct for function typed parameters.
> 
> Could you please add that as a comment?

Done.

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/scanner/p...
File dart/frog/leg/scanner/partial_parser.dart (right):

https://chromiumcodereview.appspot.com/9663047/diff/1/dart/frog/leg/scanner/p...
dart/frog/leg/scanner/partial_parser.dart:23: if (value === '=' &&
token.next.stringValue === '{') {
On 2012/03/10 23:07:18, ahe wrote:
> I should add a comment explaining this covers this case:
> 
> class Foo {
>   var map;
>   Foo() : map = {};
> }

Done.