Fix premature layout in resolveClass recursion (#1032)

Author: dcodeIO

src/builtins.ts

@@ -850,7 +850,7 @@ export function compileCall(
         }
         offset = (<Field>field).memoryOffset;
       } else {
-        offset = classType.currentMemoryOffset;
+        offset = classType.nextMemoryOffset;
       }
       if (compiler.options.isWasm64) {
         // implicitly wrap if contextual type is a 32-bit integer

src/compiler.ts

@@ -1579,7 +1579,7 @@ export class Compiler extends DiagnosticEmitter {
     var runtimeHeaderSize = program.runtimeHeaderSize;
     var arrayPrototype = assert(program.arrayPrototype);
     var arrayInstance = assert(this.resolver.resolveClass(arrayPrototype, [ elementType ]));
-    var arrayInstanceSize = arrayInstance.currentMemoryOffset;
+    var arrayInstanceSize = arrayInstance.nextMemoryOffset;
     var bufferLength = bufferSegment.buffer.length - runtimeHeaderSize;
     var arrayLength = i32(bufferLength / elementType.byteSize);
 
@@ -9123,8 +9123,8 @@ export class Compiler extends DiagnosticEmitter {
     this.compileFunction(allocInstance);
     return module.call(allocInstance.internalName, [
       options.isWasm64
-        ? module.i64(classInstance.currentMemoryOffset)
-        : module.i32(classInstance.currentMemoryOffset),
+        ? module.i64(classInstance.nextMemoryOffset)
+        : module.i32(classInstance.nextMemoryOffset),
       module.i32(
         classInstance.hasDecorator(DecoratorFlags.UNMANAGED)
           ? 0

src/program.ts

@@ -3300,7 +3300,7 @@ export class Class extends TypedElement {
   /** Contextual type arguments for fields and methods. */
   contextualTypeArguments: Map<string,Type> | null = null;
   /** Current member memory offset. */
-  currentMemoryOffset: u32 = 0;
+  nextMemoryOffset: u32 = 0;
   /** Constructor instance. */
   constructorInstance: Function | null = null;
   /** Operator overloads. */
@@ -3350,8 +3350,6 @@ export class Class extends TypedElement {
     prototype: ClassPrototype,
     /** Concrete type arguments, if any. */
     typeArguments: Type[] | null = null,
-    /** Base class, if derived. */
-    base: Class | null = null,
     _isInterface: bool = false // FIXME
   ) {
     super(
@@ -3368,26 +3366,13 @@ export class Class extends TypedElement {
     this.decoratorFlags = prototype.decoratorFlags;
     this.typeArguments = typeArguments;
     this.setType(program.options.usizeType.asClass(this));
-    this.base = base;
 
     if (!this.hasDecorator(DecoratorFlags.UNMANAGED)) {
       let id = program.nextClassId++;
       this._id = id;
       program.managedClasses.set(id, this);
     }
 
-    // inherit static members and contextual type arguments from base class
-    if (base) {
-      let inheritedTypeArguments = base.contextualTypeArguments;
-      if (inheritedTypeArguments) {
-        let contextualTypeArguments = this.contextualTypeArguments;
-        for (let [baseName, baseType] of inheritedTypeArguments) {
-          if (!contextualTypeArguments) this.contextualTypeArguments = contextualTypeArguments = new Map();
-          contextualTypeArguments.set(baseName, baseType);
-        }
-      }
-    }
-
     // apply pre-checked instance-specific contextual type arguments
     var typeParameters = prototype.typeParameterNodes;
     if (typeArguments) {
@@ -3407,6 +3392,26 @@ export class Class extends TypedElement {
     registerConcreteElement(program, this);
   }
 
+  /** Sets the base class. */
+  setBase(base: Class): void {
+    assert(!this.base);
+    this.base = base;
+
+    // Inherit contextual type arguments from base class
+    var inheritedTypeArguments = base.contextualTypeArguments;
+    if (inheritedTypeArguments) {
+      let contextualTypeArguments = this.contextualTypeArguments;
+      for (let [baseName, baseType] of inheritedTypeArguments) {
+        if (!contextualTypeArguments) {
+          this.contextualTypeArguments = contextualTypeArguments = new Map();
+          contextualTypeArguments.set(baseName, baseType);
+        } else if (!contextualTypeArguments.has(baseName)) {
+          contextualTypeArguments.set(baseName, baseType);
+        }
+      }
+    }
+  }
+
   /** Tests if a value of this class type is assignable to a target of the specified class type. */
   isAssignableTo(target: Class): bool {
     var current: Class | null = this;
@@ -3460,7 +3465,7 @@ export class Class extends TypedElement {
   /** Writes a field value to a buffer and returns the number of bytes written. */
   writeField<T>(name: string, value: T, buffer: Uint8Array, baseOffset: i32): i32 {
     var field = this.lookupInSelf(name);
-    if (field && field.kind == ElementKind.FIELD) {
+    if (field !== null && field.kind == ElementKind.FIELD) {
       let offset = baseOffset + (<Field>field).memoryOffset;
       switch ((<Field>field).type.kind) {
         case TypeKind.I8:
@@ -3657,14 +3662,12 @@ export class Interface extends Class { // FIXME
   constructor(
     nameInclTypeParameters: string,
     prototype: InterfacePrototype,
-    typeArguments: Type[] = [],
-    base: Interface | null = null
+    typeArguments: Type[] = []
   ) {
     super(
       nameInclTypeParameters,
       prototype,
       typeArguments,
-      base,
       true
     );
   }

src/resolver.ts

@@ -2729,6 +2729,9 @@ export class Resolver extends DiagnosticEmitter {
     );
   }
 
+  /** Currently resolving classes. */
+  private resolveClassPending: Class[] = [];
+
   /** Resolves a class prototype using the specified concrete type arguments. */
   resolveClass(
     /** The prototype of the class. */
@@ -2742,10 +2745,19 @@ export class Resolver extends DiagnosticEmitter {
   ): Class | null {
     var instanceKey = typeArguments ? typesToString(typeArguments) : "";
 
-    // Check if this exact instance has already been resolved
+    // Do not attempt to resolve the same class twice. This can return a class
+    // that isn't fully resolved yet, but only on deeper levels of recursion.
     var instance = prototype.getResolvedInstance(instanceKey);
     if (instance) return instance;
 
+    // Otherwise create
+    var nameInclTypeParamters = prototype.name;
+    if (instanceKey.length) nameInclTypeParamters += "<" + instanceKey + ">";
+    instance = new Class(nameInclTypeParamters, prototype, typeArguments);
+    prototype.setResolvedInstance(instanceKey, instance);
+    var pendingClasses = this.resolveClassPending;
+    pendingClasses.push(instance);
+
     // Insert contextual type arguments for this operation. Internally, this method is always
     // called with matching type parameter / argument counts.
     if (typeArguments) {
@@ -2760,13 +2772,10 @@ export class Resolver extends DiagnosticEmitter {
       let typeParameterNodes = prototype.typeParameterNodes;
       assert(!(typeParameterNodes && typeParameterNodes.length));
     }
-
-    // There shouldn't be an instance before resolving the base class
-    assert(!prototype.getResolvedInstance(instanceKey));
+    instance.contextualTypeArguments = ctxTypes;
 
     // Resolve base class if applicable
     var basePrototype = prototype.basePrototype;
-    var baseClass: Class | null = null;
     if (basePrototype) {
       let current: ClassPrototype | null = basePrototype;
       do {
@@ -2780,70 +2789,68 @@ export class Resolver extends DiagnosticEmitter {
         }
       } while (current = current.basePrototype);
       let extendsNode = assert(prototype.extendsNode); // must be present if it has a base prototype
-      baseClass = this.resolveClassInclTypeArguments(
+      let base = this.resolveClassInclTypeArguments(
         basePrototype,
         extendsNode.typeArguments,
         prototype.parent, // relative to derived class
         makeMap(ctxTypes), // don't inherit
         extendsNode,
         reportMode
       );
-      if (!baseClass) return null;
-    }
+      if (!base) return null;
+      instance.setBase(base);
 
-    // Construct the instance and remember that it has been resolved already
-    var recursiveInstanceFromResolvingBase = prototype.getResolvedInstance(instanceKey);
-    if (recursiveInstanceFromResolvingBase) {
-      // Happens if the base class already triggers resolving this class
-      instance = recursiveInstanceFromResolvingBase;
-    } else {
-      let nameInclTypeParamters = prototype.name;
-      if (instanceKey.length) nameInclTypeParamters += "<" + instanceKey + ">";
-      instance = new Class(nameInclTypeParamters, prototype, typeArguments, baseClass);
-      prototype.setResolvedInstance(instanceKey, instance);
+      // If the base class is still pending, yield here and instead resolve any
+      // derived classes once the base class's `finishResolveClass` is done.
+      // This is guaranteed to never happen at the entry of the recursion, i.e.
+      // where `resolveClass` is called from other code.
+      if (pendingClasses.includes(base)) return instance;
     }
-    instance.contextualTypeArguments = ctxTypes; // unique (as specified by the caller)
 
-    // Inherit base class members and set up the initial memory offset for own fields
+    // We only get here if the base class has been fully resolved already.
+    this.finishResolveClass(instance, reportMode);
+    return instance;
+  }
+
+  /** Finishes resolving the specified class. */
+  private finishResolveClass(
+    /** Class to finish resolving. */
+    instance: Class,
+    /** How to proceed with eventual diagnostics. */
+    reportMode: ReportMode
+  ): void {
+    var instanceMembers = instance.members;
+    if (!instanceMembers) instance.members = instanceMembers = new Map();
+
+    // Alias base members
+    var pendingClasses = this.resolveClassPending;
     var memoryOffset: u32 = 0;
-    if (baseClass) {
-      let baseMembers = baseClass.members;
+    var base = instance.base;
+    if (base) {
+      assert(!pendingClasses.includes(base));
+      let baseMembers = base.members;
       if (baseMembers) {
-        let instanceMembers = instance.members;
-        if (!instanceMembers) instance.members = instanceMembers = new Map();
         for (let [baseMemberName, baseMember] of baseMembers) {
           instanceMembers.set(baseMemberName, baseMember);
         }
       }
-      memoryOffset = baseClass.currentMemoryOffset;
+      memoryOffset = base.nextMemoryOffset;
     }
 
     // Resolve instance members
+    var prototype = instance.prototype;
     var instanceMemberPrototypes = prototype.instanceMembers;
     if (instanceMemberPrototypes) {
       for (let member of instanceMemberPrototypes.values()) {
         switch (member.kind) {
 
-          // Lay out fields in advance
           case ElementKind.FIELD_PROTOTYPE: {
-            let instanceMembers = instance.members;
-            if (!instanceMembers) instance.members = instanceMembers = new Map();
-            else if (instanceMembers.has(member.name)) {
-              let existing = instanceMembers.get(member.name)!;
-              this.errorRelated(
-                DiagnosticCode.Duplicate_identifier_0,
-                (<FieldPrototype>member).identifierNode.range,
-                existing.declaration.name.range,
-                member.name
-              );
-              break;
-            }
             let fieldTypeNode = (<FieldPrototype>member).typeNode;
             let fieldType: Type | null = null;
             // TODO: handle duplicate non-private fields specifically?
             if (!fieldTypeNode) {
-              if (baseClass) {
-                let baseMembers = baseClass.members;
+              if (base) {
+                let baseMembers = base.members;
                 if (baseMembers && baseMembers.has((<FieldPrototype>member).name)) {
                   let baseField = baseMembers.get((<FieldPrototype>member).name)!;
                   if (!baseField.is(CommonFlags.PRIVATE)) {
@@ -2869,13 +2876,13 @@ export class Resolver extends DiagnosticEmitter {
               );
             }
             if (!fieldType) break; // did report above
-            let fieldInstance = new Field(<FieldPrototype>member, instance, fieldType);
+            let field = new Field(<FieldPrototype>member, instance, fieldType);
             assert(isPowerOf2(fieldType.byteSize));
             let mask = fieldType.byteSize - 1;
             if (memoryOffset & mask) memoryOffset = (memoryOffset | mask) + 1;
-            fieldInstance.memoryOffset = memoryOffset;
+            field.memoryOffset = memoryOffset;
             memoryOffset += fieldType.byteSize;
-            instance.add(member.name, fieldInstance); // reports
+            instance.add(member.name, field); // reports
             break;
           }
           case ElementKind.FUNCTION_PROTOTYPE: {
@@ -2923,7 +2930,7 @@ export class Resolver extends DiagnosticEmitter {
     }
 
     // Finalize memory offset
-    instance.currentMemoryOffset = memoryOffset;
+    instance.nextMemoryOffset = memoryOffset;
 
     // Link _own_ constructor if present
     {
@@ -2933,7 +2940,7 @@ export class Resolver extends DiagnosticEmitter {
         let ctorInstance = this.resolveFunction(
           <FunctionPrototype>ctorPrototype,
           null,
-          instance.contextualTypeArguments,
+          assert(instance.contextualTypeArguments),
           reportMode
         );
         if (ctorInstance) instance.constructorInstance = <Function>ctorInstance;
@@ -3002,7 +3009,24 @@ export class Resolver extends DiagnosticEmitter {
         }
       }
     }
-    return instance;
+
+    // Remove this class from pending
+    var pendingIndex = pendingClasses.indexOf(instance);
+    assert(~pendingIndex); // must be pending
+    pendingClasses.splice(pendingIndex, 1);
+
+    // Finish derived classes that we postponed in `resolveClass` due to the
+    // base class still being pending, again triggering `finishResolveClass`
+    // of any classes derived from those classes, ultimately leading to all
+    // pending classes being resolved.
+    var derivedPendingClasses = new Array<Class>();
+    for (let i = 0, k = pendingClasses.length; i < k; ++i) {
+      let pending = pendingClasses[i];
+      if (instance == pending.base) derivedPendingClasses.push(pending);
+    }
+    for (let i = 0, k = derivedPendingClasses.length; i < k; ++i) {
+      this.finishResolveClass(derivedPendingClasses[i], reportMode);
+    }
   }
 
   /** Resolves a class prototype by first resolving the specified type arguments. */