diff --git a/tslang/lib/TypeScript/MLIRGenClasses.cpp b/tslang/lib/TypeScript/MLIRGenClasses.cpp index ce2a8e802..c855b543e 100644 --- a/tslang/lib/TypeScript/MLIRGenClasses.cpp +++ b/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(fieldType) && fieldType != foundField.type - && mth.isAnyFunctionType(foundField.type)) + && !isa(foundField.type) && mth.isAnyFunctionType(foundField.type)) { auto fieldName = dyn_cast(id); auto match = mth.TestFunctionTypesMatchWithObjectMethods(location, foundField.type, fieldType); diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index e3f733d3f..1fcb3b174 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/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(fieldInfo->type)) { - auto thisVal = - builder.create(location, getOpaqueType(), interfaceValue); - value = builder.create(location, getBoundFunctionType(funcType), - thisVal, value); + auto bindThis = [&](mlir::Value value) -> mlir::Value { + auto thisVal = + builder.create(location, getOpaqueType(), interfaceValue); + value = builder.create(location, getBoundFunctionType(funcType), + thisVal, value); + return value; + }; + + // an optional field (`fn?: () => int`) loads as optional: bound when present, undefined when not + if (auto optionalType = dyn_cast(value.getType())) + { + auto optionalValue = value; + auto hasValue = builder.create(location, getBooleanType(), optionalValue); + MLIRCodeLogicHelper mclh(builder, location, compileOptions); + auto result = mclh.conditionalValue( + hasValue, + [&]() -> ValueOrLogicalResult { + auto funcValue = + builder.create(location, optionalType.getElementType(), optionalValue); + auto boundFuncValue = bindThis(funcValue); + return V(builder.create( + location, getOptionalType(boundFuncValue.getType()), boundFuncValue)); + }, + [&](mlir::Type trueValueType) -> ValueOrLogicalResult { + return V(builder.create(location, trueValueType)); + }); + return V(result); + } + + value = bindThis(value); } return value; diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 89d6c17ba..c90517699 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/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 $ --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 diff --git a/tslang/test/tester/interface-errors/optional_class_function_field.ts b/tslang/test/tester/interface-errors/optional_class_function_field.ts new file mode 100644 index 000000000..e4a5d5d35 --- /dev/null +++ b/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(); +} diff --git a/tslang/test/tester/tests/00interface_optional_function_typed_field.ts b/tslang/test/tester/tests/00interface_optional_function_typed_field.ts new file mode 100644 index 000000000..3ca2e6a79 --- /dev/null +++ b/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, 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."); +}