Reading an optional function-typed interface field binds this when present

tslang/lib/TypeScript/MLIRGenClasses.cpp

@@ -2393,8 +2393,10 @@ genContext);
                 // {function, `this` or null} - or, unannotated (`fn = () => 7`), a plain one: neither can be handed out
                 // as a function taking the object as its `this`. A thunk calling `this.fn(...)` fills the
                 // slot as a method does; it reads the field at each call, so a field assigned later is seen.
+                // Not an optional field (`fn?: () => int`): the cell is shared, so the slot would read as present
+                // on every instance, set or not; that stays "field type not matching" below.
                 if (found && isa<mlir_ts::FunctionType>(fieldType) && fieldType != foundField.type
-                    && mth.isAnyFunctionType(foundField.type))
+                    && !isa<mlir_ts::OptionalType>(foundField.type) && mth.isAnyFunctionType(foundField.type))
                 {
                     auto fieldName = dyn_cast<mlir::StringAttr>(id);
                     auto match = mth.TestFunctionTypesMatchWithObjectMethods(location, foundField.type, fieldType);

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -8247,10 +8247,36 @@ class MLIRGenImpl
         // if it is FuncType, we need to create BoundMethod again
         if (auto funcType = dyn_cast<mlir_ts::FunctionType>(fieldInfo->type))
         {
-            auto thisVal =
-                builder.create<mlir_ts::ExtractInterfaceThisOp>(location, getOpaqueType(), interfaceValue);
-            value = builder.create<mlir_ts::CreateBoundFunctionOp>(location, getBoundFunctionType(funcType),
-                                                                    thisVal, value);
+            auto bindThis = [&](mlir::Value value) -> mlir::Value {
+                auto thisVal =
+                    builder.create<mlir_ts::ExtractInterfaceThisOp>(location, getOpaqueType(), interfaceValue);
+                value = builder.create<mlir_ts::CreateBoundFunctionOp>(location, getBoundFunctionType(funcType),
+                                                                        thisVal, value);
+                return value;
+            };
+
+            // an optional field (`fn?: () => int`) loads as optional<fn>: bound when present, undefined when not
+            if (auto optionalType = dyn_cast<mlir_ts::OptionalType>(value.getType()))
+            {
+                auto optionalValue = value;
+                auto hasValue = builder.create<mlir_ts::HasValueOp>(location, getBooleanType(), optionalValue);
+                MLIRCodeLogicHelper mclh(builder, location, compileOptions);
+                auto result = mclh.conditionalValue(
+                    hasValue,
+                    [&]() -> ValueOrLogicalResult {
+                        auto funcValue =
+                            builder.create<mlir_ts::ValueOp>(location, optionalType.getElementType(), optionalValue);
+                        auto boundFuncValue = bindThis(funcValue);
+                        return V(builder.create<mlir_ts::OptionalValueOp>(
+                            location, getOptionalType(boundFuncValue.getType()), boundFuncValue));
+                    },
+                    [&](mlir::Type trueValueType) -> ValueOrLogicalResult {
+                        return V(builder.create<mlir_ts::OptionalUndefOp>(location, trueValueType));
+                    });
+                return V(result);
+            }
+
+            value = bindThis(value);
         }
 
         return value;

tslang/test/tester/CMakeLists.txt

@@ -613,6 +613,7 @@ tslang_add_test(NAME test-compile-00-interface-optional-cast-order COMMAND test-
 tslang_add_test(NAME test-compile-00-interface-function-typed-field COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_function_typed_field.ts")
 tslang_add_test(NAME test-compile-00-interface-function-typed-field-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_function_typed_field_class.ts")
 tslang_add_test(NAME test-compile-00-interface-function-typed-field-class-field COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_function_typed_field_class_field.ts")
+tslang_add_test(NAME test-compile-00-interface-optional-function-typed-field COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_optional_function_typed_field.ts")
 tslang_add_test(NAME test-compile-00-interface-captures COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_captures.ts")
 tslang_add_test(NAME test-compile-00-object-annotated-method COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method.ts")
 tslang_add_test(NAME test-compile-00-object-annotated-method-params COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_params.ts")
@@ -1168,6 +1169,7 @@ tslang_add_test(NAME test-jit-00-interface-optional-cast-order COMMAND test-runn
 tslang_add_test(NAME test-jit-00-interface-function-typed-field COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_function_typed_field.ts")
 tslang_add_test(NAME test-jit-00-interface-function-typed-field-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_function_typed_field_class.ts")
 tslang_add_test(NAME test-jit-00-interface-function-typed-field-class-field COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_function_typed_field_class_field.ts")
+tslang_add_test(NAME test-jit-00-interface-optional-function-typed-field COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_optional_function_typed_field.ts")
 tslang_add_test(NAME test-jit-00-interface-captures COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_captures.ts")
 tslang_add_test(NAME test-jit-00-object-annotated-method COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method.ts")
 tslang_add_test(NAME test-jit-00-object-annotated-method-params COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00object_annotated_method_params.ts")
@@ -2043,6 +2045,7 @@ set(TSLANG_CORPUS
     00interface_object5.ts
     00interface_optional_cast_order.ts
     00interface_optional_extends.ts
+    00interface_optional_function_typed_field.ts
     00interface_optional_method_extends.ts
     00interface_optional.ts
     00interface_partial.ts
@@ -2758,6 +2761,17 @@ set_tests_properties(test-compile-diagnostic-location-rest-parameter-type
                      PROPERTIES PASS_REGULAR_EXPRESSION "rest_parameter_type\\.ts:6:12: error: A rest parameter must be of an array type, not 's32'"
                                 FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed|rest_parameter_type\\.ts:1:|rest_parameter_type\\.ts:4:|verification")
 
+# A class's optional function-typed field cannot fill an interface's optional function-typed
+# field yet: the method cell the slot would point at holds a thunk, present on every instance,
+# so it is an error until the cell carries a per-instance presence check.
+add_test(NAME test-compile-interface-optional-class-function-field
+         COMMAND $<TARGET_FILE:tslang> --emit=obj --no-default-lib -mm=none
+                 "${PROJECT_SOURCE_DIR}/test/tester/interface-errors/optional_class_function_field.ts"
+                 -o "${CMAKE_CURRENT_BINARY_DIR}/interface-optional-class-function-field.obj")
+set_tests_properties(test-compile-interface-optional-class-function-field
+                     PROPERTIES PASS_REGULAR_EXPRESSION "field type not matching for \"fn\" for interface 'MaybeFn' in class 'OptField'"
+                                FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed")
+
 # A call with fewer arguments than the callee's required parameters is an error (TS2554),
 # not padded with undef; so is one with more arguments than the callee has parameters
 # (too_many_*), which went silently into a slot the callee does not have (a rest parameter

tslang/test/tester/interface-errors/optional_class_function_field.ts

@@ -0,0 +1,17 @@
+// A class's OPTIONAL function-typed field (`fn?: () => int`) cannot fill an interface's
+// optional function-typed field yet: the slot would point at the shared method cell, a
+// thunk calling `this.fn(...)`, so the field would read as present on every instance -
+// set or not - and an unset one would be called. Until the cell carries a per-instance
+// presence check it is an error, as it was before class fields could fill such a slot.
+
+interface MaybeFn {
+    fn?: () => int;
+}
+
+class OptField {
+    fn?: () => int;
+}
+
+function main() {
+    const m: MaybeFn = new OptField();
+}

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

@@ -0,0 +1,117 @@
+// regression test: reading an OPTIONAL function-typed interface field
+// (`fn?: () => int`) as a value. The field loads as optional<function>, and
+// binding the interface's `this` to it directly failed to compile
+// ('ts.CreateBoundFunction' op operand #1 must be ...). A present field now
+// reads as an optional bound function, an absent one as undefined - for an
+// object literal and for a class whose method fills the field
+// (INTERFACE_METHOD_CELL_TAG, Defines.h) alike.
+//
+// A class's own optional function-typed field (`fn?: () => int`) filling the
+// interface's is a compile error for now: interface-errors/optional_class_function_field.ts.
+
+interface MaybeFn {
+    fn?: () => int;
+}
+
+function maybe(m: MaybeFn): int {
+    const g = m.fn;
+    if (g) {
+        return g();
+    }
+
+    return -1;
+}
+
+function callOpt(m: MaybeFn): int {
+    return m.fn?.() ?? -2;
+}
+
+function isSet(m: MaybeFn): boolean {
+    return m.fn !== undefined;
+}
+
+function getFn(m: MaybeFn): (() => int) | undefined {
+    return m.fn;
+}
+
+class WithFn {
+    constructor(public v: int) {}
+    fn(): int {
+        return this.v;
+    }
+}
+
+class WithoutFn {
+    constructor(public v: int) {}
+}
+
+interface MaybeAdd {
+    base: int;
+    add?: (x: int) => int;
+}
+
+function addOr(m: MaybeAdd, x: int): int {
+    const add = m.add;
+    if (add) {
+        return add(x);
+    }
+
+    return m.base;
+}
+
+class Adder {
+    constructor(public base: int) {}
+    add(x: int): int {
+        return this.base + x;
+    }
+}
+
+function main() {
+    // object literal
+    print(maybe({ fn() { return 3; } }), maybe({}));
+    assert(maybe({ fn() { return 3; } }) == 3);
+    assert(maybe({}) == -1);
+    assert(callOpt({ fn() { return 4; } }) == 4);
+    assert(callOpt({}) == -2);
+    assert(isSet({ fn() { return 5; } }));
+    assert(!isSet({}));
+
+    // the bound value keeps the literal's `this`
+    assert(maybe({ v: 40, fn() { return this.v + 2; } }) == 42);
+
+    // a class whose method fills the field, and one that lacks it
+    assert(maybe(new WithFn(7)) == 7);
+    assert(maybe(new WithoutFn(8)) == -1);
+    assert(callOpt(new WithFn(9)) == 9);
+    assert(callOpt(new WithoutFn(10)) == -2);
+    assert(isSet(new WithFn(11)));
+    assert(!isSet(new WithoutFn(12)));
+
+    // one access site, mixed implementers
+    const all: MaybeFn[] = [new WithFn(1), new WithoutFn(2), { fn() { return 3; } }, {}];
+    let sum = 0;
+    for (const m of all) {
+        sum += maybe(m);
+    }
+
+    print(sum);
+    assert(sum == 1 - 1 + 3 - 1);
+
+    // the bound value outlives the access; under -mm=rc a bound method does not hold its
+    // receiver yet, so the instance is kept alive while the returned value is used
+    const w13 = new WithFn(13);
+    const f = getFn(w13);
+    assert(f !== undefined);
+    if (f) {
+        assert(f() == 13);
+    }
+
+    assert(getFn(new WithoutFn(14)) === undefined);
+
+    // with a parameter
+    assert(addOr(new Adder(10), 5) == 15);
+    assert(addOr({ base: 20, add(x: int) { return this.base * x; } }, 2) == 40);
+    assert(addOr({ base: 30 }, 2) == 30);
+
+    print("done.");
+}