Share code for parsing local functions.

Share code for parsing local functions.

R=danrubel@google.com, johnniwinther@google.com

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

[ahe, 2017-08-07 11:45:53]

ahe@google.com changed reviewers:
+ danrubel@google.com, johnniwinther@google.com

[ahe, 2017-08-07 11:45:54]

Address Dan's comments from CL 2985673002 and more.

[Johnni Winther, 2017-08-07 11:57:12]

lgtm

[danrubel, 2017-08-07 12:43:35]

LGTM

[ahe, 2017-08-07 13:46:19]

ahe@google.com changed reviewers:
+ scheglov@google.com

[ahe, 2017-08-07 13:46:25]

Konstantin,

I'd uploaded this CL based on a two week old branch, so when I merged with
master, I found a minor issue. I'm going to submit this if all tests pass, but
I'd still appreciate if you'd take a look at the changes I've made to
_BodySkippingParser.

[ahe, 2017-08-07 14:06:10]

Description was changed from

==========
Share code for parsing local functions.
==========

to

==========
Share code for parsing local functions.

R=danrubel@google.com, johnniwinther@google.com

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

[ahe, 2017-08-07 14:06:12]

Committed patchset #2 (id:20001) manually as
dbe482caac4185532649b0169904e74562c0039e (presubmit successful).

[scheglov, 2017-08-08 01:42:27]

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:117: return isExpression ?
close.next : close;
I don't understand this change.
How could it be an expression if it starts with '{'?

[ahe, 2017-08-08 08:13:30]

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:117: return isExpression ?
close.next : close;
On 2017/08/08 01:42:27, scheglov wrote:
> I don't understand this change.
> How could it be an expression if it starts with '{'?

[isExpression] is true if the function is in expression context, for example:

print(() { return "I'm a function in expression context."; }());

foo() {
  print("I'm a function in statement context, or a declaration.");
}

There's a subtle difference in expression context when using the arrow notation:

print(() => "I don't have a trailing semicolon.");

bar() => "But I do.";

The way I deal with that difference is by having this invariant: when parsing a
function expression, this method returns the next token which isn't part of the
expression. Otherwise, this is a function declaration, and we always return the
end token (which is a "}" for a function with a block body, and semicolon for
arrow notation).

[scheglov, 2017-08-08 15:54:57]

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:117: return isExpression ?
close.next : close;
On 2017/08/08 08:13:30, ahe wrote:
> On 2017/08/08 01:42:27, scheglov wrote:
> > I don't understand this change.
> > How could it be an expression if it starts with '{'?
> 
> [isExpression] is true if the function is in expression context, for example:
> 
> print(() { return "I'm a function in expression context."; }());
> 
> foo() {
>   print("I'm a function in statement context, or a declaration.");
> }
> 
> There's a subtle difference in expression context when using the arrow
notation:
> 
> print(() => "I don't have a trailing semicolon.");
> 
> bar() => "But I do.";
> 
> The way I deal with that difference is by having this invariant: when parsing
a
> function expression, this method returns the next token which isn't part of
the
> expression. Otherwise, this is a function declaration, and we always return
the
> end token (which is a "}" for a function with a block body, and semicolon for
> arrow notation).

Now I understand.
Thank you.

[ahe, 2017-08-08 16:03:33]

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:117: return isExpression ?
close.next : close;
On 2017/08/08 15:54:57, scheglov wrote:
> On 2017/08/08 08:13:30, ahe wrote:
> > On 2017/08/08 01:42:27, scheglov wrote:
> > > I don't understand this change.
> > > How could it be an expression if it starts with '{'?
> > 
> > [isExpression] is true if the function is in expression context, for
example:
> > 
> > print(() { return "I'm a function in expression context."; }());
> > 
> > foo() {
> >   print("I'm a function in statement context, or a declaration.");
> > }
> > 
> > There's a subtle difference in expression context when using the arrow
> notation:
> > 
> > print(() => "I don't have a trailing semicolon.");
> > 
> > bar() => "But I do.";
> > 
> > The way I deal with that difference is by having this invariant: when
parsing
> a
> > function expression, this method returns the next token which isn't part of
> the
> > expression. Otherwise, this is a function declaration, and we always return
> the
> > end token (which is a "}" for a function with a block body, and semicolon
for
> > arrow notation).
> 
> Now I understand.
> Thank you.

Great. Would it have been less confusing initially if I had called this boolean
"isFunctionExpression"?

[scheglov, 2017-08-08 16:06:03]

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2999523002/diff/20001/pkg/front_end/lib/src/i...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:117: return isExpression ?
close.next : close;
On 2017/08/08 16:03:33, ahe wrote:
> On 2017/08/08 15:54:57, scheglov wrote:
> > On 2017/08/08 08:13:30, ahe wrote:
> > > On 2017/08/08 01:42:27, scheglov wrote:
> > > > I don't understand this change.
> > > > How could it be an expression if it starts with '{'?
> > > 
> > > [isExpression] is true if the function is in expression context, for
> example:
> > > 
> > > print(() { return "I'm a function in expression context."; }());
> > > 
> > > foo() {
> > >   print("I'm a function in statement context, or a declaration.");
> > > }
> > > 
> > > There's a subtle difference in expression context when using the arrow
> > notation:
> > > 
> > > print(() => "I don't have a trailing semicolon.");
> > > 
> > > bar() => "But I do.";
> > > 
> > > The way I deal with that difference is by having this invariant: when
> parsing
> > a
> > > function expression, this method returns the next token which isn't part
of
> > the
> > > expression. Otherwise, this is a function declaration, and we always
return
> > the
> > > end token (which is a "}" for a function with a block body, and semicolon
> for
> > > arrow notation).
> > 
> > Now I understand.
> > Thank you.
> 
> Great. Would it have been less confusing initially if I had called this
boolean
> "isFunctionExpression"?

Even better would be "ofFunctionExpression", because the body we're parsing
*isn't* function expression, but a part of it.