Add support for execution of constructor body

pkg/kernel/lib/interpreter/interpreter.dart

Index: pkg/kernel/lib/interpreter/interpreter.dart
diff --git a/pkg/kernel/lib/interpreter/interpreter.dart b/pkg/kernel/lib/interpreter/interpreter.dart
index d5b63b5b98b7c90f7163f8480f0bfb5c18c34c61..3cccb82698fa7e7292ba345fabb340178aa0e43b 100644
--- a/pkg/kernel/lib/interpreter/interpreter.dart
+++ b/pkg/kernel/lib/interpreter/interpreter.dart
@@ -43,12 +43,18 @@ class Binding {
   Binding(this.variable, this.value);
 }
 
-class Environment {
+abstract class Environment {

[Dmitry Stefantsov, 2017/05/16 08:07:50]

I'd rather handle the case of 'this' with a couple of new methods of
Environments, than creating a hierarchy for Environments.

We may forbid 'this' as a variable name in existing 'lookup', 'assign', and
'expand' methods and create methods 'lookupThisBinding()' with no arguments and
'expandThis(value)' that takes an object value. It will guarantee the special
treatment of 'this' and that it is unique across the environment branch. It will
also help to preserve the universal treatment of environments.

I just think that keeping different kinds of environments may be hard to
maintain (once we have global variables, InstanceENvironments should have a
StaticEnvironment as a parent, which we can't do now with the introduced
hierarchy) and hard to describe in terms of operational semantics.

[zhivkag, 2017/05/16 10:51:13]

I see the limitations the previous hierarchy introduced, I changed to a simpler
approach that should be isomorphic to adding some new methods in the existing
environment, what do you think?

[Dmitry Stefantsov, 2017/05/16 11:06:52]

To me, additional methods look simpler and easier to translate to
functional-programming style. But this solution is also ok.

   final List<Binding> bindings = <Binding>[];
-  final Environment parent;
+  Environment get parent;
 
-  Environment.empty() : parent = null;
-  Environment(this.parent);
+  Environment();
+  factory Environment.withParent(Environment parent) {
+    if (parent is StaticEnvironment) {
+      return new StaticEnvironment(parent);
+    }
+    assert(parent is InstanceEnvironment);
+    return new InstanceEnvironment(parent);
+  }
 
   bool contains(VariableDeclaration variable) {
     for (Binding b in bindings.reversed) {
@@ -80,6 +86,22 @@ class Environment {
   }
 }
 
+class StaticEnvironment extends Environment {
+  final StaticEnvironment parent;
+  StaticEnvironment.empty() : parent = null;
+  StaticEnvironment(this.parent);
+}
+
+class InstanceEnvironment extends Environment {
+  final Value _thisInstance;
+  final InstanceEnvironment parent;
+
+  get thisInstance => _thisInstance ?? parent.thisInstance;

[Dmitry Stefantsov, 2017/05/16 08:07:50]

Just in case we decide to go with environment class hierarchy, I guess it may be
"parent?.thisInstance", to avoid null dereferencing in case no
InstanceEnvironment in the list having a "_thisInstance".

[zhivkag, 2017/05/16 10:51:13]

Done.

+
+  InstanceEnvironment.initial(this._thisInstance) : parent = null;
+  InstanceEnvironment(this.parent) : _thisInstance = null;
+}
+
 /// Evaluate expressions.
 class Evaluator
     extends ExpressionVisitor1<Configuration, ExpressionConfiguration> {
@@ -169,20 +191,13 @@ class Evaluator
 
   Configuration visitConstructorInvocation(
       ConstructorInvocation node, ExpressionConfiguration config) {
-    // Currently, the bodies of the constructors are not executed.
-    // Currently initializer list is executed only for redirecting
-    // constructors.
-    if (node.target.function.body is! EmptyStatement) {
-      throw 'Execution for body of constructor is not implemented.';
-    }
-
     var class_ = new Class(node.target.enclosingClass.reference);
     var newObject =
         new ObjectValue(class_, new List<Value>(class_.instanceSize));
-
+    var nextConfig =
+        new NewInstanceConfiguration(config.continuation, newObject);
     ApplicationContinuation cont = new ConstructorInvocationApplication(
-        newObject, node.target, config.continuation);
-
+        newObject, node.target, nextConfig);

[Dmitry Stefantsov, 2017/05/16 08:07:50]

I think it's better to keep 'config.continuation' here. The point is that
constructor invocation produces a value, and  'config.continuation' is an
expression continuation, i.e. something that is expecting a value.
Theoretically, they are supposed to be used together without intermediate
configurations.

[zhivkag, 2017/05/16 10:51:13]

Acknowledged.

     var args =
         _createArgumentExpressionList(node.arguments, node.target.function);
 
@@ -224,6 +239,12 @@ class Evaluator
         node.expressions.first, config.environment, cont);
   }
 
+  Configuration visitThisExpression(
+      ThisExpression node, ExpressionConfiguration config) {
+    InstanceEnvironment env = config.environment;
+    return new ContinuationConfiguration(config.continuation, env.thisInstance);
+  }
+
   // Evaluation of BasicLiterals.
   Configuration visitStringLiteral(
       StringLiteral node, ExpressionConfiguration config) {
@@ -274,7 +295,7 @@ class State {
   State(this.environment, this.labels, this.statementConfiguration,
       this.returnContinuation);
 
-  State.initial() : this(new Environment.empty(), null, null, null);
+  State.initial() : this(new StaticEnvironment.empty(), null, null, null);
 
   State withEnvironment(Environment env) {
     return new State(env, labels, statementConfiguration, returnContinuation);
@@ -337,11 +358,23 @@ class ExitConfiguration extends StatementConfiguration {
 
   ExitConfiguration(this.returnContinuation) : super(null, null);
 
-  Configuration step(StatementExecuter executer) {
+  Configuration step(StatementExecuter _) {
     return returnContinuation(Value.nullInstance);
   }
 }
 
+class NewInstanceConfiguration extends StatementConfiguration {

[Dmitry Stefantsov, 2017/05/16 08:07:50]

In case we decide to keep the intermediate configuration between constructor
invocation and the expression continuation that expects the created object, I
don't see why we can't use 'ContinuationConfiguration' instead of
'NewInstanceConfiguration'.

[Dmitry Stefantsov, 2017/05/16 09:59:24]

Thank you for the explanation offline about the 'NewInstanceConfiguration'! Now
I see that we really need something like this, because it basically acts as the
missing 'return' statement at the end of the constructor body.

One additional consideration is that we may actually lookup 'this' in the
current environment here and pass the value to the continuation, rather than
keeping the object that will be mutated somewhere between the invocations of the
'NewInstanceConfiguration()' constructor and the 'step()' method.

[zhivkag, 2017/05/16 10:51:13]

I assume some of the other comments were clarified by our discussion and are
related to this one, so I haven't addressed them (just ACKed), please correct me
if I am wrong. 

+  final ExpressionContinuation continuation;
+  final ObjectValue newObject;
+
+  NewInstanceConfiguration(this.continuation, this.newObject)
+      : super(null, new State.initial());
+
+  Configuration step(StatementExecuter _) {
+    return continuation(newObject);
+  }
+}
+
 /// Represents the configuration for applying an [ExpressionContinuation].
 class ContinuationConfiguration extends Configuration {
   final ExpressionContinuation continuation;
@@ -471,13 +504,13 @@ abstract class Continuation {}
 abstract class ApplicationContinuation extends Continuation {
   Configuration call(List<InterpreterValue> values);
 
-  /// Creates an environment binding actual argument values to formal parameters
-  /// of the function in a new environment, which is used to execute the
-  /// body od the function.
+  /// Binds actual argument values to formal parameters of the function in a
+  /// new environment or in the provided initial environment.
   /// TODO: Add checks for validation of arguments according to spec.
   static Environment createEnvironment(
-      FunctionNode function, List<InterpreterValue> args) {
-    Environment newEnv = new Environment.empty();
+      FunctionNode function, List<InterpreterValue> args,
+      [initialEnv]) {
+    Environment newEnv = initialEnv ?? new StaticEnvironment.empty();
     List<PositionalArgumentValue> positional = args.reversed
         .where((InterpreterValue av) => av is PositionalArgumentValue)
         .toList();
@@ -525,14 +558,15 @@ class StaticInvocationApplication extends ApplicationContinuation {
 class ConstructorInvocationApplication extends ApplicationContinuation {
   final ObjectValue newObject;
   final Constructor constructor;
-  final ExpressionContinuation expressionContinuation;
+  final StatementConfiguration configuration;

[Dmitry Stefantsov, 2017/05/16 08:07:50]

I think it's better to keep 'ExpressionContinuation' instead of
'StatementConfiguration' here. Also, please, see the comments above and below on
the same issue.

[zhivkag, 2017/05/16 10:51:13]

Acknowledged.

 
   ConstructorInvocationApplication(
-      this.newObject, this.constructor, this.expressionContinuation);
+      this.newObject, this.constructor, this.configuration);
 
   Configuration call(List<InterpreterValue> argValues) {
-    Environment ctrEnv = ApplicationContinuation.createEnvironment(
-        constructor.function, argValues);
+    var bodyEnv = new InstanceEnvironment.initial(newObject);
+    InstanceEnvironment ctrEnv = ApplicationContinuation.createEnvironment(
+        constructor.function, argValues, bodyEnv);
 
     if (constructor.initializers.isNotEmpty &&
         constructor.initializers.last is RedirectingInitializer) {
@@ -540,7 +574,7 @@ class ConstructorInvocationApplication extends ApplicationContinuation {
       Initializer initializer = constructor.initializers.first;
       if (initializer is RedirectingInitializer) {
         var app = new ConstructorInvocationApplication(
-            newObject, initializer.target, expressionContinuation);
+            newObject, initializer.target, configuration);
         var args = _createArgumentExpressionList(
             initializer.arguments, initializer.target.function);
 
@@ -551,15 +585,24 @@ class ConstructorInvocationApplication extends ApplicationContinuation {
         assert(i is LocalInitializer);
       }
       var class_ = new Class(constructor.enclosingClass.reference);
-      var cont = new InitializerContinuation(newObject, class_, ctrEnv,
-          constructor.initializers, expressionContinuation);
+      var cont = new InitializerContinuation(
+          newObject, class_, ctrEnv, constructor.initializers, configuration);
       return new ExpressionConfiguration(
           (initializer as LocalInitializer).variable.initializer, ctrEnv, cont);
     }
 
+    var newState = configuration.state
+        .withEnvironment(bodyEnv)
+        .withConfiguration(configuration);

[Dmitry Stefantsov, 2017/05/16 08:07:50]

Here we might have used 'withExpressionContinuation' and pass previously
existing field 'expressionContinuation' to it.

[zhivkag, 2017/05/16 10:51:13]

Acknowledged.

+
+    // Set head of configurations to be executed to configuration for current
+    // constructor body.
+    var nextConfiguration =
+        new StatementConfiguration(constructor.function.body, newState);

[Dmitry Stefantsov, 2017/05/16 08:07:50]

Here we may assume that the body of constructor has implicit 'return this'
statement that returns the value to the 'returnContinuation'.

[zhivkag, 2017/05/16 10:51:13]

Acknowledged.

+
     // Initialize fields in immediately enclosing class.
     var cont = new InstanceFieldsApplication(
-        newObject, constructor, ctrEnv, expressionContinuation);
+        newObject, constructor, ctrEnv, nextConfiguration);
     var fieldExpressions = _createInstanceInitializers(constructor);
 
     return new ExpressionListConfiguration(fieldExpressions, ctrEnv, cont);
@@ -590,13 +633,13 @@ class ConstructorInvocationApplication extends ApplicationContinuation {
 class InstanceFieldsApplication extends ApplicationContinuation {
   final ObjectValue newObject;
   final Constructor constructor;
-  final Environment environment;
-  final ExpressionContinuation expressionContinuation;
+  final InstanceEnvironment environment;
+  final StatementConfiguration configuration;
 
   final Class _currentClass;
 
-  InstanceFieldsApplication(this.newObject, this.constructor, this.environment,
-      this.expressionContinuation)
+  InstanceFieldsApplication(
+      this.newObject, this.constructor, this.environment, this.configuration)
       : _currentClass = new Class(constructor.enclosingClass.reference);
 
   Configuration call(List<InterpreterValue> fieldValues) {
@@ -606,7 +649,7 @@ class InstanceFieldsApplication extends ApplicationContinuation {
 
     if (constructor.initializers.isEmpty) {
       _initializeNullFields(_currentClass, newObject);
-      return new ContinuationConfiguration(expressionContinuation, newObject);
+      return configuration;
     }
 
     if (constructor.initializers.first is SuperInitializer) {
@@ -616,16 +659,16 @@ class InstanceFieldsApplication extends ApplicationContinuation {
       var args = _createArgumentExpressionList(
           current.arguments, current.target.function);
       var superApp = new ConstructorInvocationApplication(
-          newObject, current.target, expressionContinuation);
+          newObject, current.target, configuration);
       _initializeNullFields(_currentClass, newObject);
       return new ExpressionListConfiguration(args, environment, superApp);
     }
 
     Class class_ = new Class(constructor.enclosingClass.reference);
-    Environment initEnv = new Environment(environment);
+    InstanceEnvironment initEnv = new Environment.withParent(environment);
 
-    var cont = new InitializerContinuation(newObject, class_, initEnv,
-        constructor.initializers, expressionContinuation);
+    var cont = new InitializerContinuation(
+        newObject, class_, initEnv, constructor.initializers, configuration);
     return new ExpressionConfiguration(
         _getExpression(constructor.initializers.first), initEnv, cont);
   }
@@ -636,12 +679,12 @@ class InstanceFieldsApplication extends ApplicationContinuation {
 class InitializerContinuation extends ExpressionContinuation {
   final ObjectValue newObject;
   final Class currentClass;
-  final Environment initializerEnvironment;
+  final InstanceEnvironment initializerEnvironment;
   final List<Initializer> initializers;
-  final ExpressionContinuation continuation;
+  final StatementConfiguration configuration;
 
   InitializerContinuation(this.newObject, this.currentClass,
-      this.initializerEnvironment, this.initializers, this.continuation);
+      this.initializerEnvironment, this.initializers, this.configuration);
 
   Configuration call(Value v) {
     Initializer current = initializers.first;
@@ -654,9 +697,8 @@ class InitializerContinuation extends ExpressionContinuation {
     }
 
     if (initializers.length <= 1) {
-      // todo: return configuration for body of ctr.
       _initializeNullFields(currentClass, newObject);
-      return new ContinuationConfiguration(continuation, newObject);
+      return configuration;
     }
 
     Initializer next = initializers[1];
@@ -665,7 +707,7 @@ class InitializerContinuation extends ExpressionContinuation {
       // RedirectingInitializer appears last in the initializer list.
       assert(initializers.length == 2);
       var cont = new ConstructorInvocationApplication(
-          newObject, next.target, continuation);
+          newObject, next.target, configuration);
       var args =
           _createArgumentExpressionList(next.arguments, next.target.function);
       return new ExpressionListConfiguration(
@@ -678,14 +720,14 @@ class InitializerContinuation extends ExpressionContinuation {
       var args =
           _createArgumentExpressionList(next.arguments, next.target.function);
       var superApp = new ConstructorInvocationApplication(
-          newObject, next.target, continuation);
+          newObject, next.target, configuration);
       _initializeNullFields(currentClass, newObject);
       return new ExpressionListConfiguration(
           args, initializerEnvironment, superApp);
     }
 
     var cont = new InitializerContinuation(newObject, currentClass,
-        initializerEnvironment, initializers.skip(1).toList(), continuation);
+        initializerEnvironment, initializers.skip(1).toList(), configuration);
     return new ExpressionConfiguration(
         _getExpression(next), initializerEnvironment, cont);
   }
@@ -943,7 +985,7 @@ class LetContinuation extends ExpressionContinuation {
       this.variable, this.letBody, this.environment, this.continuation);
 
   Configuration call(Value value) {
-    var letEnv = new Environment(environment);
+    var letEnv = new Environment.withParent(environment);
     letEnv.expand(variable, value);
     return new ExpressionConfiguration(letBody, letEnv, continuation);
   }
@@ -1053,7 +1095,7 @@ class StatementExecuter extends StatementVisitor1<Configuration, State> {
       return state.statementConfiguration;
     }
     State blockState =
-        state.withEnvironment(new Environment(state.environment));
+        state.withEnvironment(new Environment.withParent(state.environment));
     StatementConfiguration configuration = state.statementConfiguration;
     for (Statement s in node.statements.reversed) {
       configuration = new StatementConfiguration(