Fix for parsing EmptyFunctionBody with Fasta.

Fix for parsing EmptyFunctionBody with Fasta.

R=ahe@google.com, paulberry@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/e12dc7c2f86229020cb628edfbec4f40133e5fd7

[ahe, 2017-03-13 09:20:19]

lgtm

https://codereview.chromium.org/2742233002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/parser/listener.dart (right):

https://codereview.chromium.org/2742233002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:562: void
endEmptyFunctionBody(Token semicolon) {
Overall, I think it might be simpler to add a boolean to endFunctionBody.

However, if you prefer this approach, you should add a comment to both
beginFunctionBody and endFunctionBody that this method might be called instead
of endFunctionBody. See lines 22-24.

[scheglov, 2017-03-13 15:48:40]

https://codereview.chromium.org/2742233002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/parser/listener.dart (right):

https://codereview.chromium.org/2742233002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:562: void
endEmptyFunctionBody(Token semicolon) {
On 2017/03/13 09:20:19, ahe wrote:
> Overall, I think it might be simpler to add a boolean to endFunctionBody.

  It depends.
  This way we get at least some documentation about tokens in form of parameter
names.

> 
> However, if you prefer this approach, you should add a comment to both
> beginFunctionBody and endFunctionBody that this method might be called instead
> of endFunctionBody. See lines 22-24.

  It does not look that endFunctionBody() or most of the other methods have
documentation. So, any reader should expect to use search to learn anything.

  Actually, beginFunctionBody() is not what it says at all.
  It is not consistent with the language specification.
  It is more like beginBlockFunctionBody().
  So search and thoughtful reading is a must.

[scheglov, 2017-03-13 16:05:49]

Description was changed from

==========
Fix for parsing EmptyFunctionBody with Fasta.

R=ahe@google.com, paulberry@google.com
BUG=
==========

to

==========
Fix for parsing EmptyFunctionBody with Fasta.

R=ahe@google.com, paulberry@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/e12dc7c2f86229020cb628edfbec4f40133e5fd7
==========

[scheglov, 2017-03-13 16:05:50]

Committed patchset #1 (id:1) manually as
e12dc7c2f86229020cb628edfbec4f40133e5fd7 (presubmit successful).

[Paul Berry, 2017-03-13 16:55:16]

lgtm

https://codereview.chromium.org/2742233002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/parser/listener.dart (right):

https://codereview.chromium.org/2742233002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:562: void
endEmptyFunctionBody(Token semicolon) {
On 2017/03/13 15:48:40, scheglov wrote:
> On 2017/03/13 09:20:19, ahe wrote:
> > Overall, I think it might be simpler to add a boolean to endFunctionBody.
> 
>   It depends.
>   This way we get at least some documentation about tokens in form of
parameter
> names.
> 
> > 
> > However, if you prefer this approach, you should add a comment to both
> > beginFunctionBody and endFunctionBody that this method might be called
instead
> > of endFunctionBody. See lines 22-24.
> 
>   It does not look that endFunctionBody() or most of the other methods have
> documentation. So, any reader should expect to use search to learn anything.
> 
>   Actually, beginFunctionBody() is not what it says at all.
>   It is not consistent with the language specification.
>   It is more like beginBlockFunctionBody().
>   So search and thoughtful reading is a must.
> 

I agree that the current state of the documentation in the Fasta code base isn't
great--often the developer has to read the code carefully to understand what's
going on.  But it sounds like you're making the argument that since the
developer has to read the code carefully, there's no point in writing
documentation.  I think that's a dangerous line of thinking--applied
universally, it would mean that we never got around to adding documentation to
code that lacked it.

I think that whenever one of us learns something by reading code, or a subtlety
is discussed in a code review that would have been nice to have been in a
comment, we should add comments.  If we keep doing this consistently, the code
base will eventually have good enough documentation that developers can
consistently rely on it.