Better error recovery.

Better error recovery.

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

[ngeoffray, 2012-01-10 07:49:45]

LGTM, but did you forget to upload SyntaxTest?

http://codereview.chromium.org/9158001/diff/2004/dart/frog/leg/scanner/parser...
File dart/frog/leg/scanner/parser.dart (right):

http://codereview.chromium.org/9158001/diff/2004/dart/frog/leg/scanner/parser...
dart/frog/leg/scanner/parser.dart:371: // listener more explicitly that it must
pop an identifier.
Is that still a hack?

[ahe, 2012-01-10 08:02:28]

Thank you, Nicolas.

Uploaded SyntaxTest.dart. PTAL.

Cheers,
Peter

http://codereview.chromium.org/9158001/diff/2004/dart/frog/leg/scanner/parser...
File dart/frog/leg/scanner/parser.dart (right):

http://codereview.chromium.org/9158001/diff/2004/dart/frog/leg/scanner/parser...
dart/frog/leg/scanner/parser.dart:371: // listener more explicitly that it must
pop an identifier.
On 2012/01/10 07:49:45, ngeoffray wrote:
> Is that still a hack?

Yes. Something is wrong here, but we don't know what the user intended. Calling
endTopLevelFields keeps the node stack in the listener well balanced, but we
should explicitly tell the listener that this isn't a field.

[ahe, 2012-01-10 08:05:55]

I noticed a few things I should clean up before submitting.

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
File dart/tests/language/src/SyntaxTest.dart (right):

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
dart/tests/language/src/SyntaxTest.dart:233: typedef <T>(); /// 62: compile-time
error
I should move this up before the main method and renumber the tests.

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
dart/tests/language/src/SyntaxTest.dart:234: class B extends void {} /// 63:
compile-time error
I should also move this. This should be:

class B
extends void /// XX: compile-time error
{}

And I should remember to call new B() from the main method.

[ngeoffray, 2012-01-10 08:07:09]

Still LGTM

http://codereview.chromium.org/9158001/diff/2004/dart/frog/leg/scanner/parser...
File dart/frog/leg/scanner/parser.dart (right):

http://codereview.chromium.org/9158001/diff/2004/dart/frog/leg/scanner/parser...
dart/frog/leg/scanner/parser.dart:371: // listener more explicitly that it must
pop an identifier.
On 2012/01/10 08:02:28, ahe wrote:
> On 2012/01/10 07:49:45, ngeoffray wrote:
> > Is that still a hack?
> 
> Yes. Something is wrong here, but we don't know what the user intended.
Calling
> endTopLevelFields keeps the node stack in the listener well balanced, but we
> should explicitly tell the listener that this isn't a field.

So should you do the same line 555?

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
File dart/tests/language/src/SyntaxTest.dart (right):

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
dart/tests/language/src/SyntaxTest.dart:224: assert(null is void); /// 61:
compile-time error
assert -> print? Just to make sure no one skips the expression.

[ahe, 2012-01-10 08:32:33]

Thank you

http://codereview.chromium.org/9158001/diff/2004/dart/frog/leg/scanner/parser...
File dart/frog/leg/scanner/parser.dart (right):

http://codereview.chromium.org/9158001/diff/2004/dart/frog/leg/scanner/parser...
dart/frog/leg/scanner/parser.dart:371: // listener more explicitly that it must
pop an identifier.
On 2012/01/10 08:07:09, ngeoffray wrote:
> On 2012/01/10 08:02:28, ahe wrote:
> > On 2012/01/10 07:49:45, ngeoffray wrote:
> > > Is that still a hack?
> > 
> > Yes. Something is wrong here, but we don't know what the user intended.
> Calling
> > endTopLevelFields keeps the node stack in the listener well balanced, but we
> > should explicitly tell the listener that this isn't a field.
> 
> So should you do the same line 555?

Done.

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
File dart/tests/language/src/SyntaxTest.dart (right):

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
dart/tests/language/src/SyntaxTest.dart:224: assert(null is void); /// 61:
compile-time error
On 2012/01/10 08:07:09, ngeoffray wrote:
> assert -> print? Just to make sure no one skips the expression.

Done.

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
dart/tests/language/src/SyntaxTest.dart:233: typedef <T>(); /// 62: compile-time
error
On 2012/01/10 08:05:56, ahe wrote:
> I should move this up before the main method and renumber the tests.

Done.

http://codereview.chromium.org/9158001/diff/11001/dart/tests/language/src/Syn...
dart/tests/language/src/SyntaxTest.dart:234: class B extends void {} /// 63:
compile-time error
On 2012/01/10 08:05:56, ahe wrote:
> I should also move this. This should be:
> 
> class B
> extends void /// XX: compile-time error
> {}
> 
> And I should remember to call new B() from the main method.

Done.