@cpp overloads take a class for T | null and for its base class, in a partial resolve too

TSLANG_ISSUES.md

@@ -24,6 +24,7 @@ https://github.com/ASDAlexander77/native_ts_graphics
 | 16 | `u64 %` goes through f64 (wrong above 2^53); `u64 /` has no integer division | - | `%` **fixed** by #571; `/` stays a number, as in JavaScript |
 | 17 | Storing through `Ref<u8>` indexing (`bytes[i] = x`) is rejected: "saving to constant object" | - | **answered**: `Deref(bytes[i]) = x` stores; the error says so since #573 |
 | 18 | `ns.f()` resolves to a local variable named `f` declared earlier in the function: "not a function to call" | - | **fixed** by #569 (pre-alpha100: open) |
+| 19 | A @cpp overload call rejects a class argument for a `T \| null` or base-class parameter; a call whose argument the first overload can't take fails to compile | - | **fixed** (this PR; pre-alpha101: open) |
 
 Verified downstream: native_ts_graphics dropped its 71 forwarding constructors (PR #49) and rebuilt
 everything with pre-alpha97 (all objects deleted first): Windows build clean, CTest 61/61, Android
@@ -291,6 +292,56 @@ the bare name `value`. Workaround: name the local differently.
 **Fixed by #569**: the member is looked up in the namespace first. A name the namespace doesn't have
 still falls back to the general lookup, so `ns.missing` can still find a local `missing`.
 
+### 19. A @cpp overload call rejects a class for `T | null`, and checks the first overload's parameters
+
+Found on pre-alpha101 in native_ts_graphics' `core/nvrhi.ts`, after tsbindgen started writing
+NVRHI's overloads under one name: `device.createGraphicsPipeline(Ref(desc), framebuffer)` with
+`framebuffer: IFramebuffer` fits neither overload, `(Ref<GraphicsPipelineDesc>, Ref<FramebufferInfo>)`
+nor `(Ref<GraphicsPipelineDesc>, IFramebuffer | null)`:
+
+```ts
+@cpp
+class Shape {
+    @slot(1) area(): s32;
+}
+
+@cpp
+class Square extends Shape {
+    @slot(2) side(): s32;
+}
+
+@cpp
+class Canvas {
+    @slot(1) draw(x: s32): s32;
+    @slot(2) draw(shape: Shape | null): s32;
+}
+
+// canvas: Canvas, shape: Shape, square: Square
+canvas.draw(shape);    // error: invalid cast from Shape to s32
+```
+
+```
+error: no overload of 'createGraphicsPipeline' takes these arguments (Reference<...>, nvrhi.IFramebuffer); it has:
+  createGraphicsPipeline(Reference<...>, Reference<...>)
+  createGraphicsPipeline(Reference<...>, nvrhi.IFramebuffer | null)
+```
+
+Two causes:
+- an argument fit a parameter only by its own type, a number conversion or `canCastFromTo` (tuples
+  and interfaces): a `Shape` didn't fit `Shape | null`, nor a `Square` a `Shape`;
+- the overload was picked in the full pass only. The partial resolve before it (finding the
+  function's types) called the first overload with the arguments, and an argument it can't be cast
+  to (`Shape` to `s32` above) failed the function. The NVRHI call gave the "no overload" error
+  instead because a class does cast to a `Ref<T>`.
+
+Workaround: pass the argument through a local of the parameter's type
+(`const arg: IFramebuffer | null = framebuffer;`).
+
+**Fixed** (this PR): a class fits its own type and a `T | null` holding it (3, the same C++
+pointer) and a base class, alone or in a union (2, a derived-to-base conversion). The overload is
+picked in a partial resolve too; there a call that fits none (or two) keeps the first overload
+without an error, and the full pass reports it.
+
 ---
 
 ## 8. A union of literals in arithmetic (found live on pre-alpha97)

docs/cpp-classes.md

@@ -103,7 +103,9 @@ Writing one by hand:
 - **Overloads.** Declare each C++ overload under the C++ name, with its own `@slot` (or
   `@linkname` for a non-virtual one). A call picks the overload from its arguments, at compile
   time as C++ does: an argument of the parameter's own type fits best, then an integer for an
-  integer or a float for a float, then any other conversion. A call that fits none, or fits two
+  integer or a float for a float, or a class derived from the parameter's class, then any other
+  conversion. A class is a C++ pointer, so `T` and `T | null` fit a `T | null` parameter alike. A
+  call that fits none, or fits two
   as well, is an error that lists the overloads. MSVC groups overloads in the vtable, in reverse
   declaration order: take the numbers from the layout report (tsbindgen does).
 

tslang/lib/TypeScript/MLIRGenAccessCall.cpp

@@ -1707,13 +1707,11 @@ namespace mlirgen
             return mlir::success();
         }
 
-        if (!genContext.allowPartialResolve)
+        // in a partial resolve too: the first overload's parameters can't take another's arguments
+        actualFuncRefValue = resolveCppOverload(location, actualFuncRefValue, operands, genContext);
+        if (!actualFuncRefValue)
         {
-            actualFuncRefValue = resolveCppOverload(location, actualFuncRefValue, operands, genContext);
-            if (!actualFuncRefValue)
-            {
-                return mlir::failure();
-            }
+            return mlir::failure();
         }
 
         return mlirGenCall(location, actualFuncRefValue, operands, genContext);
@@ -1811,8 +1809,22 @@ namespace mlirgen
             return funcRefValue;
         }
 
+        // whether derivedClass is baseClass or derives from it
+        std::function<bool(mlir_ts::ClassType, mlir_ts::ClassType)> isOrDerivesFrom =
+            [&](mlir_ts::ClassType derivedClass, mlir_ts::ClassType baseClass) {
+                if (derivedClass == baseClass)
+                {
+                    return true;
+                }
+
+                auto derivedInfo = getClassInfoByFullName(derivedClass.getName().getValue());
+                return derivedInfo && llvm::any_of(derivedInfo->baseClasses, [&](ClassInfo::TypePtr base) {
+                           return isOrDerivesFrom(base->classType, baseClass);
+                       });
+            };
+
         // how well an argument fits a parameter: 3 its type, 2 an integer for an integer or a float
-        // for a float, 1 any other conversion, 0 none
+        // for a float, or a derived class for a base class, 1 any other conversion, 0 none
         auto fit = [&](mlir::Type argType, mlir::Type paramType) -> int {
             if (argType == paramType)
             {
@@ -1828,6 +1840,30 @@ namespace mlirgen
                 return 3;
             }
 
+            // a class is a C++ pointer, and `T | null` the same pointer: a `T` fits it as `T` does,
+            // and a class derived from T as a derived-to-base conversion
+            if (auto argClass = dyn_cast<mlir_ts::ClassType>(baseType))
+            {
+                auto classFit = [&](mlir::Type type) {
+                    auto paramClass = dyn_cast<mlir_ts::ClassType>(type);
+                    return !paramClass ? 0 : argClass == paramClass ? 3 : isOrDerivesFrom(argClass, paramClass) ? 2 : 0;
+                };
+
+                auto classBest = classFit(paramType);
+                if (auto unionType = dyn_cast<mlir_ts::UnionType>(paramType))
+                {
+                    for (auto member : unionType.getTypes())
+                    {
+                        classBest = std::max(classBest, classFit(member));
+                    }
+                }
+
+                if (classBest)
+                {
+                    return classBest;
+                }
+            }
+
             // an integer for an integer, a float for a float, before a conversion between the two
             // (C++ ranks a promotion or an integral conversion above a floating-integral one)
             auto isFloat = [&](mlir::Type type) { return isDouble(type) || isa<mlir::FloatType>(type); };
@@ -1907,6 +1943,12 @@ namespace mlirgen
 
         if (best.empty() || best.size() > 1)
         {
+            // a partial resolve may not know the arguments' types yet: the full one reports it
+            if (genContext.allowPartialResolve)
+            {
+                return funcRefValue;
+            }
+
             auto diagnostic = emitError(location);
             diagnostic << (best.empty() ? "no overload of '" : "the call of '") << name
                        << (best.empty() ? "' takes these arguments (" : "' is ambiguous: its arguments (");

tslang/test/tester/tests/00cpp_overloads.ts

@@ -17,6 +17,26 @@ class Derived extends Base {
     @slot(5) write(s: string): s32;       // an overload of its own
 }
 
+// classes are C++ pointers: a class fits its own type and `T | null` alike, and a derived class
+// fits its base as a derived-to-base conversion, below an exact fit
+@cpp
+class Shape {
+    @slot(1) area(): s32;
+}
+
+@cpp
+class Square extends Shape {
+    @slot(2) side(): s32;
+}
+
+@cpp
+class Canvas {
+    @slot(1) draw(x: s32): s32;
+    @slot(2) draw(shape: Shape | null): s32;
+    @slot(3) fill(shape: Square): s32;
+    @slot(4) fill(shape: Shape): s32;
+}
+
 type ObjectData = [vtable: Opaque, value: s32];
 
 function slot0(self: Opaque): s32 { return -1; }
@@ -26,6 +46,11 @@ function slot3(self: Opaque, x: s32, y: s32): s32 { return 300 + x * y; }
 function slot4(self: Opaque): s32 { return Deref(<Ref<ObjectData>>self).value; }
 function slot5(self: Opaque, s: string): s32 { return 500 + s.length; }
 
+function canvasDrawInt(self: Opaque, x: s32): s32 { return 1000 + x; }
+function canvasDrawShape(self: Opaque, shape: Opaque): s32 { return 2000; }
+function canvasFillSquare(self: Opaque, shape: Opaque): s32 { return 3000; }
+function canvasFillShape(self: Opaque, shape: Opaque): s32 { return 4000; }
+
 function main() {
     let vtable: Opaque[] = [<Opaque>slot0, <Opaque>slot1, <Opaque>slot2, <Opaque>slot3, <Opaque>slot4, <Opaque>slot5];
     let data: ObjectData = [<Opaque>Ref(vtable[0]), 7];
@@ -46,5 +71,19 @@ function main() {
     assert(derived.write("abc") == 503, "the derived class's own overload");
     assert(derived.write(2, 3) == 306, "another base overload");
 
+    let canvasVtable: Opaque[] = [<Opaque>slot0, <Opaque>canvasDrawInt, <Opaque>canvasDrawShape,
+        <Opaque>canvasFillSquare, <Opaque>canvasFillShape];
+    let canvasData: ObjectData = [<Opaque>Ref(canvasVtable[0]), 0];
+    const canvas = <Canvas><Opaque>Ref(canvasData);
+    const shape = <Shape><Opaque>Ref(data);
+    const square = <Square><Opaque>Ref(data);
+    const maybeShape: Shape | null = shape;
+    assert(canvas.draw(3) == 1003, "an integer: draw(s32)");
+    assert(canvas.draw(maybeShape) == 2000, "a Shape | null: draw(Shape | null)");
+    assert(canvas.draw(shape) == 2000, "a Shape: draw(Shape | null)");
+    assert(canvas.draw(square) == 2000, "a Square: draw(Shape | null), derived to base");
+    assert(canvas.fill(square) == 3000, "a Square: fill(Square) over fill(Shape)");
+    assert(canvas.fill(shape) == 4000, "a Shape: fill(Shape)");
+
     print("done.");
 }