diff --git a/tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h b/tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h index 979ee8513..2058c08fb 100644 --- a/tslang/include/TypeScript/LowerToLLVM/CastLogicHelper.h +++ b/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(baseType).getTypes(); - if (isPointerNullableType(inType) + if (MLIRTypeHelper::isNullAbsorbingType(inType) && llvm::any_of(storageTypes, [](mlir::Type type) { return isa(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(type); - } - bool isBool(mlir::Type type) { return type.isInteger(1); diff --git a/tslang/include/TypeScript/LowerToLLVM/UnaryBinLogicalOrHelper.h b/tslang/include/TypeScript/LowerToLLVM/UnaryBinLogicalOrHelper.h index a5dc95d2a..e60eac808 100644 --- a/tslang/include/TypeScript/LowerToLLVM/UnaryBinLogicalOrHelper.h +++ b/tslang/include/TypeScript/LowerToLLVM/UnaryBinLogicalOrHelper.h @@ -110,6 +110,32 @@ mlir::Value LogicOp(Operation *binOp, SyntaxKind op, mlir::Value left, mlir::Typ : builder.create(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(actualType); + if (actualUnionType && mth.isNullAbsorbingUnion(actualUnionType)) + { + // the first word of the value field: the pointer every member holds there + auto pointerWord = builder.create(loc, llvmUnion, ArrayRef{UNION_VALUE_INDEX, 0, 0}); + auto zero = builder.create(loc, pointerWord.getType()); + return builder.create(loc, v1, pointerWord, zero); + } + + auto tagValue = builder.create(loc, llvmtch.typeConverter->convertType(mth.getStringType()), + llvmUnion, MLIRHelper::getStructIndex(builder, UNION_TAG_INDEX)); + auto nullStr = builder.create(loc, mth.getStringType(), builder.getStringAttr("null")); + return builder.create( + loc, mth.getBooleanType(), tagValue, nullStr, builder.getI32IntegerAttr((int)op)); + }; + if (isa(leftType) || isa(rightType)) { return OptionalTypeLogicalOp(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(loc, llvmtch.typeConverter->convertType(mth.getStringType()), - asLLVMUnion(right, unionType), MLIRHelper::getStructIndex(builder, UNION_TAG_INDEX)); - auto nullStr = builder.create(loc, mth.getStringType(), builder.getStringAttr("null")); - auto cmpOp = builder.create( - 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() || right.getDefiningOp() || matchPattern(right, m_Zero()); if (isRightNullValue) { - auto tagValue = builder.create(loc, llvmtch.typeConverter->convertType(mth.getStringType()), - asLLVMUnion(left, unionType), MLIRHelper::getStructIndex(builder, UNION_TAG_INDEX)); - auto nullStr = builder.create(loc, mth.getStringType(), builder.getStringAttr("null")); - auto cmpOp = builder.create( - 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(leftType) << "'"; diff --git a/tslang/include/TypeScript/MLIRLogic/MLIRTypeHelper.h b/tslang/include/TypeScript/MLIRLogic/MLIRTypeHelper.h index b3790fbd9..bcb0a8318 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIRTypeHelper.h +++ b/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(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(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 types) { - return llvm::count_if(types, [](mlir::Type type) { return !isa(type); }) > 1; + auto nonNull = llvm::count_if(types, [](mlir::Type type) { return !isa(type); }); + return nonNull > 1 && !llvm::all_of(types, [](mlir::Type type) { + return isa(type) || isNullAbsorbingType(type); + }); } // TODO: review all union merge logic @@ -3409,7 +3430,8 @@ class MLIRTypeHelper mlir::SmallVector 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()); } diff --git a/tslang/lib/TypeScript/MLIRGenCast.cpp b/tslang/lib/TypeScript/MLIRGenCast.cpp index 445feba6f..e050ca1d9 100644 --- a/tslang/lib/TypeScript/MLIRGenCast.cpp +++ b/tslang/lib/TypeScript/MLIRGenCast.cpp @@ -2254,6 +2254,33 @@ namespace mlirgen return mlir::failure(); } + auto toBoolean = isa(type) || isa(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(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(type) + || (isa(type) + && llvm::any_of(mlir::cast(type).getTypes(), [](mlir::Type member) { return isa(member); })) + || (!compileOptions.strictNullChecks && MLIRTypeHelper::isNullAbsorbingType(type)); + ss << S("if (t === null) ") + << (toBoolean ? S("return false;") + : isa(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(type) || isa(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"); diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index aa7bf3194..aa3f0efbe 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/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(rightExpressionValue.getType())) + || (isRightUnion && isa(leftExpressionValue.getType()))) + { + return isLeftUnion ? leftExpressionValue : rightExpressionValue; + } + if (isLeftUnion && isRightUnion) { // TODO: finish cast between unions diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 74d71b5c8..a0392616d 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/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 diff --git a/tslang/test/tester/tests/00union_null_in_pointer.ts b/tslang/test/tester/tests/00union_null_in_pointer.ts new file mode 100644 index 000000000..4770d1ede --- /dev/null +++ b/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."); +}