Skip to content

Commit bf0ece3

Browse files
ASDAlexander77ASDAlexander77
committed
Fix cross-module super-accessor crash and add class coverage
super.<accessor> (get or set) crashed MLIR verification because ClassAccessorAccess never repaired the `this` operand via getThisRefOfClass the way ClassMethodAccess does, leaving a raw by-value ClassStorageType struct where a pointer was expected. Not cross-module-specific - fixed by threading isSuperClass through ClassAccessorAccess, with a same-file regression test added alongside the cross-module one that surfaced it. Also adds cross-module (export/import) test coverage for abstract classes and static members, both passing cleanly with no changes needed.
1 parent 6241930 commit bf0ece3

12 files changed

Lines changed: 319 additions & 6 deletions

‎tslang/lib/TypeScript/MLIRGenAccessCall.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -537,8 +537,9 @@ namespace mlirgen
537537
}
538538
}
539539

540-
mlir::Value MLIRGenImpl::ClassAccessorAccess(ClassInfo::TypePtr classInfo,
541-
mlir::Location location, mlir::Value thisValue, int accessorIndex, mlir_ts::AccessLevel accessingFromLevel, const GenContext &genContext)
540+
mlir::Value MLIRGenImpl::ClassAccessorAccess(ClassInfo::TypePtr classInfo,
541+
mlir::Location location, mlir::Value thisValue, int accessorIndex,
542+
bool isSuperClass, mlir_ts::AccessLevel accessingFromLevel, const GenContext &genContext)
542543
{
543544

544545
auto accessorInfo = classInfo->accessors[accessorIndex];
@@ -589,8 +590,9 @@ namespace mlirgen
589590
}
590591
else
591592
{
593+
auto effectiveThisValue = getThisRefOfClass(location, classInfo->classType, thisValue, isSuperClass, genContext);
592594
auto thisAccessorOp = builder.create<mlir_ts::ThisAccessorOp>(
593-
location, accessorResultType, thisValue,
595+
location, accessorResultType, effectiveThisValue,
594596
getFunc ? mlir::FlatSymbolRefAttr::get(builder.getContext(), getFunc.name)
595597
: mlir::FlatSymbolRefAttr{},
596598
setFunc ? mlir::FlatSymbolRefAttr::get(builder.getContext(), setFunc.name)
@@ -736,7 +738,7 @@ namespace mlirgen
736738
auto accessorIndex = classInfo->getAccessorIndex(name);
737739
if (accessorIndex >= 0)
738740
{
739-
return ClassAccessorAccess(classInfo, location, thisValue, accessorIndex, accessingFromLevel, genContext);
741+
return ClassAccessorAccess(classInfo, location, thisValue, accessorIndex, isSuperClass, accessingFromLevel, genContext);
740742
}
741743

742744
for (auto [index, baseClass] : enumerate(classInfo->baseClasses))

‎tslang/lib/TypeScript/MLIRGenImpl.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5522,8 +5522,9 @@ class MLIRGenImpl
55225522
mlir::Location location, mlir::Value thisValue, int genericMethodIndex,
55235523
bool isSuperClass, mlir_ts::AccessLevel accessingFromLevel, const GenContext &genContext);
55245524

5525-
mlir::Value ClassAccessorAccess(ClassInfo::TypePtr classInfo,
5526-
mlir::Location location, mlir::Value thisValue, int accessorIndex, mlir_ts::AccessLevel accessingFromLevel, const GenContext &genContext);
5525+
mlir::Value ClassAccessorAccess(ClassInfo::TypePtr classInfo,
5526+
mlir::Location location, mlir::Value thisValue, int accessorIndex,
5527+
bool isSuperClass, mlir_ts::AccessLevel accessingFromLevel, const GenContext &genContext);
55275528

55285529
// TODO: why isSuperClass is not used here?
55295530
mlir::Value ClassIndexAccess(ClassInfo::TypePtr classInfo,

‎tslang/test/tester/CMakeLists.txt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ add_test(NAME test-compile-00-class-static-block COMMAND test-runner "${PROJECT_
235235
add_test(NAME test-compile-00-class-discover-types COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_discover_types.ts")
236236
add_test(NAME test-compile-00-class-accessor COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor.ts")
237237
add_test(NAME test-compile-00-class-accessor-2 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor2.ts")
238+
add_test(NAME test-compile-00-class-accessor-super COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor_super.ts")
238239
add_test(NAME test-compile-00-class-super COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_super.ts")
239240
add_test(NAME test-compile-00-class-super-static COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_super_static.ts")
240241
add_test(NAME test-compile-00-class-default-constructor COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_def_constr.ts")
@@ -596,6 +597,7 @@ add_test(NAME test-jit-00-class-static-block COMMAND test-runner -jit "${PROJECT
596597
add_test(NAME test-jit-00-class-discover-types COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_discover_types.ts")
597598
add_test(NAME test-jit-00-class-accessor COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor.ts")
598599
add_test(NAME test-jit-00-class-accessor-2 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor2.ts")
600+
add_test(NAME test-jit-00-class-accessor-super COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_accessor_super.ts")
599601
add_test(NAME test-jit-00-class-super COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_super.ts")
600602
add_test(NAME test-jit-00-class-super-static COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_super_static.ts")
601603
add_test(NAME test-jit-00-class-default-constructor COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_def_constr.ts")
@@ -850,6 +852,23 @@ add_test(NAME test-compile-export-import-class-interface COMMAND test-runner "${
850852
add_test(NAME test-compile-export-import-class-extends COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts")
851853
add_test(NAME test-compile-export-import-class-extends-multilevel COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_multilevel.ts")
852854
add_test(NAME test-compile-export-import-class-extends-implements-diamond COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_implements_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_implements_diamond.ts")
855+
add_test(NAME test-compile-export-import-class-abstract COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_abstract.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_abstract.ts")
856+
add_test(NAME test-compile-export-import-class-static COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_static.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_static.ts")
857+
# Cross-module generic class instantiation: DISABLED, known issue found 2026-07-22. A generic
858+
# class specialization instantiated in the IMPORTING module incorrectly inherits `export`
859+
# (dllexport) linkage from the original `export class` declaration, even though the
860+
# specialization is defined fresh, locally, in this module. For a multi-type-param generic
861+
# (Pair<A,B>) the resulting mangled name contains a raw comma
862+
# (M.Pair<!ts.number,!ts.string>..instanceOf), which is a linker-directive metacharacter -
863+
# lld rejects it with "invalid /export:". Separately, the -shared tiers fail even earlier:
864+
# once a compiled .dll/.lib for the exporting module already exists next to its .ts source,
865+
# import resolution reads declarations back from that binary artifact instead of
866+
# reparsing source, and that path has no representation for an uninstantiated generic
867+
# template at all, so the whole namespace (M) fails to resolve ("can't resolve name: M").
868+
# Re-enable once both are fixed; test files are kept in the tree (export_class_generic.ts /
869+
# import_class_generic.ts) for that follow-up.
870+
# add_test(NAME test-compile-export-import-class-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_generic.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_generic.ts")
871+
add_test(NAME test-compile-export-import-class-accessor COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_accessor.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_accessor.ts")
853872

854873
# shared libs tests (dlls/compile-time)
855874
add_test(NAME test-compile-time-shared-decl-emit-type COMMAND test-runner -shared -compile-time "${PROJECT_SOURCE_DIR}/test/tester/tests/emit_compiletime_func.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/decl_type.ts")
@@ -878,6 +897,16 @@ add_test(NAME test-compile-shared-export-import-object-literal-with-class-types
878897
add_test(NAME test-compile-shared-export-import-class-extends COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts")
879898
add_test(NAME test-compile-shared-export-import-class-extends-implements-diamond COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_implements_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_implements_diamond.ts")
880899
add_test(NAME test-compile-shared-export-import-class-extends-multilevel COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_multilevel.ts")
900+
add_test(NAME test-compile-shared-export-import-class-abstract COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_abstract.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_abstract.ts")
901+
add_test(NAME test-compile-shared-export-import-class-static COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_static.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_static.ts")
902+
# DISABLED: see the matching test-compile-export-import-class-generic comment above (known issue, 2026-07-22).
903+
# add_test(NAME test-compile-shared-export-import-class-generic COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_generic.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_generic.ts")
904+
# DISABLED: known issue found 2026-07-22 (separate from the super-accessor-call crash fixed
905+
# above). Under the -shared dynamic-import path, a base class's get/set accessors are not
906+
# resolvable at all from a derived class in another module ("Class member 'celsius' can't be
907+
# found") - the same class of gap as the disabled generic-class tests: declaration
908+
# reconstruction for a dynamically-imported class is incomplete for some member kinds.
909+
# add_test(NAME test-compile-shared-export-import-class-accessor COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_accessor.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_accessor.ts")
881910
add_test(NAME test-compile-shared-export-import-object-literal-with-interface COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_interface.ts")
882911
add_test(NAME test-compile-shared-export-import-object-literal-untyped COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_untyped.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_untyped.ts")
883912
add_test(NAME test-compile-shared-export-import-object-literal-untyped-multi-method COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_untyped_multi_method.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_untyped_multi_method.ts")
@@ -907,6 +936,12 @@ add_test(NAME test-jit-shared-export-import-object-literal-with-class-types COMM
907936
add_test(NAME test-jit-shared-export-import-class-extends COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts")
908937
add_test(NAME test-jit-shared-export-import-class-extends-multilevel COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_multilevel.ts")
909938
add_test(NAME test-jit-shared-export-import-class-extends-implements-diamond COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_implements_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_implements_diamond.ts")
939+
add_test(NAME test-jit-shared-export-import-class-abstract COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_abstract.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_abstract.ts")
940+
add_test(NAME test-jit-shared-export-import-class-static COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_static.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_static.ts")
941+
# DISABLED: see the matching test-compile-export-import-class-generic comment above (known issue, 2026-07-22).
942+
# add_test(NAME test-jit-shared-export-import-class-generic COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_generic.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_generic.ts")
943+
# DISABLED: see the matching test-compile-shared-export-import-class-accessor comment above (known issue, 2026-07-22).
944+
# add_test(NAME test-jit-shared-export-import-class-accessor COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_accessor.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_accessor.ts")
910945
add_test(NAME test-jit-shared-export-import-object-literal-with-interface COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_interface.ts")
911946
add_test(NAME test-jit-shared-export-import-object-literal-untyped COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_untyped.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_untyped.ts")
912947
add_test(NAME test-jit-shared-export-import-object-literal-untyped-multi-method COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_untyped_multi_method.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_untyped_multi_method.ts")

‎tslang/test/tester/tests/00class_accessor_super.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
class Temperature {
2+
protected _celsius: number = 0;
3+
4+
get celsius(): number {
5+
return this._celsius;
6+
}
7+
8+
set celsius(v: number) {
9+
this._celsius = v;
10+
}
11+
12+
get fahrenheit(): number {
13+
return this._celsius * 9 / 5 + 32;
14+
}
15+
}
16+
17+
// overriding an accessor and calling back into the base implementation via
18+
// `super.<accessor>` / `super.<accessor> = value` used to crash MLIR
19+
// verification: the setter call's `this` operand was left as a raw by-value
20+
// ClassStorageType struct instead of being materialized to a pointer (the
21+
// same repair ordinary `super.method()` calls already got via
22+
// getThisRefOfClass), so `'llvm.call' op operand type mismatch` was thrown
23+
// the first time this combination was exercised (found via the cross-module
24+
// accessor test, but the bug itself is general, not cross-module-specific).
25+
class ClampedTemperature extends Temperature {
26+
get celsius(): number {
27+
return super.celsius;
28+
}
29+
30+
set celsius(v: number) {
31+
super.celsius = v < 0 ? 0 : v;
32+
}
33+
}
34+
35+
function main() {
36+
const t = new ClampedTemperature();
37+
t.celsius = -10;
38+
assert(t.celsius == 0);
39+
assert(t.fahrenheit == 32);
40+
41+
t.celsius = 100;
42+
assert(t.celsius == 100);
43+
assert(t.fahrenheit == 212);
44+
45+
print("done.");
46+
}

‎tslang/test/tester/tests/export_class_abstract.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
namespace M {
2+
3+
// 2-level abstract chain (abstract extends abstract) defined entirely in
4+
// the exporting module - the importer supplies the first concrete
5+
// override. This is the class analogue of the cross-module optional/vtable
6+
// interface coverage but for abstract methods specifically: an abstract
7+
// method's vtable slot has no implementation in the declaring module at
8+
// all, so the slot must be patched purely from a module that never even
9+
// sees the base class definition being compiled alongside it.
10+
11+
export abstract class Shape {
12+
color: string = "red";
13+
14+
abstract area(): number;
15+
16+
describe(): string {
17+
return `${this.color} area=${this.area()}`;
18+
}
19+
}
20+
21+
export abstract class NamedShape extends Shape {
22+
name: string = "shape";
23+
24+
abstract area(): number;
25+
26+
describe(): string {
27+
return `${this.name}:${super.describe()}`;
28+
}
29+
}
30+
}

‎tslang/test/tester/tests/export_class_accessor.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
namespace M {
2+
3+
// A get/set accessor pair defined in the exporting module, overridden in
4+
// an importer subclass which calls back into the base accessor via
5+
// `super.celsius`. `super.<accessor> = value` across a module boundary
6+
// used to crash MLIR verification (the `this` operand for the setter
7+
// call was left as a raw by-value ClassStorageType struct instead of
8+
// being materialized to a pointer via getThisRefOfClass, unlike ordinary
9+
// super method calls) - fixed by threading isSuperClass through
10+
// ClassAccessorAccess.
11+
12+
export class Temperature {
13+
protected _celsius: number = 0;
14+
15+
get celsius(): number {
16+
return this._celsius;
17+
}
18+
19+
set celsius(v: number) {
20+
this._celsius = v;
21+
}
22+
23+
get fahrenheit(): number {
24+
return this._celsius * 9 / 5 + 32;
25+
}
26+
}
27+
}

‎tslang/test/tester/tests/export_class_generic.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
namespace M {
2+
3+
// Generic class exported from one module and instantiated with concrete
4+
// type arguments in another. Each distinct instantiation (Box<number>,
5+
// Box<string>, Pair<number,string>) needs its own correctly-laid-out
6+
// fields/methods to be materialized across the module boundary, not just
7+
// a single generic-erased shape.
8+
9+
export class Box<T> {
10+
value: T;
11+
12+
constructor(v: T) {
13+
this.value = v;
14+
}
15+
16+
get(): T {
17+
return this.value;
18+
}
19+
20+
set(v: T): void {
21+
this.value = v;
22+
}
23+
}
24+
25+
export class Pair<A, B> {
26+
first: A;
27+
second: B;
28+
29+
constructor(a: A, b: B) {
30+
this.first = a;
31+
this.second = b;
32+
}
33+
34+
swapDescribe(): string {
35+
return `${this.second}-${this.first}`;
36+
}
37+
}
38+
}

‎tslang/test/tester/tests/export_class_static.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
namespace M {
2+
3+
// Static fields/methods defined in the exporting module, inherited (not
4+
// shadowed) by a subclass in the importer. TS static members are shared,
5+
// not per-subclass storage - `Derived.count` and `Base.count` must be the
6+
// SAME global cell across a module boundary, and `super.increment()`
7+
// called from a static method in the importer must reach the base's
8+
// static method and mutate the base's storage in place.
9+
10+
export class Counter {
11+
static count: number = 0;
12+
static label: string = "counter";
13+
14+
static increment(n: number): void {
15+
Counter.count = Counter.count + n;
16+
}
17+
18+
static describe(): string {
19+
return `${Counter.label}:${Counter.count}`;
20+
}
21+
}
22+
}

‎tslang/test/tester/tests/import_class_abstract.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import './export_class_abstract'
2+
3+
class Circle extends M.NamedShape {
4+
radius: number = 2;
5+
6+
constructor() {
7+
super();
8+
this.name = "circle";
9+
}
10+
11+
area(): number {
12+
return 3 * this.radius * this.radius; // fake pi=3 to keep the check integer-exact
13+
}
14+
}
15+
16+
function main() {
17+
const c = new Circle();
18+
assert(c.area() == 12);
19+
assert(c.describe() == "circle:red area=12");
20+
21+
// virtual dispatch of both the abstract method (area) and the concrete
22+
// override (describe, which itself calls the abstract method through
23+
// `this`) must still reach Circle's implementation through every
24+
// abstract-typed ancestor reference.
25+
const asNamedShape: M.NamedShape = c;
26+
assert(asNamedShape.area() == 12);
27+
assert(asNamedShape.describe() == "circle:red area=12");
28+
29+
const asShape: M.Shape = c;
30+
assert(asShape.area() == 12);
31+
assert(asShape.describe() == "circle:red area=12");
32+
33+
print("done.");
34+
}

0 commit comments

Comments
 (0)