Fix optional interface method bugs (vtable crash + undefined comparison) and extend test coverage

README.md

@@ -2,6 +2,8 @@
 
 Powered by [![LLVM|MLIR](https://llvm.org/img/LLVM-Logo-Derivative-1.png)](https://llvm.org/)
 
+Developed by [![Claude Code](https://img.shields.io/badge/Claude-Code-D97757?logo=claude&logoColor=white)](https://claude.com/claude-code)
+
 [![Donate](https://img.shields.io/badge/Donate-PayPal-green.svg)](https://www.paypal.com/donate/?hosted_button_id=BBJ4SQYLA6D2L)
 
 A native ahead-of-time (AOT) and JIT compiler for **TypeScript**, built on **LLVM/MLIR**.

tslang/include/TypeScript/LowerToLLVM/UndefLogicHelper.h

@@ -118,6 +118,59 @@ class UndefLogicHelper
                     return LogicOp<StdIOpTy, V1, v1, StdFOpTy, V2, v2>(binOp, opCmpCode, val2, val2.getType(), casted, casted.getType(),
                                                                        rewriter, typeConverter, compileOptions);
                 }
+                else if (isa<mlir_ts::BoundFunctionType>(t2))
+                {
+                    // an optional (`?`) interface/class METHOD compared against
+                    // `undefined` (e.g. `missing.opt == undefined` -
+                    // 00interface_optional_method_extends.ts). Unlike an optional
+                    // FIELD (OptionalType + HasValueOp runtime check), a bound_func has
+                    // no such wrapper, so this used to fall to whenOneValueIsUndef below
+                    // and return a compile-time-constant false/true regardless of the
+                    // actual value - meaning the comparison never reflected whether the
+                    // method was really present. InterfaceSymbolRefOpLowering now selects
+                    // a null `this` pointer when the vtable slot holds the "missing
+                    // optional member" -1 sentinel (a real bound method's `this` is
+                    // never null), so check that instead.
+                    //
+                    // NOTE: deliberately NOT using LogicOp<StdIOpTy, V1, v1, ...> here to
+                    // compute "is this null" - v1 is a template parameter baked in from
+                    // the ORIGINAL comparison operator that triggered this whole call
+                    // (e.g. arith::CmpIPredicate::ne for a source-level `!=`), not
+                    // something the `op`/SyntaxKind argument can override at the call
+                    // site (LogicOp's isIntOrIndex branch ignores `op` entirely and uses
+                    // `v1` directly) - passing SyntaxKind::EqualsEqualsToken here while v1
+                    // is still `ne` silently computed "this != null" instead of "this ==
+                    // null", inverting the result. Emit the LLVM::ICmpOp directly instead,
+                    // pointer-converting both sides the same way LogicOp's
+                    // isNullableTypeNoUnion branch would.
+                    auto thisVal = rewriter.create<mlir_ts::GetThisOp>(loc, mlir_ts::OpaqueType::get(rewriter.getContext()), val2);
+                    auto nullVal = rewriter.create<mlir_ts::NullOp>(loc, mlir_ts::NullType::get(rewriter.getContext()));
+                    LLVMTypeConverterHelper llvmtch(&typeConverter);
+                    auto intPtrType = llvmtch.getIntPtrType(0);
+                    auto thisValAsLLVMType = rewriter.create<mlir_ts::DialectCastOp>(loc, typeConverter.convertType(thisVal.getType()), thisVal);
+                    auto nullValAsLLVMType = rewriter.create<mlir_ts::DialectCastOp>(loc, typeConverter.convertType(nullVal.getType()), nullVal);
+                    auto thisPtrValue = rewriter.create<LLVM::PtrToIntOp>(loc, intPtrType, thisValAsLLVMType);
+                    auto nullPtrValue = rewriter.create<LLVM::PtrToIntOp>(loc, intPtrType, nullValAsLLVMType);
+                    mlir::Value isNull = rewriter.create<LLVM::ICmpOp>(loc, LLVM::ICmpPredicate::eq, thisPtrValue, nullPtrValue);
+
+                    switch (opCmpCode)
+                    {
+                    case SyntaxKind::EqualsEqualsToken:
+                    case SyntaxKind::EqualsEqualsEqualsToken:
+                        return isNull;
+                    case SyntaxKind::ExclamationEqualsToken:
+                    case SyntaxKind::ExclamationEqualsEqualsToken:
+                    {
+                        auto trueVal = clh.createI1ConstantOf(true);
+                        return (mlir::Value)rewriter.create<LLVM::XOrOp>(loc, isNull, trueVal);
+                    }
+                    default:
+                        // ordering comparisons against undefined aren't meaningful for a
+                        // callable - same "result is false already" fallback as any
+                        // other unhandled type below.
+                        return whenOneValueIsUndef(rewriter, loc);
+                    }
+                }
                 else
                 {
                     // result is false already

tslang/lib/TypeScript/LowerToLLVM.cpp

@@ -4880,8 +4880,36 @@ struct InterfaceSymbolRefOpLowering : public TsLlvmPattern<mlir_ts::InterfaceSym
 
         if (auto boundFunc = dyn_cast<mlir_ts::BoundFunctionType>(interfaceSymbolRefOp.getType()))
         {
+            mlir::Value thisValEffective = thisVal;
+            if (isOptional)
+            {
+                // an optional (`?`) interface method the object literal doesn't provide
+                // is patched with a -1 sentinel in the vtable slot (see
+                // mlirGenObjectVirtualTableDefinitionForInterface's missing-method
+                // branch), same convention as the FIELD case below. Select a null
+                // `this` when the slot is that sentinel, so the resulting bound_func
+                // has a null `this` pointer - a real bound method's `this` is never
+                // null - giving a later undefined-comparison something concrete to
+                // check instead of always reading as "present". A branchless
+                // LLVM::SelectOp is used here rather than CodeLogicHelper's
+                // conditionalExpressionLowering (block-splitting control flow) since
+                // this pattern - used successfully elsewhere in this file, e.g.
+                // ValueOrDefaultOpLowering - is simpler and carries no risk of a
+                // malformed/dangling basic block.
+                LLVMTypeConverterHelper llvmtch(static_cast<const LLVMTypeConverter *>(getTypeConverter()));
+                auto intPtrType = llvmtch.getIntPtrType(0);
+                auto negative1 = tsLlvmContext->compileOptions.sizeBits == 32
+                    ? clh.createI32ConstantOf(-1)
+                    : clh.createI64ConstantOf(-1);
+                auto methodOrFieldIntPtrValue = rewriter.create<LLVM::PtrToIntOp>(loc, intPtrType, methodOrFieldPtr);
+                auto isMissing =
+                    rewriter.create<LLVM::ICmpOp>(loc, LLVM::ICmpPredicate::eq, methodOrFieldIntPtrValue, negative1);
+                auto nullThisVal = rewriter.create<LLVM::ZeroOp>(loc, th.getPtrType());
+                thisValEffective = rewriter.create<LLVM::SelectOp>(loc, isMissing, nullThisVal, thisVal);
+            }
+
             auto thisOpaque =
-                rewriter.create<mlir_ts::CastOp>(loc, mlir_ts::OpaqueType::get(rewriter.getContext()), thisVal);
+                rewriter.create<mlir_ts::CastOp>(loc, mlir_ts::OpaqueType::get(rewriter.getContext()), thisValEffective);
             auto methodTypedPtr = rewriter.create<mlir_ts::CastOp>(
                 loc, mlir_ts::FunctionType::get(rewriter.getContext(), boundFunc.getInputs(), boundFunc.getResults()),
                 methodOrFieldPtr);

tslang/lib/TypeScript/MLIRGenInterfaces.cpp

@@ -449,8 +449,29 @@ namespace mlirgen
                                 MLIRHelper::getStructIndex(builder, fieldIndex));
                         }
                     }
+                    else if (methodOrField.isMissing)
+                    {
+                        // an optional (`?`) interface METHOD the object literal doesn't
+                        // provide - same "null value, as missing field/method" placeholder
+                        // as the isField branch above (getStructIndex + -1 sentinel), just
+                        // cast to the method's function-pointer-ref type instead of a field
+                        // type. Reachable via extends (see 00interface_optional_method_extends.ts):
+                        // a required method is always compile-time-resolvable through
+                        // lookupObjectLiteralMethodSymbol/findMethod above and never reaches
+                        // isMissing here; only a genuinely-absent optional method does.
+                        auto negative1 = builder.create<mlir_ts::ConstantOp>(location, builder.getI64Type(),
+                                                                             mth.getI64AttrValue(-1));
+                        auto castedPtr = cast(location, mlir_ts::RefType::get(methodOrField.methodInfo.funcType),
+                                              negative1, genContext);
+                        vtableValue = builder.create<mlir_ts::InsertPropertyOp>(
+                            location, virtTuple, castedPtr, vtableValue,
+                            MLIRHelper::getStructIndex(builder, fieldIndex));
+                    }
                     else
                     {
+                        // a real, present, non-object-literal method (e.g. a class
+                        // implementing the interface) reaching the METHOD (not
+                        // methodsAsFields) branch - not yet exercised by any test.
                         llvm_unreachable("not implemented yet");
                         /*
                         auto methodConstName = builder.create<mlir_ts::SymbolRefOp>(

tslang/test/tester/CMakeLists.txt

@@ -293,6 +293,7 @@ add_test(NAME test-compile-00-object-annotated-method-interleaved COMMAND test-r
 add_test(NAME test-compile-00-object-annotated-method-extends-interface COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_extends_interface.ts")
 add_test(NAME test-compile-00-object-annotated-method-extends-interface-multilevel COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_extends_interface_multilevel.ts")
 add_test(NAME test-compile-00-interface-optional-extends COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_optional_extends.ts")
+add_test(NAME test-compile-00-interface-optional-method-extends COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_optional_method_extends.ts")
 add_test(NAME test-compile-00-interface-object-array COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_object_array.ts")
 add_test(NAME test-compile-00-interface-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_generic.ts")
 add_test(NAME test-compile-00-interface-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_new.ts")
@@ -653,6 +654,7 @@ add_test(NAME test-jit-00-object-annotated-method-interleaved COMMAND test-runne
 add_test(NAME test-jit-00-object-annotated-method-extends-interface COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_extends_interface.ts")
 add_test(NAME test-jit-00-object-annotated-method-extends-interface-multilevel COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_extends_interface_multilevel.ts")
 add_test(NAME test-jit-00-interface-optional-extends COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_optional_extends.ts")
+add_test(NAME test-jit-00-interface-optional-method-extends COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_optional_method_extends.ts")
 add_test(NAME test-jit-00-interface-object-array COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_object_array.ts")
 add_test(NAME test-jit-00-interface-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_generic.ts")
 add_test(NAME test-jit-00-interface-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_new.ts")
@@ -871,6 +873,7 @@ add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-
 add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface-multilevel COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts")
 add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface-diamond COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts")
 add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface-triple COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_triple.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_triple.ts")
+add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface-optional COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_optional.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_optional.ts")
 add_test(NAME test-compile-shared-export-import-vars COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars.ts")
 add_test(NAME test-compile-shared-export-import-vars-2 COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars2.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars2.ts")
 add_test(NAME test-compile-shared-export-import-enum COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_enum.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_enum.ts")
@@ -896,6 +899,7 @@ add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-exte
 add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface-multilevel COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts")
 add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface-diamond COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts")
 add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface-triple COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_triple.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_triple.ts")
+add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface-optional COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_optional.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_optional.ts")
 add_test(NAME test-jit-shared-export-import-vars COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars.ts")
 add_test(NAME test-jit-shared-export-import-vars-2 COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars2.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars2.ts")
 add_test(NAME test-jit-shared-export-import-enum COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_enum.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_enum.ts")

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

@@ -0,0 +1,67 @@
+// Sibling of 00interface_optional_extends.ts (optional FIELD inherited via
+// extends), but for an optional METHOD instead. Methods go through a
+// different vtable-patch path than data fields
+// (mlirGenCreateInterfaceVTableForObject clones/patches function pointers,
+// not byte offsets), so a fix proven for optional fields isn't automatically
+// proven for optional methods.
+//
+// Two bugs found and fixed here:
+//
+// 1. Casting an object literal that OMITS an optional method inherited via
+// extends used to crash the compiler outright
+// (llvm_unreachable("not implemented yet") in
+// mlirGenObjectVirtualTableDefinitionForInterface - building the placeholder
+// vtable for a missing METHOD had never been implemented at all, only for a
+// missing FIELD). Fixed by mirroring the missing-field's -1-sentinel
+// placeholder pattern for a missing method's vtable slot.
+//
+// 2. Comparing an optional interface METHOD against `undefined`
+// (`someObj.optMethod == undefined`) never worked for ANY interface, extends
+// or not - hardcoded to a compile-time-constant result in
+// UndefLogicHelper.h's processUndefVale, which only special-cased
+// InterfaceType/ClassType against undefined, not BoundFunctionType. Fixed by:
+// (a) InterfaceSymbolRefOpLowering now selects a null `this` pointer
+// (branchless LLVM::SelectOp) when an optional method's vtable slot holds the
+// "missing" -1 sentinel - a real bound method's `this` is never null; (b)
+// UndefLogicHelper.h's new BoundFunctionType branch checks that null-or-not
+// via a directly-emitted LLVM::ICmpOp - NOT the shared LogicOp<...> helper,
+// whose comparison predicate is a template parameter baked in from the
+// OUTER comparison operator that triggered the whole call (passing a
+// different SyntaxKind at the call site is silently ignored and inverts the
+// result).
+
+function main() {
+    interface Base {
+        base: number;
+        opt?(n: number): number;
+    }
+
+    interface Derived extends Base {
+        derived: number;
+    }
+
+    let present: Derived = <Derived>{
+        base: 1.0,
+        opt(n: number) { return this.base + n; },
+        derived: 10.0,
+    };
+    let missing: Derived = <Derived>{ base: 2.0, derived: 20.0 };
+
+    assert(present.base == 1);
+    assert(present.opt != undefined);
+    assert(present.opt(4) == 5);
+    assert(present.derived == 10);
+
+    assert(missing.base == 2);
+    assert(missing.opt == undefined);
+    assert(missing.derived == 20);
+
+    // re-read the providing object's method after the non-providing one was
+    // cast, to catch the shared-virtualIndex-clobber bug class if it
+    // resurfaces here
+    print(present.opt(9));
+    assert(present.opt(9) == 10);
+    assert(present.opt != undefined);
+
+    print("done.");
+}

tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_optional.ts

@@ -0,0 +1,28 @@
+namespace M4 {
+    export interface Base {
+        base: number;
+        opt?: number;
+    }
+
+    export interface Derived extends Base {
+        derived: number;
+    }
+
+    export var rawPresent: {
+        base: number;
+        opt: number;
+        derived: number;
+    } = {
+        base: 1.0,
+        opt: 5.0,
+        derived: 10.0,
+    };
+
+    export var rawMissing: {
+        base: number;
+        derived: number;
+    } = {
+        base: 2.0,
+        derived: 20.0,
+    };
+}

tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_optional.ts

@@ -0,0 +1,17 @@
+import './export_object_literal_structural_typed_extends_interface_optional'
+
+let present: M4.Derived = <M4.Derived>M4.rawPresent;
+let missing: M4.Derived = <M4.Derived>M4.rawMissing;
+
+assert(present.base == 1);
+assert(present.opt == 5);
+assert(present.derived == 10);
+
+assert(missing.base == 2);
+assert(missing.opt == undefined);
+assert(missing.derived == 20);
+
+print(present.opt);
+assert(present.opt == 5);
+
+print("done.");