fix: Fix constructor parameter initialization order (AssemblyScript#1934

)
nearprotocol · Jun 29, 2021 · f4091b8 · f4091b8
1 parent a7f7226
commit f4091b8
Show file tree

Hide file tree

Showing 15 changed files with 566 additions and 144 deletions.
diff --git a/src/compiler.ts b/src/compiler.ts
@@ -6717,6 +6717,8 @@ export class Compiler extends DiagnosticEmitter {
     // Compile omitted arguments with final argument locals blocked. Doesn't need to take care of
     // side-effects within earlier expressions because these already happened on set.
     this.currentFlow = flow;
+    var isConstructor = instance.is(CommonFlags.CONSTRUCTOR);
+    if (isConstructor) flow.set(FlowFlags.CTORPARAM_CONTEXT);
     for (let i = numArguments; i < numParameters; ++i) {
       let initType = parameterTypes[i];
       let initExpr = this.compileExpression(
@@ -6729,12 +6731,13 @@ export class Compiler extends DiagnosticEmitter {
         this.makeLocalAssignment(argumentLocal, initExpr, initType, false)
       );
     }
+    flow.unset(FlowFlags.CTORPARAM_CONTEXT);
 
     // Compile the called function's body in the scope of the inlined flow
     this.compileFunctionBody(instance, body);
 
     // If a constructor, perform field init checks on its flow directly
-    if (instance.is(CommonFlags.CONSTRUCTOR)) {
+    if (isConstructor) {
       let parent = instance.parent;
       assert(parent.kind == ElementKind.CLASS);
       this.checkFieldInitializationInFlow(<Class>parent, flow);
@@ -6815,6 +6818,7 @@ export class Compiler extends DiagnosticEmitter {
     // accounting for additional locals and a proper `this` context.
     var previousFlow = this.currentFlow;
     var flow = stub.flow;
+    if (original.is(CommonFlags.CONSTRUCTOR)) flow.set(FlowFlags.CTORPARAM_CONTEXT);
     this.currentFlow = flow;
 
     // create a br_table switching over the number of optional parameters provided
@@ -7641,21 +7645,32 @@ export class Compiler extends DiagnosticEmitter {
           this.currentType = this.options.usizeType;
           return module.unreachable();
         }
-        if (actualFunction.is(CommonFlags.CONSTRUCTOR) && !(constraints & Constraints.IS_THIS)) {
-          let parent = actualFunction.parent;
-          assert(parent.kind == ElementKind.CLASS);
-          this.checkFieldInitialization(<Class>parent, expression);
+        if (actualFunction.is(CommonFlags.CONSTRUCTOR)) {
+          if (flow.is(FlowFlags.CTORPARAM_CONTEXT)) {
+            this.error(
+              DiagnosticCode._this_cannot_be_referenced_in_constructor_arguments,
+              expression.range
+            );
+          }
+          if (!(constraints & Constraints.IS_THIS)) {
+            let parent = actualFunction.parent;
+            assert(parent.kind == ElementKind.CLASS);
+            this.checkFieldInitialization(<Class>parent, expression);
+          }
         }
         let thisLocal = assert(flow.lookupLocal(CommonNames.this_));
         flow.set(FlowFlags.ACCESSES_THIS);
         this.currentType = thisType;
         return module.local_get(thisLocal.index, thisType.toRef());
       }
       case NodeKind.SUPER: {
-        let flow = this.currentFlow;
-        let actualFunction = flow.actualFunction;
         if (actualFunction.is(CommonFlags.CONSTRUCTOR)) {
-          if (!flow.is(FlowFlags.CALLS_SUPER)) {
+          if (flow.is(FlowFlags.CTORPARAM_CONTEXT)) {
+            this.error(
+              DiagnosticCode._super_cannot_be_referenced_in_constructor_arguments,
+              expression.range
+            );
+          } else if (!flow.is(FlowFlags.CALLS_SUPER)) {
             // TS1034 in the parser effectively limits this to property accesses
             this.error(
               DiagnosticCode._super_must_be_called_before_accessing_a_property_of_super_in_the_constructor_of_a_derived_class,
@@ -10292,10 +10307,9 @@ export class Compiler extends DiagnosticEmitter {
     var module = this.module;
     var flow = this.currentFlow;
     var isInline = flow.isInline;
-    var thisLocalIndex = isInline
-      ? flow.lookupLocal(CommonNames.this_)!.index
-      : 0;
+    var thisLocalIndex = isInline ? flow.lookupLocal(CommonNames.this_)!.index : 0;
     var sizeTypeRef = this.options.sizeTypeRef;
+    var nonParameterFields: Field[] | null = null;
 
     // TODO: for (let member of members.values()) {
     for (let _values = Map_values(members), i = 0, k = _values.length; i < k; ++i) {
@@ -10304,44 +10318,57 @@ export class Compiler extends DiagnosticEmitter {
         member.kind != ElementKind.FIELD || // not a field
         member.parent != classInstance      // inherited field
       ) continue;
-
       let field = <Field>member;
       assert(!field.isAny(CommonFlags.CONST));
-      let fieldType = field.type;
-      let fieldTypeRef = fieldType.toRef();
       let fieldPrototype = field.prototype;
-      let initializerNode = fieldPrototype.initializerNode;
       let parameterIndex = fieldPrototype.parameterIndex;
-      let initExpr: ExpressionRef;
-      let typeNode = field.typeNode;
-      if (typeNode) this.checkTypeSupported(fieldType, typeNode);
-
-      // if declared as a constructor parameter, use its value
-      if (parameterIndex >= 0) {
-        initExpr = module.local_get(
-          isInline
-            ? flow.lookupLocal(field.name)!.index
-            : 1 + parameterIndex, // this is local 0
-          fieldTypeRef
-        );
-
-      // fall back to use initializer if present
-      } else if (initializerNode) {
-        initExpr = this.compileExpression(initializerNode, fieldType, Constraints.CONV_IMPLICIT);
 
-      // otherwise initialize with zero
-      } else {
-        initExpr = this.makeZero(fieldType, fieldPrototype.declaration);
+      // Defer non-parameter fields until parameter fields are initialized
+      if (parameterIndex < 0) {
+        if (!nonParameterFields) nonParameterFields = new Array();
+        nonParameterFields.push(field);
+        continue;
       }
 
+      // Initialize constructor parameter field
+      let fieldType = field.type;
+      let fieldTypeRef = fieldType.toRef();
+      assert(!fieldPrototype.initializerNode);
       this.compileFieldSetter(field);
       stmts.push(
         module.call(field.internalSetterName, [
           module.local_get(thisLocalIndex, sizeTypeRef),
-          initExpr
+          module.local_get(
+            isInline
+              ? flow.lookupLocal(field.name)!.index
+              : 1 + parameterIndex, // `this` is local 0
+            fieldTypeRef
+          )
         ], TypeRef.None)
       );
     }
+
+    // Initialize deferred non-parameter fields
+    if (nonParameterFields) {
+      for (let i = 0, k = nonParameterFields.length; i < k; ++i) {
+        let field = unchecked(nonParameterFields[i]);
+        let fieldType = field.type;
+        let fieldPrototype = field.prototype;
+        let initializerNode = fieldPrototype.initializerNode;
+        assert(fieldPrototype.parameterIndex < 0);
+        this.compileFieldSetter(field);
+        stmts.push(
+          module.call(field.internalSetterName, [
+            module.local_get(thisLocalIndex, sizeTypeRef),
+            initializerNode // use initializer if present, otherwise initialize with zero
+              ? this.compileExpression(initializerNode, fieldType, Constraints.CONV_IMPLICIT)
+              : this.makeZero(fieldType, fieldPrototype.declaration)
+          ], TypeRef.None)
+        );
+      }
+    }
+
+    this.currentType = Type.void;
     return stmts;
   }
 

diff --git a/src/diagnosticMessages.generated.ts b/src/diagnosticMessages.generated.ts
@@ -124,7 +124,9 @@ export enum DiagnosticCode {
   Type_0_is_not_assignable_to_type_1 = 2322,
   Index_signature_is_missing_in_type_0 = 2329,
   _this_cannot_be_referenced_in_current_location = 2332,
+  _this_cannot_be_referenced_in_constructor_arguments = 2333,
   _super_can_only_be_referenced_in_a_derived_class = 2335,
+  _super_cannot_be_referenced_in_constructor_arguments = 2336,
   Super_calls_are_not_permitted_outside_constructors_or_in_nested_functions_inside_constructors = 2337,
   Property_0_does_not_exist_on_type_1 = 2339,
   Property_0_is_private_and_only_accessible_within_class_1 = 2341,
@@ -304,7 +306,9 @@ export function diagnosticCodeToString(code: DiagnosticCode): string {
     case 2322: return "Type '{0}' is not assignable to type '{1}'.";
     case 2329: return "Index signature is missing in type '{0}'.";
     case 2332: return "'this' cannot be referenced in current location.";
+    case 2333: return "'this' cannot be referenced in constructor arguments.";
     case 2335: return "'super' can only be referenced in a derived class.";
+    case 2336: return "'super' cannot be referenced in constructor arguments.";
     case 2337: return "Super calls are not permitted outside constructors or in nested functions inside constructors.";
     case 2339: return "Property '{0}' does not exist on type '{1}'.";
     case 2341: return "Property '{0}' is private and only accessible within class '{1}'.";

diff --git a/src/diagnosticMessages.json b/src/diagnosticMessages.json
@@ -122,7 +122,9 @@
   "Type '{0}' is not assignable to type '{1}'.": 2322,
   "Index signature is missing in type '{0}'.": 2329,
   "'this' cannot be referenced in current location.": 2332,
+  "'this' cannot be referenced in constructor arguments.": 2333,
   "'super' can only be referenced in a derived class.": 2335,
+  "'super' cannot be referenced in constructor arguments.": 2336,
   "Super calls are not permitted outside constructors or in nested functions inside constructors.": 2337,
   "Property '{0}' does not exist on type '{1}'.": 2339,
   "Property '{0}' is private and only accessible within class '{1}'.": 2341,

diff --git a/src/flow.ts b/src/flow.ts
@@ -129,6 +129,8 @@ export const enum FlowFlags {
 
   /** This is a flow with explicitly disabled bounds checking. */
   UNCHECKED_CONTEXT = 1 << 15,
+  /** This is a flow compiling a constructor parameter. */
+  CTORPARAM_CONTEXT = 1 << 16,
 
   // masks
 
@@ -757,7 +759,7 @@ export class Flow {
       newFlags |= FlowFlags.TERMINATES;
     }
 
-    this.flags = newFlags | (thisFlags & FlowFlags.UNCHECKED_CONTEXT);
+    this.flags = newFlags | (thisFlags & (FlowFlags.UNCHECKED_CONTEXT | FlowFlags.CTORPARAM_CONTEXT));
 
     // local flags
     var thisLocalFlags = this.localFlags;
@@ -869,7 +871,7 @@ export class Flow {
       newFlags |= FlowFlags.TERMINATES;
     }
 
-    this.flags = newFlags | (this.flags & FlowFlags.UNCHECKED_CONTEXT);
+    this.flags = newFlags | (this.flags & (FlowFlags.UNCHECKED_CONTEXT | FlowFlags.CTORPARAM_CONTEXT));
 
     // local flags
     var thisLocalFlags = this.localFlags;

diff --git a/tests/compiler/class.optimized.wat b/tests/compiler/class.optimized.wat
@@ -1727,7 +1727,7 @@
   call $~lib/memory/memory.fill
   local.get $1
  )
- (func $~lib/array/Array<i32>#set:buffer (param $0 i32) (param $1 i32)
+ (func $class/GenericInitializer<i32>#set:foo (param $0 i32) (param $1 i32)
   local.get $0
   local.get $1
   i32.store
@@ -1826,7 +1826,7 @@
    i32.store
    local.get $0
    i32.const 0
-   call $~lib/array/Array<i32>#set:buffer
+   call $class/GenericInitializer<i32>#set:foo
    local.get $0
    i32.const 0
    i32.store offset=4
@@ -1847,7 +1847,7 @@
    call $~lib/memory/memory.fill
    local.get $0
    local.get $1
-   call $~lib/array/Array<i32>#set:buffer
+   call $class/GenericInitializer<i32>#set:foo
    local.get $0
    local.get $1
    i32.store offset=4
@@ -1863,7 +1863,7 @@
    global.set $~lib/memory/__stack_pointer
    local.get $2
    local.get $0
-   call $~lib/array/Array<i32>#set:buffer
+   call $class/GenericInitializer<i32>#set:foo
    global.get $~lib/memory/__stack_pointer
    i32.const 4
    i32.add

diff --git a/tests/compiler/class.untouched.wat b/tests/compiler/class.untouched.wat
@@ -2525,6 +2525,15 @@
    end
   end
  )
+ (func $class/GenericInitializer<i32>#set:foo (param $0 i32) (param $1 i32)
+  local.get $0
+  local.get $1
+  i32.store
+  local.get $0
+  local.get $1
+  i32.const 0
+  call $~lib/rt/itcms/__link
+ )
  (func $~lib/array/Array<i32>#set:buffer (param $0 i32) (param $1 i32)
   local.get $0
   local.get $1
@@ -2549,15 +2558,6 @@
   local.get $1
   i32.store offset=12
  )
- (func $class/GenericInitializer<i32>#set:foo (param $0 i32) (param $1 i32)
-  local.get $0
-  local.get $1
-  i32.store
-  local.get $0
-  local.get $1
-  i32.const 0
-  call $~lib/rt/itcms/__link
- )
  (func $class/testGenericInitializer
   i32.const 0
   call $class/GenericInitializer<i32>#constructor

diff --git a/tests/compiler/constructor-errors.json b/tests/compiler/constructor-errors.json
@@ -0,0 +1,23 @@
+{
+  "asc_flags": [
+  ],
+  "stderr": [
+    "TS2333: 'this' cannot be referenced in constructor arguments.",
+    "b: i32 = this.a, // TS2333",
+    "TS2333: 'this' cannot be referenced in constructor arguments.",
+    "c: i32 = this.foo() // TS2333",
+    "TS2333: 'this' cannot be referenced in constructor arguments.",
+    "e: i32 = this.d, // TS2333",
+    "TS2333: 'this' cannot be referenced in constructor arguments.",
+    "f: i32 = this.bar() // TS2333",
+    "TS2336: 'super' cannot be referenced in constructor arguments.",
+    "h: i32 = super.g, // TS2336",
+    "TS2336: 'super' cannot be referenced in constructor arguments.",
+    "i: i32 = super.baz() // TS2336",
+    "TS2336: 'super' cannot be referenced in constructor arguments.",
+    "j: i32 = super.g, // TS2336",
+    "TS2336: 'super' cannot be referenced in constructor arguments.",
+    "k: i32 = super.baz() // TS2336",
+    "EOF"
+  ]
+}
diff --git a/tests/compiler/constructor-errors.ts b/tests/compiler/constructor-errors.ts
@@ -0,0 +1,52 @@
+class CtorAccessThis {
+  a: i32 = 0;
+  constructor(
+    public b: i32 = this.a, // TS2333
+    public c: i32 = this.foo() // TS2333
+  ) {}
+  foo(): i32 { return 0; }
+}
+
+new CtorAccessThis();
+
+class CtorAccessThisInline {
+  d: i32 = 0;
+  @inline
+  constructor(
+    public e: i32 = this.d, // TS2333
+    public f: i32 = this.bar() // TS2333
+  ) {}
+  bar(): i32 { return 0; }
+}
+
+new CtorAccessThisInline();
+
+class CtorSuper {
+  g: i32 = 0;
+  baz(): i32 { return 0; }
+}
+
+class CtorAccessSuper extends CtorSuper {
+  constructor(
+    public h: i32 = super.g, // TS2336
+    public i: i32 = super.baz() // TS2336
+  ) {
+    super();
+  }
+}
+
+new CtorAccessSuper();
+
+class CtorAccessSuperInline extends CtorSuper {
+  @inline
+  constructor(
+    public j: i32 = super.g, // TS2336
+    public k: i32 = super.baz() // TS2336
+  ) {
+    super();
+  }
+}
+
+new CtorAccessSuperInline();
+
+ERROR("EOF");