A class's optional function field fills an interface's optional one

tslang/include/TypeScript/Defines.h

@@ -164,8 +164,12 @@
 // plus this bit. An access reads the cell (InterfaceSymbolRefOpLowering), then binds `this` as for
 // any function-typed field. The bit is free in every other function-typed slot: an offset of a
 // pointer-sized field (or of optional<function>, which starts with one) is pointer-aligned, and so
-// is the cell; -1 (absent) is tested before it. A write through such a field changes the cell,
-// so it changes the method for every instance of the class seen through that interface.
+// is the cell; -1 (absent) is tested before it. A function-typed interface field cannot be
+// written ("saving to constant object"): a read binds `this`, so nothing stores to the cell.
+// The cell is {method, flag offset}, the method first: a class's optional field holding a function
+// (`fn?: () => int`, called through a thunk) has its flag at `this` + the offset, so a read of the
+// interface's optional field (`fn?: () => int`) is absent while the instance's flag is clear; -1
+// is always present (a method, or a field that is not optional).
 #define INTERFACE_METHOD_CELL_TAG 1
 // on an InterfaceSymbolRefOp of a `v?: T` field that is stored to: true sets the flag of a tagged slot's
 // field, false (a store of undefined) clears it

tslang/lib/TypeScript/LowerToLLVM.cpp

@@ -6478,10 +6478,13 @@ struct InterfaceSymbolRefOpLowering : public TsLlvmPattern<mlir_ts::InterfaceSym
             // a function-typed field's slot tagged INTERFACE_METHOD_CELL_TAG: a class fills the field with a
             // method, and the slot is the address of the cell holding it. Tested ahead of
             // INTERFACE_OPTIONAL_STORAGE_TAG, which a cell's address can carry on a 32-bit target.
+            // With checkPresence (a read of an optional field), the cell's flag offset decides: -1 is
+            // present, any other is the offset from `this` of the instance's flag, and a clear flag reads
+            // as absent (a null address).
             auto fieldRefType = dyn_cast<mlir_ts::RefType>(interfaceSymbolRefOp.getType());
             auto isFunctionField = fieldRefType && isa<mlir_ts::FunctionType>(fieldRefType.getElementType());
-            auto orMethodCell = [&](std::function<mlir::Value(OpBuilder &, Location)> offsetAddrFunc) {
-                return [&, offsetAddrFunc](OpBuilder &builder, Location location) -> mlir::Value {
+            auto orMethodCell = [&](std::function<mlir::Value(OpBuilder &, Location)> offsetAddrFunc, bool checkPresence) {
+                return [&, offsetAddrFunc, checkPresence](OpBuilder &builder, Location location) -> mlir::Value {
                     auto intPtrType = rewriter.getIntegerType(tsLlvmContext->compileOptions.sizeBits());
                     auto slotValue = rewriter.create<LLVM::PtrToIntOp>(loc, intPtrType, methodOrFieldPtr);
                     auto tag = rewriter.create<LLVM::ConstantOp>(
@@ -6491,7 +6494,35 @@ struct InterfaceSymbolRefOpLowering : public TsLlvmPattern<mlir_ts::InterfaceSym
                     auto isMethodCell = rewriter.create<LLVM::ICmpOp>(loc, LLVM::ICmpPredicate::ne, tagBit, zero);
                     auto methodCellAddrFunc = [&](OpBuilder &builder, Location location) -> mlir::Value {
                         auto cellInt = rewriter.create<LLVM::SubOp>(loc, intPtrType, slotValue, tag);
-                        return rewriter.create<LLVM::IntToPtrOp>(loc, fieldLLVMTypeRef, cellInt.getResult());
+                        mlir::Value cellAddr = rewriter.create<LLVM::IntToPtrOp>(loc, fieldLLVMTypeRef, cellInt.getResult());
+                        if (!checkPresence)
+                        {
+                            return cellAddr;
+                        }
+
+                        // the cell is {method, flag offset}
+                        auto cellLLVMType =
+                            LLVM::LLVMStructType::getLiteral(rewriter.getContext(), {th.getPtrType(), intPtrType}, false);
+                        auto flagOffsetPtr = rewriter.create<LLVM::GEPOp>(loc, th.getPtrType(), cellLLVMType, cellAddr,
+                                                                          ArrayRef<LLVM::GEPArg>{0, 1});
+                        auto flagOffset = rewriter.create<LLVM::LoadOp>(loc, intPtrType, flagOffsetPtr);
+                        auto negative1 =
+                            rewriter.create<LLVM::ConstantOp>(loc, intPtrType, rewriter.getIntegerAttr(intPtrType, -1));
+                        auto alwaysPresent =
+                            rewriter.create<LLVM::ICmpOp>(loc, LLVM::ICmpPredicate::eq, flagOffset, negative1);
+                        auto cellAddrFunc = [&](OpBuilder &builder, Location location) -> mlir::Value { return cellAddr; };
+                        auto instanceFlagAddrFunc = [&](OpBuilder &builder, Location location) -> mlir::Value {
+                            auto thisInt = rewriter.create<LLVM::PtrToIntOp>(loc, intPtrType, thisVal);
+                            auto flagInt = rewriter.create<LLVM::AddOp>(loc, intPtrType, thisInt, flagOffset);
+                            auto flagPtr = rewriter.create<LLVM::IntToPtrOp>(loc, th.getPtrType(), flagInt.getResult());
+                            auto hasValue = rewriter.create<LLVM::LoadOp>(loc, rewriter.getI1Type(), flagPtr);
+                            auto nullAddr = rewriter.create<LLVM::ZeroOp>(loc, fieldLLVMTypeRef);
+                            return rewriter.create<LLVM::SelectOp>(loc, hasValue, cellAddr, nullAddr);
+                        };
+
+                        // the flag is loaded only when there is one
+                        return clh.conditionalExpressionLowering(loc, fieldLLVMTypeRef, alwaysPresent, cellAddrFunc,
+                                                                 instanceFlagAddrFunc);
                     };
 
                     return clh.conditionalExpressionLowering(loc, fieldLLVMTypeRef, isMethodCell, methodCellAddrFunc,
@@ -6554,7 +6585,9 @@ struct InterfaceSymbolRefOpLowering : public TsLlvmPattern<mlir_ts::InterfaceSym
                                                              storedAsOptionalAddrFunc, calcFieldTotalAddrFunc);
                 };
 
-                auto presentOrMethodCellAddrFunc = orMethodCell(presentAddrFunc);
+                // a function-typed field is only read (a write is "saving to constant object"), so the
+                // presence check always applies
+                auto presentOrMethodCellAddrFunc = orMethodCell(presentAddrFunc, true);
                 auto result = clh.conditionalExpressionLowering(
                     loc, fieldLLVMTypeRef, condVal, nullAddrFunc,
                     isFunctionField ? mlir::function_ref<mlir::Value(OpBuilder &, Location)>(presentOrMethodCellAddrFunc)
@@ -6563,7 +6596,7 @@ struct InterfaceSymbolRefOpLowering : public TsLlvmPattern<mlir_ts::InterfaceSym
             }
             else if (isFunctionField)
             {
-                fieldAddr = orMethodCell(calcFieldTotalAddrFunc)(rewriter, loc);
+                fieldAddr = orMethodCell(calcFieldTotalAddrFunc, false)(rewriter, loc);
             }
             else
             {

tslang/lib/TypeScript/MLIRGenClasses.cpp

@@ -2355,6 +2355,9 @@ genContext);
         // the interface's function-typed fields (`fn: () => int`) the class implements with a method
         // (`fn(): int`): their slots point at a cell holding the method (see INTERFACE_METHOD_CELL_TAG)
         llvm::SmallVector<std::pair<mlir::Attribute, MethodInfo>> methodsForFields;
+        // of those, the ones filled by an optional class field (`fn?: () => int`): their cells carry the
+        // offset of the field's flag, so an access reads the instance's presence
+        llvm::SmallVector<mlir::Attribute> optionalFieldsForCells;
         auto result = newInterfacePtr->getVirtualTable(
             virtualTable,
             [&](mlir::Attribute id, mlir::Type fieldType, bool isConditional) -> std::pair<mlir_ts::FieldInfo, mlir::LogicalResult> {
@@ -2393,10 +2396,11 @@ 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.
+                // An optional field (`fn?: () => int`) fills only an optional one (`fn?: () => int`), and its cell
+                // carries the offset of the field's flag: the cell is shared, the flag is the instance's.
+                auto optionalField = found && isa<mlir_ts::OptionalType>(foundField.type);
                 if (found && isa<mlir_ts::FunctionType>(fieldType) && fieldType != foundField.type
-                    && !isa<mlir_ts::OptionalType>(foundField.type) && mth.isAnyFunctionType(foundField.type))
+                    && (!optionalField || isConditional) && mth.isAnyFunctionType(foundField.type))
                 {
                     auto fieldName = dyn_cast<mlir::StringAttr>(id);
                     auto match = mth.TestFunctionTypesMatchWithObjectMethods(location, foundField.type, fieldType);
@@ -2428,6 +2432,10 @@ genContext);
                         thunkInfo.virtualIndex = -1;
                         thunkInfo.accessLevel = mlir_ts::AccessLevel::Public;
                         methodsForFields.push_back({id, thunkInfo});
+                        if (optionalField)
+                        {
+                            optionalFieldsForCells.push_back(id);
+                        }
                     }
 
                     return {mlir_ts::FieldInfo{id, fieldType, false, mlir_ts::AccessLevel::Public}, mlir::success()};
@@ -2609,7 +2617,55 @@ genContext);
                         return TypeValueInitType{mlir::Type(), mlir::Value(), TypeProvided::No};
                     }
 
-                    return TypeValueInitType{methodInfo.funcType, methodValue, TypeProvided::Yes};
+                    // the cell is {method, flag offset}, the method first so the slot is a ref to it. The offset
+                    // is of the flag of an optional class field (`fn?: () => int`), from `this`; -1 is always
+                    // present (a method, or a field that is not optional)
+                    auto ptrIntType = builder.getIntegerType(compileOptions.sizeBits());
+                    mlir::Value flagOffsetValue;
+                    if (llvm::is_contained(optionalFieldsForCells, id))
+                    {
+                        // &(null)->field, stored as optional<T> - {T, flag}, the flag right after T
+                        MLIRCodeLogic mcl(builder, compileOptions);
+                        auto nullObj = builder.create<mlir_ts::NullOp>(location, getNullType());
+                        auto classNull = cast(location, newClassPtr->classType, nullObj, genContext, true);
+                        auto fieldValue = mlirGenPropertyAccessExpression(location, classNull, id, genContext);
+                        auto fieldRef = fieldValue ? mcl.GetReferenceFromValue(location, fieldValue) : mlir::Value();
+                        if (!fieldRef)
+                        {
+                            emitError(location) << "can't find reference for field: " << id
+                                                << " in interface: " << newInterfacePtr->fullName
+                                                << " for class: " << newClassPtr->fullName;
+                            return TypeValueInitType{mlir::Type(), mlir::Value(), TypeProvided::No};
+                        }
+
+                        auto optionalType = mlir::cast<mlir_ts::OptionalType>(
+                            mlir::cast<mlir_ts::RefType>(fieldRef.getType()).getElementType());
+                        auto fieldOffset = builder.create<mlir_ts::CastOp>(location, ptrIntType, fieldRef);
+                        auto valueSize = builder.create<mlir_ts::SizeOfOp>(location, mth.getIndexType(),
+                                                                           optionalType.getElementType());
+                        auto valueSizeInt = builder.create<mlir_ts::CastOp>(location, ptrIntType, valueSize);
+                        flagOffsetValue = builder.create<mlir_ts::ArithmeticBinaryOp>(
+                            location, ptrIntType, builder.getI32IntegerAttr((int)SyntaxKind::PlusToken), fieldOffset,
+                            valueSizeInt);
+                    }
+                    else
+                    {
+                        flagOffsetValue =
+                            builder.create<mlir_ts::ConstantOp>(location, ptrIntType, builder.getIntegerAttr(ptrIntType, -1));
+                    }
+
+                    mlir::SmallVector<mlir_ts::FieldInfo> cellFields{
+                        {MLIRHelper::TupleFieldName("method", builder.getContext()), methodInfo.funcType, false,
+                         mlir_ts::AccessLevel::Public},
+                        {MLIRHelper::TupleFieldName("flagOffset", builder.getContext()), ptrIntType, false,
+                         mlir_ts::AccessLevel::Public}};
+                    auto cellTupleType = getTupleType(cellFields);
+                    mlir::Value cellValue = builder.create<mlir_ts::UndefOp>(location, cellTupleType);
+                    cellValue = builder.create<mlir_ts::InsertPropertyOp>(location, cellTupleType, methodValue, cellValue,
+                                                                          MLIRHelper::getStructIndex(builder, 0));
+                    cellValue = builder.create<mlir_ts::InsertPropertyOp>(location, cellTupleType, flagOffsetValue,
+                                                                          cellValue, MLIRHelper::getStructIndex(builder, 1));
+                    return TypeValueInitType{cellTupleType, cellValue, TypeProvided::Yes};
                 },
                 genContext);
             if (!cellType)

tslang/test/tester/CMakeLists.txt

@@ -614,6 +614,7 @@ tslang_add_test(NAME test-compile-00-interface-function-typed-field COMMAND test
 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-optional-function-typed-field-class-field COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_optional_function_typed_field_class_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")
@@ -1170,6 +1171,7 @@ tslang_add_test(NAME test-jit-00-interface-function-typed-field COMMAND test-run
 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-optional-function-typed-field-class-field COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00interface_optional_function_typed_field_class_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")
@@ -2046,6 +2048,7 @@ set(TSLANG_CORPUS
     00interface_optional_cast_order.ts
     00interface_optional_extends.ts
     00interface_optional_function_typed_field.ts
+    00interface_optional_function_typed_field_class_field.ts
     00interface_optional_method_extends.ts
     00interface_optional.ts
     00interface_partial.ts
@@ -2761,15 +2764,14 @@ 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
+# A function-typed interface field cannot be written: for a class its slot points at a method cell
+# shared by every instance, so a write would change the function for all of them.
+add_test(NAME test-compile-interface-function-field-write
          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'"
+                 "${PROJECT_SOURCE_DIR}/test/tester/interface-errors/function_field_write.ts"
+                 -o "${CMAKE_CURRENT_BINARY_DIR}/interface-function-field-write.obj")
+set_tests_properties(test-compile-interface-function-field-write
+                     PROPERTIES PASS_REGULAR_EXPRESSION "function_field_write\\.ts:9:5: error: saving to constant object"
                                 FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed")
 
 # A call with fewer arguments than the callee's required parameters is an error (TS2554),

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

@@ -0,0 +1,10 @@
+// A function-typed interface field is read-only: a read binds the interface's `this`, and a
+// write could not be checked against the object behind the interface. For a class the slot
+// points at a method cell shared by every instance (INTERFACE_METHOD_CELL_TAG), which a write
+// must not reach.
+interface MaybeFn { fn?: () => int; }
+class O { fn?: () => int; }
+function main() {
+    const m: MaybeFn = new O();
+    m.fn = () => 9;
+}