A union of pointer types stores null in its pointer, not as a member

tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h

@@ -636,7 +636,7 @@ class CastLogicHelper
                     // instead, a null `Handle | null` widened to `boolean | Handle | null` was a class
                     // holding a null pointer, which `=== null` did not see and `instanceof` read through.
                     auto storageTypes = mlir::cast<mlir_ts::UnionType>(baseType).getTypes();
-                    if (isPointerNullableType(inType)
+                    if (MLIRTypeHelper::isNullAbsorbingType(inType)
                         && llvm::any_of(storageTypes, [](mlir::Type type) { return isa<mlir_ts::NullType>(type); }))
                     {
                         auto llvmStringType = tch.convertType(stringType);
@@ -708,12 +708,6 @@ class CastLogicHelper
         return mlir::Value();
     }
 
-    // A type whose value is a single pointer that is null when it holds null.
-    static bool isPointerNullableType(mlir::Type type)
-    {
-        return isa<mlir_ts::ClassType, mlir_ts::StringType, mlir_ts::ArrayType, mlir_ts::ObjectType, mlir_ts::OpaqueType>(type);
-    }
-
     bool isBool(mlir::Type type)
     {
         return type.isInteger(1);

tslang/include/TypeScript/LowerToLLVM/UnaryBinLogicalOrHelper.h

@@ -110,6 +110,32 @@ mlir::Value LogicOp(Operation *binOp, SyntaxKind op, mlir::Value left, mlir::Typ
             : builder.create<mlir_ts::DialectCastOp>(loc, llvmUnionType, value).getResult();
     };
 
+    // Compared with null, a tagged union tells by its tag when its actual type (isUnionTypeNeedsTag)
+    // has a null member. Without one, and with members that all hold null in their own pointer
+    // (`A | B | null`), it is null when its value's pointer is null, whichever member's tag it
+    // carries: a null `Handle` is a Handle holding a null pointer.
+    auto nullCompare = [&](mlir::Value unionValue, mlir_ts::UnionType unionType) -> mlir::Value {
+        ::typescript::MLIRTypeHelper mth(builder.getContext(), compileOptions);
+        auto llvmUnion = asLLVMUnion(unionValue, unionType);
+
+        mlir::Type actualType;
+        mth.isUnionTypeNeedsTag(loc, unionType, actualType);
+        auto actualUnionType = dyn_cast<mlir_ts::UnionType>(actualType);
+        if (actualUnionType && mth.isNullAbsorbingUnion(actualUnionType))
+        {
+            // the first word of the value field: the pointer every member holds there
+            auto pointerWord = builder.create<LLVM::ExtractValueOp>(loc, llvmUnion, ArrayRef<int64_t>{UNION_VALUE_INDEX, 0, 0});
+            auto zero = builder.create<LLVM::ZeroOp>(loc, pointerWord.getType());
+            return builder.create<StdIOpTy>(loc, v1, pointerWord, zero);
+        }
+
+        auto tagValue = builder.create<LLVM::ExtractValueOp>(loc, llvmtch.typeConverter->convertType(mth.getStringType()),
+                            llvmUnion, MLIRHelper::getStructIndex(builder, UNION_TAG_INDEX));
+        auto nullStr = builder.create<mlir_ts::ConstantOp>(loc, mth.getStringType(), builder.getStringAttr("null"));
+        return builder.create<mlir_ts::StringCompareOp>(
+            loc, mth.getBooleanType(), tagValue, nullStr, builder.getI32IntegerAttr((int)op));
+    };
+
     if (isa<mlir_ts::OptionalType>(leftType) || isa<mlir_ts::OptionalType>(rightType))
     {
         return OptionalTypeLogicalOp<StdIOpTy, V1, v1, StdFOpTy, V2, v2>(binOp, op, builder, typeConverter, compileOptions);
@@ -221,12 +247,7 @@ mlir::Value LogicOp(Operation *binOp, SyntaxKind op, mlir::Value left, mlir::Typ
             mlir::Type baseType;
             if (mth.isUnionTypeNeedsTag(loc, unionType, baseType))
             {
-                auto tagValue = builder.create<LLVM::ExtractValueOp>(loc, llvmtch.typeConverter->convertType(mth.getStringType()),
-                                    asLLVMUnion(right, unionType), MLIRHelper::getStructIndex(builder, UNION_TAG_INDEX));
-                auto nullStr = builder.create<mlir_ts::ConstantOp>(loc, mth.getStringType(), builder.getStringAttr("null"));
-                auto cmpOp = builder.create<mlir_ts::StringCompareOp>(
-                    loc, mth.getBooleanType(), tagValue, nullStr, builder.getI32IntegerAttr((int)op));
-                return cmpOp;
+                return nullCompare(right, unionType);
             }
 
             // the union is the right operand, so it is the right type that becomes its base type:
@@ -314,12 +335,7 @@ mlir::Value LogicOp(Operation *binOp, SyntaxKind op, mlir::Value left, mlir::Typ
                 right.getDefiningOp<mlir_ts::NullOp>() || right.getDefiningOp<LLVM::ZeroOp>() || matchPattern(right, m_Zero());            
             if (isRightNullValue)
             {
-                auto tagValue = builder.create<LLVM::ExtractValueOp>(loc, llvmtch.typeConverter->convertType(mth.getStringType()),
-                                    asLLVMUnion(left, unionType), MLIRHelper::getStructIndex(builder, UNION_TAG_INDEX));
-                auto nullStr = builder.create<mlir_ts::ConstantOp>(loc, mth.getStringType(), builder.getStringAttr("null"));
-                auto cmpOp = builder.create<mlir_ts::StringCompareOp>(
-                    loc, mth.getBooleanType(), tagValue, nullStr, builder.getI32IntegerAttr((int)op));
-                return cmpOp;
+                return nullCompare(left, unionType);
             }
 
             emitError(loc, "Not applicable logical operator for type: '") << to_print<int>(leftType) << "'";

tslang/include/TypeScript/MLIRLogic/MLIRTypeHelper.h

@@ -3329,12 +3329,33 @@ class MLIRTypeHelper
         return getUnionTypeWithMerge(location, types, mergeLiterals, mergeTypes);
     }
 
-    // Without strict null checks a nullable type (string, a class) holds null in its own pointer, so
-    // null goes away beside it. A union of several types is a tagged value, not one pointer: there
-    // null has nowhere to go but a member of its own.
+    // A type whose value is one pointer that is null when it holds null: it absorbs `| null`.
+    static bool isNullAbsorbingType(mlir::Type type)
+    {
+        return isa<mlir_ts::ClassType, mlir_ts::StringType, mlir_ts::ArrayType, mlir_ts::ObjectType, mlir_ts::OpaqueType>(type);
+    }
+
+    // A union whose members, null aside, all absorb null (`A | B | null`, `string | A | null`): stored
+    // without a null member, its value is null exactly when its value's pointer is null, whichever
+    // member's tag it carries.
+    bool isNullAbsorbingUnion(mlir_ts::UnionType unionType)
+    {
+        return llvm::all_of(unionType.getTypes(), [](mlir::Type type) {
+            return isa<mlir_ts::NullType>(type) || isNullAbsorbingType(type);
+        });
+    }
+
+    // A nullable type (string, a class) holds null in its own pointer, so null goes away beside it:
+    // `Class | null` is stored as the class. So does a union of several such types - null is a null
+    // pointer in whichever member. Only a member that cannot hold null (a number: 0 is not null)
+    // makes null a member of its own. The type still says `| null` under strict null checks, which
+    // is what allows or refuses assigning null; this is about how the value is stored.
     bool needsNullMember(mlir::ArrayRef<mlir::Type> types)
     {
-        return llvm::count_if(types, [](mlir::Type type) { return !isa<mlir_ts::NullType>(type); }) > 1;
+        auto nonNull = llvm::count_if(types, [](mlir::Type type) { return !isa<mlir_ts::NullType>(type); });
+        return nonNull > 1 && !llvm::all_of(types, [](mlir::Type type) {
+            return isa<mlir_ts::NullType>(type) || isNullAbsorbingType(type);
+        });
     }
 
     // TODO: review all union merge logic
@@ -3409,7 +3430,8 @@ class MLIRTypeHelper
             mlir::SmallVector<mlir::Type> mergedTypesAll;
             this->mergeTypes(location, typesAll, mergedTypesAll);
 
-            if (unionContext.isNullable && (compileOptions.strictNullChecks || needsNullMember(mergedTypesAll)))
+            if (unionContext.isNullable
+                && ((compileOptions.strictNullChecks && !disableStrickNullCheck) || needsNullMember(mergedTypesAll)))
             {
                 mergedTypesAll.push_back(getNullType());
             }

tslang/lib/TypeScript/MLIRGenCast.cpp

@@ -2254,6 +2254,33 @@ namespace mlirgen
                     return mlir::failure();
                 }
 
+                auto toBoolean = isa<mlir_ts::BooleanType>(type) || isa<mlir_ts::TypePredicateType>(type);
+
+                // A union whose actual type has no null member but whose members all hold null in
+                // their own pointer (`A | B | null`) is null when that pointer is null, whatever its
+                // tag: asked first, so that no member's test - `instanceof` reads the vtable - meets a
+                // null instance. A tag of its own for null is then never the one that tells.
+                mlir::Type actualType;
+                auto actualUnionType = mth.isUnionTypeNeedsTag(location, unionType, actualType)
+                    ? dyn_cast<mlir_ts::UnionType>(actualType) : mlir_ts::UnionType();
+                if (actualUnionType && mth.isNullAbsorbingUnion(actualUnionType))
+                {
+                    typeOfs.erase("null");
+
+                    // false tested for truth, the text "null" as text (`String(null)`), null where the
+                    // target takes null, and otherwise no value at all - as the end of this function
+                    auto targetTakesNull = isa<mlir_ts::NullType>(type)
+                        || (isa<mlir_ts::UnionType>(type)
+                            && llvm::any_of(mlir::cast<mlir_ts::UnionType>(type).getTypes(), [](mlir::Type member) { return isa<mlir_ts::NullType>(member); }))
+                        || (!compileOptions.strictNullChecks && MLIRTypeHelper::isNullAbsorbingType(type));
+                    ss << S("if (t === null) ")
+                       << (toBoolean ? S("return false;")
+                           : isa<mlir_ts::StringType>(type) ? S("return \"null\";")
+                           : targetTakesNull ? S("return null;")
+                           : S("throw \"Can't cast from union type\";"))
+                       << S("\n");
+                }
+
                 if (isNullDest)
                 {
                     // to null
@@ -2297,7 +2324,6 @@ namespace mlirgen
                     // to 'boolean'" - so a union of null and two or more other types, which is what
                     // `enabled && handle` makes of a boolean and a `Handle | null`, could not be
                     // tested in an `if` at all.
-                    auto toBoolean = isa<mlir_ts::BooleanType>(type) || isa<mlir_ts::TypePredicateType>(type);
 
                     // default
                     auto next = false;
@@ -2312,14 +2338,6 @@ namespace mlirgen
                         {
                             ss << S("return false;\n");
                         }
-                        else if (toBoolean && pair.getKey() == "class" && classInstances.size() == 1)
-                        {
-                            // tested for truth, an instance is true and null is false - and a null
-                            // instance does carry the class's tag, when a `Class | null` became this
-                            // union (`enabled && handle` with handle null). `instanceof` would read
-                            // its vtable through the null pointer.
-                            ss << S("return t;\n");
-                        }
                         else if (pair.getKey() == "class")
                         {
                             ss << S("{ \n");

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -6430,6 +6430,15 @@ class MLIRGenImpl
             }
         }
 
+        // Compared with null, a union stays a union: the lowering (LogicOp) asks the union itself,
+        // by its tag or, where null is a null pointer in a member, by that pointer. Cast to null
+        // first, it went through a generated cast whose `instanceof` read a null instance's vtable.
+        if ((isLeftUnion && isa<mlir_ts::NullType>(rightExpressionValue.getType()))
+            || (isRightUnion && isa<mlir_ts::NullType>(leftExpressionValue.getType())))
+        {
+            return isLeftUnion ? leftExpressionValue : rightExpressionValue;
+        }
+
         if (isLeftUnion && isRightUnion)
         {
             // TODO: finish cast between unions

tslang/test/tester/CMakeLists.txt

@@ -410,6 +410,7 @@ tslang_add_test(NAME test-compile-00-union-errors COMMAND test-runner "${PROJECT
 tslang_add_test(NAME test-compile-00-union-null-to-string COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_to_string.ts")
 tslang_add_test(NAME test-compile-00-union-null-truthiness COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_truthiness.ts")
 tslang_add_test(NAME test-compile-00-null-into-tagged-union COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00null_into_tagged_union.ts")
+tslang_add_test(NAME test-compile-00-union-null-in-pointer COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_in_pointer.ts")
 tslang_add_test(NAME test-compile-00-union-null-undefined COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_undefined.ts")
 tslang_add_test(NAME test-compile-00-union-null-undefined-nonstrict COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_undefined_nonstrict.ts")
 tslang_add_test(NAME test-compile-00-optional-number-compare COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00optional_number_compare.ts")
@@ -938,6 +939,7 @@ tslang_add_test(NAME test-jit-00-union-errors COMMAND test-runner -jit "${PROJEC
 tslang_add_test(NAME test-jit-00-union-null-to-string COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_to_string.ts")
 tslang_add_test(NAME test-jit-00-union-null-truthiness COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_truthiness.ts")
 tslang_add_test(NAME test-jit-00-null-into-tagged-union COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00null_into_tagged_union.ts")
+tslang_add_test(NAME test-jit-00-union-null-in-pointer COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_in_pointer.ts")
 tslang_add_test(NAME test-jit-00-union-null-undefined COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_undefined.ts")
 tslang_add_test(NAME test-jit-00-union-null-undefined-nonstrict COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_null_undefined_nonstrict.ts")
 tslang_add_test(NAME test-jit-00-optional-number-compare COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00optional_number_compare.ts")
@@ -2055,6 +2057,7 @@ set(TSLANG_CORPUS
     00union_null_to_string.ts
     00union_null_truthiness.ts
     00null_into_tagged_union.ts
+    00union_null_in_pointer.ts
     00union_ops.ts
     00union_to_any.ts
     00union_type.ts

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

@@ -0,0 +1,100 @@
+// A class or a string holds null in its own pointer, so a union of such types with null
+// (`A | B | null`, `string | A | null`) stores no null member: its value is null when its pointer is
+// null, whichever member's tag it carries. The type keeps `| null`, which is what allows assigning
+// null under strict null checks. A null `A` widened to `A | B | null` used to crash `=== null`, a truth
+// test and `??` (the generated casts read its vtable through the null pointer), and printing such a
+// union did not compile.
+
+class A {
+    a = 1;
+    toString() {
+        return "A";
+    }
+}
+
+class B {
+    b = 2;
+    toString() {
+        return "B";
+    }
+}
+
+function widen(h: A | null): A | B | null {
+    return h;
+}
+
+function literal(): A | B | null {
+    return null;
+}
+
+function isNull(u: A | B | null) {
+    return u === null;
+}
+
+function notNull(u: A | B | null) {
+    return u != null;
+}
+
+function truthy(u: A | B | null) {
+    return u ? true : false;
+}
+
+function orB(u: A | B | null): A | B {
+    return u ?? new B();
+}
+
+function which(u: A | B | null) {
+    if (u !== null) {
+        return u instanceof A ? "A" : "B";
+    }
+
+    return "none";
+}
+
+function text(u: A | B | null): string {
+    return `${u}`;
+}
+
+function nulls() {
+    // a null that came in as a null A, and a literal null
+    for (const u of [widen(null), literal()]) {
+        assert(isNull(u), "=== null");
+        assert(!notNull(u), "!= null");
+        assert(!truthy(u), "falsy");
+        assert(orB(u) instanceof B, "?? B");
+        assert(which(u) == "none", "narrowed by !== null");
+        assert(text(u) == "null", "as text");
+    }
+}
+
+function values() {
+    const a = widen(new A());
+    assert(!isNull(a), "A === null");
+    assert(notNull(a), "A != null");
+    assert(truthy(a), "A truthy");
+    assert(orB(a) instanceof A, "A ?? B");
+    assert(which(a) == "A", "A narrowed");
+    assert(text(a) == "A", "A as text");
+
+    assert(which(new B()) == "B", "B narrowed");
+}
+
+function strings() {
+    let s: string | A | null = null;
+    assert(s === null, "string | A | null holding null");
+    assert(!s, "null falsy");
+    s = "";
+    assert(s !== null, "empty string is not null");
+    assert(!s, "empty string falsy");
+    s = "x";
+    assert(!!s, "string truthy");
+    s = new A();
+    assert(!!s, "A truthy in string | A | null");
+}
+
+function main() {
+    nulls();
+    values();
+    strings();
+    print("done.");
+}