Fix accessor vtable dispatch through base-typed references

tslang/lib/TypeScript/MLIRGenAccessCall.cpp

@@ -592,6 +592,100 @@ namespace mlirgen
         {
             auto effectiveThisValue = getThisRefOfClass(location, classInfo->classType, thisValue, isSuperClass, genContext);
 
+            // Accessor vtable dispatch (see accessor-vtable-dispatch-fix memory): overriding
+            // a get/set pair in a subclass and accessing it through a BASE-TYPED reference
+            // (an ordinary upcast, not `super`) used to always resolve statically to the
+            // base's own accessor - accessors were never part of the vtable dispatch
+            // decision at all, even though their backing get/set functions are ALREADY
+            // registered as ordinary (virtual-by-default) MethodInfo entries with real
+            // vtable slots via registerClassMethodMember/getVirtualTable, exactly like
+            // regular methods. Mirror ClassMethodAccess's own isVirtual/isStorageType check
+            // (isStorageType is true for `super.<accessor>`'s raw-struct thisValue, so this
+            // naturally excludes super access from virtual dispatch, same as methods) and
+            // route through the vtable via VirtualSymbolRefOp + the existing "indirect"
+            // (function-value) ThisIndirectAccessorOp, instead of hardcoding this class's
+            // own get/set symbol names. Checked BEFORE the isDynamicImport branch below,
+            // same ordering as ClassMethodAccess - the vtable's own construction already
+            // resolves dynamic-import-owned slots (mlirGenClassVirtualTableDefinition's
+            // isOwnedByDynamicImport handling), so virtual dispatch through it is correct
+            // regardless of module boundaries, without needing separate dlsym resolution.
+            auto isStorageType = isa<mlir_ts::ClassStorageType>(thisValue.getType());
+
+            auto findMethodInfo = [&](FunctionEntry funcEntry) -> const MethodInfo * {
+                if (!funcEntry)
+                {
+                    return nullptr;
+                }
+
+                auto it = llvm::find_if(classInfo->methods,
+                    [&](auto &m) { return m.funcName == funcEntry.name; });
+                return it != classInfo->methods.end() ? &*it : nullptr;
+            };
+
+            auto getMethodInfo = findMethodInfo(getFunc);
+            auto setMethodInfo = findMethodInfo(setFunc);
+
+            auto isVirtualAccessor = !isStorageType &&
+                ((getMethodInfo && (getMethodInfo->isAbstract || getMethodInfo->isVirtual)) ||
+                 (setMethodInfo && (setMethodInfo->isAbstract || setMethodInfo->isVirtual)));
+
+            if (isVirtualAccessor)
+            {
+                // effectiveThisValue can still carry a wider static type here (e.g. a
+                // union like `T | null` that the caller narrowed at the type-info level to
+                // find classInfo, but never actually cast the VALUE to) - getThisRefOfClass
+                // only narrows for the isSuperClass branch, a no-op otherwise.
+                // mlirGenPropertyAccessExpression's vtable lookup needs a genuine class
+                // reference, unlike the plain-AnyType ThisAccessorOp this branch replaces,
+                // so cast explicitly first (same narrowing getThisRefOfClass already does
+                // for super).
+                CAST_A(vtableThisValue, location, classInfo->classType, effectiveThisValue, genContext);
+
+                auto vtableAccess =
+                    mlirGenPropertyAccessExpression(location, vtableThisValue, VTABLE_NAME, genContext);
+
+                if (!vtableAccess)
+                {
+                    emitError(location, "") << "class '" << classInfo->fullName << "' missing 'virtual table'";
+                }
+
+                EXIT_IF_FAILED_OR_NO_VALUE(vtableAccess)
+
+                auto resolveVirtualAccessorFunc = [&](FunctionEntry funcEntry, const MethodInfo *methodInfo) -> mlir::Value {
+                    if (!funcEntry)
+                    {
+                        return mlir::Value();
+                    }
+
+                    assert(genContext.allowPartialResolve || methodInfo->virtualIndex >= 0);
+
+                    return builder.create<mlir_ts::VirtualSymbolRefOp>(
+                        location, funcEntry.funcType, vtableAccess,
+                        builder.getI32IntegerAttr(methodInfo->virtualIndex),
+                        mlir::FlatSymbolRefAttr::get(builder.getContext(), funcEntry.name));
+                };
+
+                auto getterValue = resolveVirtualAccessorFunc(getFunc, getMethodInfo);
+                auto setterValue = resolveVirtualAccessorFunc(setFunc, setMethodInfo);
+
+                auto opaqueThisType = mlir_ts::OpaqueType::get(builder.getContext());
+                if (!getterValue)
+                {
+                    getterValue = builder.create<mlir_ts::UndefOp>(
+                        location, mlir_ts::FunctionType::get(builder.getContext(), {opaqueThisType}, {accessorResultType}, false));
+                }
+
+                if (!setterValue)
+                {
+                    setterValue = builder.create<mlir_ts::UndefOp>(
+                        location, mlir_ts::FunctionType::get(builder.getContext(), {opaqueThisType, accessorResultType}, {}, false));
+                }
+
+                auto thisIndirectAccessorOp = builder.create<mlir_ts::ThisIndirectAccessorOp>(
+                    location, accessorResultType, vtableThisValue, getterValue, setterValue, mlir::Value());
+                return thisIndirectAccessorOp.getResult(0);
+            }
+
             if (classInfo->isDynamicImport)
             {
                 // A plain FlatSymbolRefAttr (the path below) lowers to a call the static linker

tslang/test/tester/CMakeLists.txt

@@ -236,6 +236,7 @@ add_test(NAME test-compile-00-class-discover-types COMMAND test-runner "${PROJEC
 add_test(NAME test-compile-00-class-accessor COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor.ts")
 add_test(NAME test-compile-00-class-accessor-2 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor2.ts")
 add_test(NAME test-compile-00-class-accessor-super COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor_super.ts")
+add_test(NAME test-compile-00-class-accessor-virtual COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor_virtual.ts")
 add_test(NAME test-compile-00-class-super COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_super.ts")
 add_test(NAME test-compile-00-class-super-static COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_super_static.ts")
 add_test(NAME test-compile-00-class-default-constructor COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_def_constr.ts")
@@ -599,6 +600,7 @@ add_test(NAME test-jit-00-class-discover-types COMMAND test-runner -jit "${PROJE
 add_test(NAME test-jit-00-class-accessor COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor.ts")
 add_test(NAME test-jit-00-class-accessor-2 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor2.ts")
 add_test(NAME test-jit-00-class-accessor-super COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor_super.ts")
+add_test(NAME test-jit-00-class-accessor-virtual COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor_virtual.ts")
 add_test(NAME test-jit-00-class-super COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_super.ts")
 add_test(NAME test-jit-00-class-super-static COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_super_static.ts")
 add_test(NAME test-jit-00-class-default-constructor COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_def_constr.ts")

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

@@ -0,0 +1,39 @@
+// Regression test for the accessor-vtable-dispatch-fix: overriding a
+// get/set accessor pair in a subclass and accessing it through a
+// BASE-TYPED reference (an ordinary upcast, not `super`) used to always
+// resolve statically to the base's own accessor - accessors were never
+// part of the vtable dispatch decision at all, unlike ordinary methods.
+class Base {
+    protected _val: number = 10;
+
+    get val(): number {
+        return this._val;
+    }
+
+    set val(v: number) {
+        this._val = v;
+    }
+}
+
+class Derived extends Base {
+    get val(): number {
+        return this._val * 2;
+    }
+
+    set val(v: number) {
+        this._val = v + 1;
+    }
+}
+
+function main() {
+    const d = new Derived();
+    const asBase: Base = d;
+
+    asBase.val = 5;
+    assert(asBase.val == 12);
+
+    // and through the derived-typed reference directly, for good measure
+    assert(d.val == 12);
+
+    print("done.");
+}

tslang/test/tester/tests/import_class_accessor.ts

@@ -20,12 +20,17 @@ function main() {
     assert(t.celsius == 100);
     assert(t.fahrenheit == 212);
 
-    // NOTE: accessing an overridden accessor through a base-typed reference
-    // (`const asBase: M.Temperature = t; asBase.celsius`) is a known,
-    // pre-existing, non-cross-module-specific gap: get/set accessors are not
-    // part of the vtable, so such access resolves statically to the base's
-    // own accessor rather than dispatching virtually. Not exercised here -
-    // this test only covers the super-accessor-call crash fix.
+    // Accessing an overridden accessor through a base-typed reference used to
+    // be a known, pre-existing, non-cross-module-specific gap: get/set
+    // accessors were not part of the vtable, so such access resolved
+    // statically to the base's own accessor rather than dispatching
+    // virtually (see accessor-vtable-dispatch-fix memory). Fixed - this now
+    // dispatches to ClampedTemperature's override even through the
+    // M.Temperature-typed reference, cross-module.
+    const asBase: M.Temperature = t;
+    asBase.celsius = -5;
+    assert(asBase.celsius == 0);
+    assert(asBase.fahrenheit == 32);
 
     print("done.");
 }