fix: Compile virtual dependencies before finalizing stubs (#2040)

Author: dcodeIO

src/compiler.ts

@@ -389,8 +389,8 @@ export class Compiler extends DiagnosticEmitter {
   lazyFunctions: Set<Function> = new Set();
   /** Pending class-specific instanceof helpers. */
   pendingClassInstanceOf: Set<ClassPrototype> = new Set();
-  /** Functions potentially involving a virtual call. */
-  virtualCalls: Set<Function> = new Set();
+  /** Virtually called stubs that may have overloads. */
+  virtualStubs: Set<Function> = new Set();
   /** Elements currently undergoing compilation. */
   pendingElements: Set<Element> = new Set();
   /** Elements, that are module exports, already processed */
@@ -450,6 +450,7 @@ export class Compiler extends DiagnosticEmitter {
     var options = this.options;
     var module = this.module;
     var program = this.program;
+    var resolver = this.resolver;
     var hasShadowStack = options.stackSize > 0; // implies runtime=incremental
 
     // initialize lookup maps, built-ins, imports, exports, etc.
@@ -530,26 +531,37 @@ export class Compiler extends DiagnosticEmitter {
       compileClassInstanceOf(this, prototype);
     }
 
-    // set up virtual lookup tables
+    // set up virtual stubs
     var functionTable = this.functionTable;
+    var virtualStubs = this.virtualStubs;
     for (let i = 0, k = functionTable.length; i < k; ++i) {
       let instance = functionTable[i];
       if (instance.is(CommonFlags.VIRTUAL)) {
         assert(instance.is(CommonFlags.INSTANCE));
-        functionTable[i] = this.ensureVirtualStub(instance); // incl. varargs
-        this.finalizeVirtualStub(instance);
+        functionTable[i] = this.ensureVirtualStub(instance); // includes varargs stub
       } else if (instance.signature.requiredParameters < instance.signature.parameterTypes.length) {
         functionTable[i] = this.ensureVarargsStub(instance);
       }
     }
-    var virtualCalls = this.virtualCalls;
-    while (virtualCalls.size) {
-      // finalizing a stub may discover more virtual calls, so do this in a loop
-      for (let _values = Set_values(virtualCalls), i = 0, k = _values.length; i < k; ++i) {
+    var virtualStubsSeen = new Set<Function>();
+    do {
+      // virtual stubs and overloads have cross-dependencies on each other, in that compiling
+      // either may discover the respective other. do this in a loop until no more are found.
+      resolver.discoveredOverload = false;
+      for (let _values = Set_values(virtualStubs), i = 0, k = _values.length; i < k; ++i) {
         let instance = unchecked(_values[i]);
-        this.finalizeVirtualStub(instance);
-        virtualCalls.delete(instance);
+        let overloadInstances = resolver.resolveOverloads(instance);
+        if (overloadInstances) {
+          for (let i = 0, k = overloadInstances.length; i < k; ++i) {
+            this.compileFunction(overloadInstances[i]);
+          }
+        }
+        virtualStubsSeen.add(instance);
       }
+    } while (virtualStubs.size > virtualStubsSeen.size || resolver.discoveredOverload);
+    virtualStubsSeen.clear();
+    for (let _values = Set_values(virtualStubs), i = 0, k = _values.length; i < k; ++i) {
+      this.finalizeVirtualStub(_values[i]);
     }
 
     // finalize runtime features
@@ -6949,7 +6961,7 @@ export class Compiler extends DiagnosticEmitter {
       null,
       module.unreachable()
     );
-    this.virtualCalls.add(original);
+    this.virtualStubs.add(original);
     return stub;
   }
 
@@ -6958,11 +6970,7 @@ export class Compiler extends DiagnosticEmitter {
     var stub = this.ensureVirtualStub(instance);
     if (stub.is(CommonFlags.COMPILED)) return;
 
-    // Wouldn't be here if there wasn't at least one overload
-    var overloadPrototypes = assert(instance.prototype.overloads);
-
     assert(instance.parent.kind == ElementKind.CLASS || instance.parent.kind == ElementKind.INTERFACE);
-    var parentClassInstance = <Class>instance.parent;
     var module = this.module;
     var usizeType = this.options.usizeType;
     var sizeTypeRef = usizeType.toRef();
@@ -6986,106 +6994,65 @@ export class Compiler extends DiagnosticEmitter {
         TypeRef.I32
       )
     );
-
-    // A method's `overloads` property contains its unbound overload prototypes
-    // so we first have to find the concrete classes it became bound to, obtain
-    // their bound prototypes and make sure these are resolved and compiled as
-    // we are going to call them conditionally based on this's class id.
-    for (let _values = Set_values(overloadPrototypes), i = 0, k = _values.length; i < k; ++i) {
-      let unboundOverloadPrototype = _values[i];
-      assert(!unboundOverloadPrototype.isBound);
-      let unboundOverloadParent = unboundOverloadPrototype.parent;
-      let isProperty = unboundOverloadParent.kind == ElementKind.PROPERTY_PROTOTYPE;
-      let classInstances: Map<string,Class> | null;
-      if (isProperty) {
-        let propertyParent = (<PropertyPrototype>unboundOverloadParent).parent;
-        assert(propertyParent.kind == ElementKind.CLASS_PROTOTYPE);
-        classInstances = (<ClassPrototype>propertyParent).instances;
-      } else {
-        assert(unboundOverloadParent.kind == ElementKind.CLASS_PROTOTYPE);
-        classInstances = (<ClassPrototype>unboundOverloadParent).instances;
-      }
-      if (classInstances) {
-        for (let _values = Map_values(classInstances), j = 0, l = _values.length; j < l; ++j) {
-          let classInstance = _values[j];
-          // Chcek if the parent class is a subtype of instance's class
-          if (!classInstance.isAssignableTo(parentClassInstance)) continue;
-          let overloadInstance: Function | null;
-          if (isProperty) {
-            let boundProperty = assert(classInstance.members!.get(unboundOverloadParent.name));
-            assert(boundProperty.kind == ElementKind.PROPERTY_PROTOTYPE);
-            let boundPropertyInstance = this.resolver.resolveProperty(<PropertyPrototype>boundProperty);
-            if (!boundPropertyInstance) continue;
-            if (instance.is(CommonFlags.GET)) {
-              overloadInstance = boundPropertyInstance.getterInstance;
-            } else {
-              assert(instance.is(CommonFlags.SET));
-              overloadInstance = boundPropertyInstance.setterInstance;
-            }
-          } else {
-            let boundPrototype = assert(classInstance.members!.get(unboundOverloadPrototype.name));
-            assert(boundPrototype.kind == ElementKind.FUNCTION_PROTOTYPE);
-            overloadInstance = this.resolver.resolveFunction(<FunctionPrototype>boundPrototype, instance.typeArguments);
-          }
-          if (!overloadInstance || !this.compileFunction(overloadInstance)) continue;
-          let overloadType = overloadInstance.type;
-          let originalType = instance.type;
-          if (!overloadType.isAssignableTo(originalType)) {
-            this.error(
-              DiagnosticCode.Type_0_is_not_assignable_to_type_1,
-              overloadInstance.identifierNode.range, overloadType.toString(), originalType.toString()
-            );
-            continue;
-          }
-          // TODO: additional optional parameters are not permitted by `isAssignableTo` yet
-          let overloadSignature = overloadInstance.signature;
-          let overloadParameterTypes = overloadSignature.parameterTypes;
-          let overloadNumParameters = overloadParameterTypes.length;
-          let paramExprs = new Array<ExpressionRef>(1 + overloadNumParameters);
-          paramExprs[0] = module.local_get(0, sizeTypeRef); // this
-          for (let n = 1; n <= numParameters; ++n) {
-            paramExprs[n] = module.local_get(n, parameterTypes[n - 1].toRef());
-          }
-          let needsVarargsStub = false;
-          for (let n = numParameters; n < overloadNumParameters; ++n) {
-            // TODO: inline constant initializers and skip varargs stub
-            paramExprs[1 + n] = this.makeZero(overloadParameterTypes[n], overloadInstance.declaration);
-            needsVarargsStub = true;
-          }
-          let calledName = needsVarargsStub
-            ? this.ensureVarargsStub(overloadInstance).internalName
-            : overloadInstance.internalName;
-          let returnTypeRef = overloadSignature.returnType.toRef();
-          let stmts = new Array<ExpressionRef>();
-          if (needsVarargsStub) {
-            // Safe to prepend since paramExprs are local.get's
-            stmts.push(module.global_set(this.ensureArgumentsLength(), module.i32(numParameters)));
-          }
-          if (returnType == Type.void) {
-            stmts.push(
+    var overloadInstances = this.resolver.resolveOverloads(instance);
+    if (overloadInstances) {
+      for (let i = 0, k = overloadInstances.length; i < k; ++i) {
+        let overloadInstance = overloadInstances[i];
+        if (!overloadInstance.is(CommonFlags.COMPILED)) continue; // errored
+        let overloadType = overloadInstance.type;
+        let originalType = instance.type;
+        if (!overloadType.isAssignableTo(originalType)) {
+          this.error(
+            DiagnosticCode.Type_0_is_not_assignable_to_type_1,
+            overloadInstance.identifierNode.range, overloadType.toString(), originalType.toString()
+          );
+          continue;
+        }
+        // TODO: additional optional parameters are not permitted by `isAssignableTo` yet
+        let overloadSignature = overloadInstance.signature;
+        let overloadParameterTypes = overloadSignature.parameterTypes;
+        let overloadNumParameters = overloadParameterTypes.length;
+        let paramExprs = new Array<ExpressionRef>(1 + overloadNumParameters);
+        paramExprs[0] = module.local_get(0, sizeTypeRef); // this
+        for (let n = 1; n <= numParameters; ++n) {
+          paramExprs[n] = module.local_get(n, parameterTypes[n - 1].toRef());
+        }
+        let needsVarargsStub = false;
+        for (let n = numParameters; n < overloadNumParameters; ++n) {
+          // TODO: inline constant initializers and skip varargs stub
+          paramExprs[1 + n] = this.makeZero(overloadParameterTypes[n], overloadInstance.declaration);
+          needsVarargsStub = true;
+        }
+        let calledName = needsVarargsStub
+          ? this.ensureVarargsStub(overloadInstance).internalName
+          : overloadInstance.internalName;
+        let returnTypeRef = overloadSignature.returnType.toRef();
+        let stmts = new Array<ExpressionRef>();
+        if (needsVarargsStub) {
+          // Safe to prepend since paramExprs are local.get's
+          stmts.push(module.global_set(this.ensureArgumentsLength(), module.i32(numParameters)));
+        }
+        if (returnType == Type.void) {
+          stmts.push(
+            module.call(calledName, paramExprs, returnTypeRef)
+          );
+          stmts.push(
+            module.return()
+          );
+        } else {
+          stmts.push(
+            module.return(
               module.call(calledName, paramExprs, returnTypeRef)
-            );
-            stmts.push(
-              module.return()
-            );
-          } else {
-            stmts.push(
-              module.return(
-                module.call(calledName, paramExprs, returnTypeRef)
-              )
-            );
-          }
-          builder.addCase(classInstance.id, stmts);
-          // Also alias each extendee inheriting this exact overload
-          let extendees = classInstance.getAllExtendees(
-            isProperty
-              ? unboundOverloadParent.name
-              : instance.prototype.name
+            )
           );
-          for (let _values = Set_values(extendees), a = 0, b = _values.length; a < b; ++a) {
-            let extendee = _values[a];
-            builder.addCase(extendee.id, stmts);
-          }
+        }
+        let classInstance = assert(overloadInstance.getClassOrInterface());
+        builder.addCase(classInstance.id, stmts);
+        // Also alias each extendee inheriting this exact overload
+        let extendees = classInstance.getAllExtendees(instance.declaration.name.text); // without get:/set:
+        for (let _values = Set_values(extendees), a = 0, b = _values.length; a < b; ++a) {
+          let extendee = _values[a];
+          builder.addCase(extendee.id, stmts);
         }
       }
     }

src/program.ts

@@ -3456,7 +3456,7 @@ export class FunctionPrototype extends DeclaredElement {
   /** Methods overloading this one, if any. These are unbound. */
   overloads: Set<FunctionPrototype> | null = null;
 
-  /** Clones of this prototype that are bounds to specific classes. */
+  /** Clones of this prototype that are bound to specific classes. */
   private boundPrototypes: Map<Class,FunctionPrototype> | null = null;
 
   /** Constructs a new function prototype. */
@@ -3504,11 +3504,9 @@ export class FunctionPrototype extends DeclaredElement {
   /** Tests if this prototype is bound to a class. */
   get isBound(): bool {
     var parent = this.parent;
-    return parent.kind == ElementKind.CLASS ||
-           parent.kind == ElementKind.PROPERTY_PROTOTYPE && (
-             parent.parent.kind == ElementKind.CLASS ||
-             parent.parent.kind == ElementKind.INTERFACE
-           );
+    var parentKind = parent.kind;
+    if (parentKind == ElementKind.PROPERTY_PROTOTYPE) parentKind = parent.parent.kind;
+    return parentKind == ElementKind.CLASS || parentKind == ElementKind.INTERFACE;
   }
 
   /** Creates a clone of this prototype that is bound to a concrete class instead. */
@@ -3661,6 +3659,16 @@ export class Function extends TypedElement {
       : getDefaultParameterName(index);
   }
 
+  /** Gets the class or interface this function belongs to, if an instance method. */
+  getClassOrInterface(): Class | null {
+    var parent = this.parent;
+    if (parent.kind == ElementKind.PROPERTY) parent = parent.parent;
+    if (parent.kind == ElementKind.CLASS || parent.kind == ElementKind.INTERFACE) {
+      return <Class>parent;
+    }
+    return null;
+  }
+
   /** Creates a stub for use with this function, i.e. for varargs or virtual calls. */
   newStub(postfix: string): Function {
     var stub = new Function(

src/resolver.ts

@@ -126,6 +126,8 @@ export class Resolver extends DiagnosticEmitter {
   currentThisExpression: Expression | null = null;
   /** Element expression of the previously resolved element access. */
   currentElementExpression : Expression | null = null;
+  /** Whether a new overload has been discovered. */
+  discoveredOverload: bool = false;
 
   /** Constructs the resolver for the specified program. */
   constructor(
@@ -2757,6 +2759,20 @@ export class Resolver extends DiagnosticEmitter {
       ctxTypes
     );
     prototype.setResolvedInstance(instanceKey, instance);
+
+    // remember discovered overloads for virtual stub finalization
+    if (classInstance) {
+      let methodOrPropertyName = instance.declaration.name.text;
+      let baseClass = classInstance.base;
+      while (baseClass) {
+        let baseMembers = baseClass.members;
+        if (baseMembers && baseMembers.has(methodOrPropertyName)) {
+          this.discoveredOverload = true;
+          break;
+        }
+        baseClass = baseClass.base;
+      }
+    }
     return instance;
   }
 
@@ -2833,6 +2849,59 @@ export class Resolver extends DiagnosticEmitter {
     );
   }
 
+  /** Resolves reachable overloads of the given instance method. */
+  resolveOverloads(instance: Function): Function[] | null {
+    var overloadPrototypes = instance.prototype.overloads;
+    if (!overloadPrototypes) return null;
+
+    var parentClassInstance = assert(instance.getClassOrInterface());
+    var overloads = new Set<Function>();
+
+    // A method's `overloads` property contains its unbound overload prototypes
+    // so we first have to find the concrete classes it became bound to, obtain
+    // their bound prototypes and make sure these are resolved.
+    for (let _values = Set_values(overloadPrototypes), i = 0, k = _values.length; i < k; ++i) {
+      let unboundOverloadPrototype = _values[i];
+      assert(!unboundOverloadPrototype.isBound);
+      let unboundOverloadParent = unboundOverloadPrototype.parent;
+      let isProperty = unboundOverloadParent.kind == ElementKind.PROPERTY_PROTOTYPE;
+      let classInstances: Map<string,Class> | null;
+      if (isProperty) {
+        let propertyParent = (<PropertyPrototype>unboundOverloadParent).parent;
+        assert(propertyParent.kind == ElementKind.CLASS_PROTOTYPE);
+        classInstances = (<ClassPrototype>propertyParent).instances;
+      } else {
+        assert(unboundOverloadParent.kind == ElementKind.CLASS_PROTOTYPE);
+        classInstances = (<ClassPrototype>unboundOverloadParent).instances;
+      }
+      if (!classInstances) continue;
+      for (let _values = Map_values(classInstances), j = 0, l = _values.length; j < l; ++j) {
+        let classInstance = _values[j];
+        // Check if the parent class is a subtype of instance's class
+        if (!classInstance.isAssignableTo(parentClassInstance)) continue;
+        let overloadInstance: Function | null;
+        if (isProperty) {
+          let boundProperty = assert(classInstance.members!.get(unboundOverloadParent.name));
+          assert(boundProperty.kind == ElementKind.PROPERTY_PROTOTYPE);
+          let boundPropertyInstance = this.resolveProperty(<PropertyPrototype>boundProperty);
+          if (!boundPropertyInstance) continue;
+          if (instance.is(CommonFlags.GET)) {
+            overloadInstance = boundPropertyInstance.getterInstance;
+          } else {
+            assert(instance.is(CommonFlags.SET));
+            overloadInstance = boundPropertyInstance.setterInstance;
+          }
+        } else {
+          let boundPrototype = assert(classInstance.members!.get(unboundOverloadPrototype.name));
+          assert(boundPrototype.kind == ElementKind.FUNCTION_PROTOTYPE);
+          overloadInstance = this.resolveFunction(<FunctionPrototype>boundPrototype, instance.typeArguments);
+        }
+        if (overloadInstance) overloads.add(overloadInstance);
+      }
+    }
+    return Set_values(overloads);
+  }
+
   /** Currently resolving classes. */
   private resolveClassPending: Set<Class> = new Set();