Add support for execution of constructor body

pkg/kernel/lib/interpreter/interpreter.dart

 // Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 library kernel.interpreter;
 
 import '../ast.dart';
 import '../ast.dart' as ast show Class;
 
 import '../log.dart';
 export '../log.dart';
@@ skipping 25 matching lines @@
   }
 }
 
 class Binding {
   final VariableDeclaration variable;
   Value value;
 
   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) {
       if (identical(b.variable, variable)) return true;
     }
     return parent?.contains(variable) ?? false;
   }
 
   Binding lookupBinding(VariableDeclaration variable) {
     assert(contains(variable));
@@ skipping 11 matching lines @@
     assert(contains(variable));
     lookupBinding(variable).value = value;
   }
 
   void expand(VariableDeclaration variable, Value value) {
     assert(!contains(variable));
     bindings.add(new Binding(variable, value));
   }
 }
 
+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> {
   Configuration eval(Expression expr, ExpressionConfiguration config) =>
       expr.accept1(this, config);
 
   Configuration evalList(List<InterpreterExpression> list, Environment env,
       ApplicationContinuation cont) {
     if (list.isNotEmpty) {
       return new ExpressionConfiguration(list.first.expression, env,
@@ skipping 69 matching lines @@
     // Currently supports only method invocation with <2 arguments and is used
     // to evaluate implemented operators for int, double and String values.
     var cont = new MethodInvocationContinuation(
         node.arguments, node.name, config.environment, config.continuation);
 
     return new ExpressionConfiguration(node.receiver, config.environment, cont);
   }
 
   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);
 
     return new ExpressionListConfiguration(args, config.environment, cont);
   }
 
   Configuration visitNot(Not node, ExpressionConfiguration config) {
     return new ExpressionConfiguration(node.operand, config.environment,
         new NotContinuation(config.continuation));
   }
@@ skipping 21 matching lines @@
   }
 
   Configuration visitStringConcatenation(
       StringConcatenation node, ExpressionConfiguration config) {
     var cont = new StringConcatenationContinuation(
         node.expressions, config.environment, config.continuation);
     return new ExpressionConfiguration(
         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) {
     return new ContinuationConfiguration(
         config.continuation, new StringValue(node.value));
   }
 
   Configuration visitIntLiteral(
       IntLiteral node, ExpressionConfiguration config) {
     return new ContinuationConfiguration(
@@ skipping 30 matching lines @@
 class State {
   final Environment environment;
   final Label labels;
   final StatementConfiguration statementConfiguration;
 
   final ExpressionContinuation returnContinuation;
 
   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);
   }
 
   State withBreak(Statement stmt) {
     Label breakLabels = new Label(stmt, statementConfiguration, labels);
     return new State(
         environment, breakLabels, statementConfiguration, returnContinuation);
   }
@@ skipping 42 matching lines @@
 
   Configuration step(StatementExecuter executer) =>
       executer.exec(statement, state);
 }
 
 class ExitConfiguration extends StatementConfiguration {
   final ExpressionContinuation returnContinuation;
 
   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;
   final Value value;
 
   ContinuationConfiguration(this.continuation, this.value);
 
   Configuration step(StatementExecuter _) => continuation(value);
 }
 
@@ skipping 109 matching lines @@
   FieldInitializerValue(this.field, this.value);
 }
 
 abstract class Continuation {}
 
 /// Represents the continuation called after the evaluation of argument
 /// expressions.
 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();
 
     // Add positional parameters.
     for (int i = 0; i < positional.length; ++i) {
       newEnv.expand(function.positionalParameters[i], positional[i].value);
     }
 
     Map<String, Value> named = new Map.fromIterable(
@@ skipping 27 matching lines @@
         .withEnvironment(functionEnv);
     return new StatementConfiguration(function.body, bodyState);
   }
 }
 
 /// Represents the application continuation for constructor invocation applied
 /// on the list of evaluated arguments.
 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) {
       // Constructor is redirecting.
       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);
 
         return new ExpressionListConfiguration(args, ctrEnv, app);
       }
       // Redirecting initializer is not the only initializer.
       for (Initializer i in constructor.initializers.reversed.skip(1)) {
         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);
   }
 
   /// Creates a list of expressions for instance field initializers in
   /// immediately enclosing class.
   static List<InterpreterExpression> _createInstanceInitializers(
       Constructor ctr) {
     Class currentClass = new Class(ctr.enclosingClass.reference);
@@ skipping 10 matching lines @@
 
     return es;
   }
 }
 
 /// Represents the application continuation applied on the list of evaluated
 /// field initializer expressions.
 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) {
     for (FieldInitializerValue current in fieldValues.reversed) {
       _currentClass.setProperty(newObject, current.field, current.value);
     }
 
     if (constructor.initializers.isEmpty) {
       _initializeNullFields(_currentClass, newObject);
-      return new ContinuationConfiguration(expressionContinuation, newObject);
+      return configuration;
     }
 
     if (constructor.initializers.first is SuperInitializer) {
       // SuperInitializer appears last in the initializer list.
       assert(constructor.initializers.length == 1);
       SuperInitializer current = constructor.initializers.first;
       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);
   }
 }
 
 /// Represents the expression continuation applied on the list of evaluated
 /// initializer expressions preceding a super call in the list.
 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;
     if (current is FieldInitializer) {
       currentClass.setProperty(newObject, current.field, v);
     } else if (current is LocalInitializer) {
       initializerEnvironment.expand(current.variable, v);
     } else {
       throw 'Assigning value $v to ${current.runtimeType}';
     }
 
     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];
 
     if (next is RedirectingInitializer) {
       // 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(
           args, initializerEnvironment, cont);
     }
 
     if (next is SuperInitializer) {
       // SuperInitializer appears last in the initializer list.
       assert(initializers.length == 2);
       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);
   }
 }
 
 /// Represents the application continuation called after the evaluation of all
 /// argument expressions for an invocation.
 class ValueApplication extends ApplicationContinuation {
   final InterpreterValue value;
   final ApplicationContinuation applicationContinuation;
@@ skipping 237 matching lines @@
 class LetContinuation extends ExpressionContinuation {
   final VariableDeclaration variable;
   final Expression letBody;
   final Environment environment;
   final ExpressionContinuation continuation;
 
   LetContinuation(
       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);
   }
 }
 
 /// Represents the continuation for the condition expression in [WhileStatement].
 class WhileConditionContinuation extends ExpressionContinuation {
   final WhileStatement node;
   final State state;
 
@@ skipping 89 matching lines @@
         new ExpressionStatementContinuation(state.statementConfiguration);
     return new ExpressionConfiguration(
         node.expression, state.environment, cont);
   }
 
   Configuration visitBlock(Block node, State state) {
     if (node.statements.isEmpty) {
       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(
           s, blockState.withConfiguration(configuration));
     }
     return configuration;
   }
 
   Configuration visitEmptyStatement(EmptyStatement node, State state) {
     return state.statementConfiguration;
@@ skipping 342 matching lines @@
 void _initializeNullFields(Class class_, ObjectValue newObject) {
   int superClassSize = class_.superclass?.instanceSize ?? 0;
   for (int i = superClassSize; i < class_.instanceSize; i++) {
     Field field = class_.instanceFields[i];
     if (class_.getProperty(newObject, field) == null) {
       assert(field.initializer == null);
       class_.setProperty(newObject, field, Value.nullInstance);
     }
   }
 }