Add support for function declarations and function expressions

Add support for function declarations and function expressions


BUG=
R=dmitryas@google.com

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

[zhivkag, 2017-08-04 08:06:24]

zhivkag@google.com changed reviewers:
+ dmitryas@google.com, kmillikin@google.com

[Dmitry Stefantsov, 2017-08-16 07:57:54]

Please, find a question in the comments.

https://codereview.chromium.org/2997443002/diff/20001/pkg/kernel/lib/interpre...
File pkg/kernel/lib/interpreter/interpreter.dart (right):

https://codereview.chromium.org/2997443002/diff/20001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:1740: final EnvironmentLocation
location;
Why do we need an [EnvironmentLocation] here?  As I understand it, the
environment doesn't change for a [FunctionValue], so we may just use an
[Environment].  Also, the spec uses an [Environment] in this case.

[zhivkag, 2017-08-16 08:16:30]

https://codereview.chromium.org/2997443002/diff/20001/pkg/kernel/lib/interpre...
File pkg/kernel/lib/interpreter/interpreter.dart (right):

https://codereview.chromium.org/2997443002/diff/20001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:1740: final EnvironmentLocation
location;
On 2017/08/16 07:57:54, Dmitry Stefantsov wrote:
> Why do we need an [EnvironmentLocation] here?  As I understand it, the
> environment doesn't change for a [FunctionValue], so we may just use an
> [Environment].  Also, the spec uses an [Environment] in this case.  

The specification is updated w.r.t. the discussion we had about "recursive"
definition of the FunctionValue and/or the environment two weeks ago.

The discussion was: 
When we define a local function, a variable declaration is associated with it.
The current environment will be extended to bind this variable declaration to
the function value. The function value captures the formals, the body and
environment in scope of the function declaration.

The new binding should be also visible in the captured environment (ex.
recursive functions).
I think this implies that we need to use a location to store the environment and
store the extended environment in it after FunctionValue is created.

[Dmitry Stefantsov, 2017-08-16 08:32:33]

https://codereview.chromium.org/2997443002/diff/20001/pkg/kernel/lib/interpre...
File pkg/kernel/lib/interpreter/interpreter.dart (right):

https://codereview.chromium.org/2997443002/diff/20001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:1740: final EnvironmentLocation
location;
On 2017/08/16 08:16:29, zhivkag wrote:
> On 2017/08/16 07:57:54, Dmitry Stefantsov wrote:
> > Why do we need an [EnvironmentLocation] here?  As I understand it, the
> > environment doesn't change for a [FunctionValue], so we may just use an
> > [Environment].  Also, the spec uses an [Environment] in this case.  
> 
> The specification is updated w.r.t. the discussion we had about "recursive"
> definition of the FunctionValue and/or the environment two weeks ago.
> 
> The discussion was: 
> When we define a local function, a variable declaration is associated with it.
> The current environment will be extended to bind this variable declaration to
> the function value. The function value captures the formals, the body and
> environment in scope of the function declaration.
> 
> The new binding should be also visible in the captured environment (ex.
> recursive functions).
> I think this implies that we need to use a location to store the environment
and
> store the extended environment in it after FunctionValue is created.

Yes, I remember the discussion.  However, I may have misinterpreted its
conclusion.  My understanding is that we may create a new environment for the
function right away, based on the current environment, with one extra location
for the function value that is yet to be evaluated; `null` can be stored in this
location temporarily.

We may create this environment and use it immediately to form the function
value.  One location in it will be updated after the function value is created.

This way we may avoid having the whole new locations class for storing
environments in locations, i.e. [EnvironmentLocation].  It also feels strange
that a non-value object (an [Environment]) can be stored in locations.

[zhivkag, 2017-08-16 09:33:13]

PTAL

https://codereview.chromium.org/2997443002/diff/20001/pkg/kernel/lib/interpre...
File pkg/kernel/lib/interpreter/interpreter.dart (right):

https://codereview.chromium.org/2997443002/diff/20001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:1740: final EnvironmentLocation
location;
On 2017/08/16 08:32:33, Dmitry Stefantsov wrote:
> On 2017/08/16 08:16:29, zhivkag wrote:
> > On 2017/08/16 07:57:54, Dmitry Stefantsov wrote:
> > > Why do we need an [EnvironmentLocation] here?  As I understand it, the
> > > environment doesn't change for a [FunctionValue], so we may just use an
> > > [Environment].  Also, the spec uses an [Environment] in this case.  
> > 
> > The specification is updated w.r.t. the discussion we had about "recursive"
> > definition of the FunctionValue and/or the environment two weeks ago.
> > 
> > The discussion was: 
> > When we define a local function, a variable declaration is associated with
it.
> > The current environment will be extended to bind this variable declaration
to
> > the function value. The function value captures the formals, the body and
> > environment in scope of the function declaration.
> > 
> > The new binding should be also visible in the captured environment (ex.
> > recursive functions).
> > I think this implies that we need to use a location to store the environment
> and
> > store the extended environment in it after FunctionValue is created.
> 
> Yes, I remember the discussion.  However, I may have misinterpreted its
> conclusion.  My understanding is that we may create a new environment for the
> function right away, based on the current environment, with one extra location
> for the function value that is yet to be evaluated; `null` can be stored in
this
> location temporarily.
> 
> We may create this environment and use it immediately to form the function
> value.  One location in it will be updated after the function value is
created.
> 
> This way we may avoid having the whole new locations class for storing
> environments in locations, i.e. [EnvironmentLocation].  It also feels strange
> that a non-value object (an [Environment]) can be stored in locations.

Thanks for the suggestion, I like this approach better too!

[Dmitry Stefantsov, 2017-08-16 11:59:06]

LGTM

[zhivkag, 2017-08-21 05:52:28]

Description was changed from

==========
Add support for function declarations and function expressions


BUG=
==========

to

==========
Add support for function declarations and function expressions


BUG=
R=dmitryas@google.com

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

[zhivkag, 2017-08-21 05:52:30]

Committed patchset #4 (id:60001) manually as
e09b3830cf452655ab0a519a9ff862ecb98debe9 (presubmit successful).