Fix accessor vtable dispatch through base-typed references

Closes the "much bigger gap" flagged alongside super-accessor-crash-fix
(PR #278): overriding a get/set accessor pair in a subclass and
accessing it through a BASE-TYPED reference (an ordinary upcast, not
`super`) always resolved statically to the base's own accessor instead
of dispatching virtually to the override - a real correctness gap
(this is standard OOP virtual dispatch, not an edge case), unlike
ordinary methods which already dispatch through the vtable correctly.

Root cause: an accessor's backing get/set functions are ALREADY
registered as ordinary MethodInfo entries in ClassInfo::methods (same
list ordinary methods use) with a real vtable slot assigned via
registerClassMethodMember/getVirtualTable - accessors are virtual by
default exactly like methods. But ClassAccessorAccess never checked
isVirtual at all: it unconditionally built ThisAccessorOp with a
hardcoded FlatSymbolRefAttr naming THIS class's own get/set symbol,
completely bypassing the vtable that was already being correctly
built.

Fix: mirror ClassMethodAccess's own isVirtual/isStorageType check
(isStorageType is true for `super.<accessor>`'s raw-struct thisValue,
so super access is naturally excluded from virtual dispatch, same as
methods) and route through the vtable via VirtualSymbolRefOp (the same
op static virtual method access already uses) feeding into the
existing "indirect" (function-value) ThisIndirectAccessorOp, instead
of the hardcoded-symbol ThisAccessorOp. Checked before the
isDynamicImport branch, matching ClassMethodAccess's ordering - the
vtable's own construction already resolves dynamic-import-owned slots,
so virtual dispatch through it is correct regardless of module
boundaries.

One regression found and fixed during verification: getThisRefOfClass
only narrows `thisValue` for the isSuperClass case, so an ordinary
access whose static type is still a wider union (e.g. `T | null`
narrowed only at the type-info level, never cast at the value level -
hit by 00iterator_bug.ts's spread-operator-generated `.length` access)
reached the new vtable lookup with an un-narrowed union type, which
mlirGenPropertyAccessExpression's PropertyRefOp rejects (unlike the
old AnyType-typed ThisAccessorOp it replaces). Fixed by explicitly
casting to classInfo->classType before the vtable lookup.

New same-file regression test 00class_accessor_virtual.ts (compile +
jit tiers). Also closed the historical NOTE in
import_class_accessor.ts documenting this exact gap as deliberately
untested - now asserts virtual dispatch through a base-typed reference
works, cross-module, across all 3 tiers (plain, -shared compile,
-jit -shared). Full suite: 810/810 green.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: ASDAlexander77

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.");
 }