A value a builtin cannot cast is an error, not a compiler crash (#438)

tslang/include/TypeScript/MLIRLogic/MLIRCodeLogic.h

@@ -553,13 +553,21 @@ class MLIRCustomMethods
         if (!isa<mlir::IndexType>(bufferSize.getType()))
         {
             bufferSize = castFn(location, mlir::IndexType::get(builder.getContext()), bufferSize, genContext, false);
+            if (!bufferSize)
+            {
+                return mlir::failure();
+            }
         }
 
         auto stringType = mlir_ts::StringType::get(builder.getContext());
         format = operands[1];
         if (!isa<mlir_ts::StringType>(format.getType()))
         {
             format = castFn(location, stringType, format, genContext, false);
+            if (!format)
+            {
+                return mlir::failure();
+            }
         }
 
         SmallVector<mlir::Value> vals;
@@ -618,6 +626,10 @@ class MLIRCustomMethods
         if (!isa<mlir_ts::BooleanType>(op.getType()))
         {
             op = castFn(location, mlir_ts::BooleanType::get(builder.getContext()), op, genContext, false);
+            if (!op)
+            {
+                return mlir::failure();
+            }
         }
 
         auto assertOp =
@@ -626,7 +638,7 @@ class MLIRCustomMethods
         return mlir::success();
     }
 
-    mlir::Value mlirGenParseInt(const mlir::Location &location, ArrayRef<mlir::Value> operands,
+    ValueOrLogicalResult mlirGenParseInt(const mlir::Location &location, ArrayRef<mlir::Value> operands,
         std::function<ValueOrLogicalResult(mlir::Location, mlir::Type, mlir::Value, const GenContext &, bool)> castFn, const GenContext &genContext)
     {
         auto hasTwoOps = operands.size() == 2;
@@ -636,6 +648,10 @@ class MLIRCustomMethods
         if (!isa<mlir_ts::StringType>(op.getType()))
         {
             op = castFn(location, mlir_ts::StringType::get(builder.getContext()), op, genContext, false);
+            if (!op)
+            {
+                return mlir::failure();
+            }
         }
 
         if (hasTwoOps)
@@ -644,42 +660,54 @@ class MLIRCustomMethods
             if (!isa<mlir::IntegerType>(op2.getType()))
             {
                 op2 = castFn(location, mlir::IntegerType::get(builder.getContext(), 32), op2, genContext, false);
+                if (!op2)
+                {
+                    return mlir::failure();
+                }
             }
         }
 
         auto parseIntOp = hasTwoOps ? builder.create<mlir_ts::ParseIntOp>(location, builder.getI32Type(), op, op2)
                                     : builder.create<mlir_ts::ParseIntOp>(location, builder.getI32Type(), op);
 
-        return parseIntOp;
+        return V(parseIntOp);
     }
 
-    mlir::Value mlirGenParseFloat(const mlir::Location &location, ArrayRef<mlir::Value> operands,
+    ValueOrLogicalResult mlirGenParseFloat(const mlir::Location &location, ArrayRef<mlir::Value> operands,
         std::function<ValueOrLogicalResult(mlir::Location, mlir::Type, mlir::Value, const GenContext &, bool)> castFn, const GenContext &genContext)
     {
         auto op = operands.front();
         if (!isa<mlir_ts::StringType>(op.getType()))
         {
             op = castFn(location, mlir_ts::StringType::get(builder.getContext()), op, genContext, false);
+            if (!op)
+            {
+                return mlir::failure();
+            }
         }
 
         auto parseFloatOp =
             builder.create<mlir_ts::ParseFloatOp>(location, mlir_ts::NumberType::get(builder.getContext()), op);
 
-        return parseFloatOp;
+        return V(parseFloatOp);
     }
 
-    mlir::Value mlirGenIsNaN(const mlir::Location &location, ArrayRef<mlir::Value> operands,
+    ValueOrLogicalResult mlirGenIsNaN(const mlir::Location &location, ArrayRef<mlir::Value> operands,
         std::function<ValueOrLogicalResult(mlir::Location, mlir::Type, mlir::Value, const GenContext &, bool)> castFn, const GenContext &genContext)
     {
         auto op = operands.front();
         if (!isa<mlir_ts::NumberType>(op.getType()))
         {
             op = castFn(location, mlir_ts::NumberType::get(builder.getContext()), op, genContext, false);
+            if (!op)
+            {
+                return mlir::failure();
+            }
         }
 
         auto isNaNOp = builder.create<mlir_ts::IsNaNOp>(location, mlir_ts::BooleanType::get(builder.getContext()), op);
 
-        return isNaNOp;
+        return V(isNaNOp);
     }
 
     mlir::Value mlirGenSizeOf(const mlir::Location &location, mlir::SmallVector<mlir::Type> typeArgs, ArrayRef<mlir::Value> operands)
@@ -776,6 +804,32 @@ class MLIRCustomMethods
         }
     }
 
+    // Each value cast to `type`, into `castedValues`. A value the cast rejects fails the builtin:
+    // its null result went on to the op's builder, which crashed the compiler (#438).
+    mlir::LogicalResult castEach(const mlir::Location &location, mlir::Type type, ArrayRef<mlir::Value> values,
+        SmallVector<mlir::Value> &castedValues,
+        std::function<ValueOrLogicalResult(mlir::Location, mlir::Type, mlir::Value, const GenContext &, bool)> castFn, const GenContext &genContext)
+    {
+        for (auto value : values)
+        {
+            if (value.getType() == type)
+            {
+                castedValues.push_back(value);
+                continue;
+            }
+
+            auto casted = castFn(location, type, value, genContext, false);
+            if (casted.failed_or_no_value())
+            {
+                return mlir::failure();
+            }
+
+            castedValues.push_back(V(casted));
+        }
+
+        return mlir::success();
+    }
+
     ValueOrLogicalResult mlirGenArrayPush(const mlir::Location &location, mlir::Value thisValue, ArrayRef<mlir::Value> values,
         std::function<ValueOrLogicalResult(mlir::Location, mlir::Type, mlir::Value, const GenContext &, bool)> castFn, const GenContext &genContext)
     {
@@ -784,16 +838,9 @@ class MLIRCustomMethods
         auto arrayElement = cast<mlir_ts::ArrayType>(thisValue.getType()).getElementType();
 
         SmallVector<mlir::Value> castedValues;
-        for (auto value : values)
+        if (mlir::failed(castEach(location, arrayElement, values, castedValues, castFn, genContext)))
         {
-            if (value.getType() != arrayElement)
-            {
-                castedValues.push_back(castFn(location, arrayElement, value, genContext, false));
-            }
-            else
-            {
-                castedValues.push_back(value);
-            }
+            return mlir::failure();
         }
 
         auto thisValueLoaded = mcl.GetReferenceFromValue(location, thisValue);
@@ -847,16 +894,9 @@ class MLIRCustomMethods
         auto arrayElement = cast<mlir_ts::ArrayType>(thisValue.getType()).getElementType();
 
         SmallVector<mlir::Value> castedValues;
-        for (auto value : values)
+        if (mlir::failed(castEach(location, arrayElement, values, castedValues, castFn, genContext)))
         {
-            if (value.getType() != arrayElement)
-            {
-                castedValues.push_back(castFn(location, arrayElement, value, genContext, false));
-            }
-            else
-            {
-                castedValues.push_back(value);
-            }
+            return mlir::failure();
         }
 
         auto thisValueLoaded = mcl.GetReferenceFromValue(location, thisValue);
@@ -917,6 +957,10 @@ class MLIRCustomMethods
             if (value.getType() != si64Type)
             {
                 value = castFn(location, si64Type, value, genContext, false);
+                if (!value)
+                {
+                    return value;
+                }
             }
 
             return castFn(location, mlir::IndexType::get(builder.getContext()), value, genContext, false);
@@ -932,16 +976,9 @@ class MLIRCustomMethods
         auto arrayElement = cast<mlir_ts::ArrayType>(thisValue.getType()).getElementType();
 
         SmallVector<mlir::Value> castedValues;
-        for (auto value : values)
+        if (mlir::failed(castEach(location, arrayElement, values, castedValues, castFn, genContext)))
         {
-            if (value.getType() != arrayElement)
-            {
-                castedValues.push_back(castFn(location, arrayElement, value, genContext, false));
-            }
-            else
-            {
-                castedValues.push_back(value);
-            }
+            return mlir::failure();
         }
 
         auto thisValueLoaded = mcl.GetReferenceFromValue(location, thisValue);
@@ -986,16 +1023,9 @@ class MLIRCustomMethods
         auto indexType = builder.getIndexType();
 
         SmallVector<mlir::Value> castedValues;
-        for (auto value : values)
+        if (mlir::failed(castEach(location, indexType, values, castedValues, castFn, genContext)))
         {
-            if (value.getType() != indexType)
-            {
-                castedValues.push_back(castFn(location, indexType, value, genContext, false));
-            }
-            else
-            {
-                castedValues.push_back(value);
-            }
+            return mlir::failure();
         }
 
         auto count = builder.create<mlir_ts::ArithmeticBinaryOp>(
@@ -1033,6 +1063,10 @@ class MLIRCustomMethods
         if (op.getType() != int32Type)
         {
             op = castFn(location, int32Type, op, genContext, false);
+            if (!op)
+            {
+                return mlir::failure();
+            }
         }
 
         auto switchStateOp =
@@ -1765,6 +1799,10 @@ class MLIRPropertyAccessCodeLogic
                     MLIRTypeHelper mth(builder.getContext(), compileOptions);
                     auto nonConstArray = mth.convertConstArrayTypeToArrayType(expression.getType());
                     expression = castFn(location, nonConstArray, expression, genContext, false);
+                    if (!expression)
+                    {
+                        return mlir::Value();
+                    }
                 }
                 else
                 {

tslang/test/tester/CMakeLists.txt

@@ -350,6 +350,7 @@ tslang_add_test(NAME test-compile-00-array-static-nested COMMAND test-runner "${
 tslang_add_test(NAME test-compile-00-array-literal-copy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_literal_copy.ts")
 tslang_add_test(NAME test-compile-00-tuple-runtime-arrays COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00tuple_runtime_arrays.ts")
 tslang_add_test(NAME test-compile-00-record-to-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00record_to_class.ts")
+tslang_add_test(NAME test-compile-00-array-insert-functions COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_insert_functions.ts")
 tslang_add_test(NAME test-compile-00-const-record-owned-fields COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_record_owned_fields.ts")
 tslang_add_test(NAME test-compile-00-array-length-valid COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts")
 tslang_add_test(NAME test-compile-00-string-view-copy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_view_copy.ts")
@@ -857,6 +858,7 @@ tslang_add_test(NAME test-jit-00-array-static-nested COMMAND test-runner -jit "$
 tslang_add_test(NAME test-jit-00-array-literal-copy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_literal_copy.ts")
 tslang_add_test(NAME test-jit-00-tuple-runtime-arrays COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00tuple_runtime_arrays.ts")
 tslang_add_test(NAME test-jit-00-record-to-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00record_to_class.ts")
+tslang_add_test(NAME test-jit-00-array-insert-functions COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_insert_functions.ts")
 tslang_add_test(NAME test-jit-00-const-record-owned-fields COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_record_owned_fields.ts")
 tslang_add_test(NAME test-jit-00-array-length-valid COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts")
 tslang_add_test(NAME test-jit-00-string-view-copy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_view_copy.ts")
@@ -1693,6 +1695,7 @@ set(TSLANG_CORPUS
     00array_literal_copy.ts
     00tuple_runtime_arrays.ts
     00record_to_class.ts
+    00array_insert_functions.ts
     00const_record_owned_fields.ts
     00array_length_valid.ts
     00string_view_copy.ts
@@ -3178,6 +3181,29 @@ foreach(object_literal_to_class_name method accessor parameter return)
                                     FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed")
 endforeach()
 
+# A value a builtin cannot cast to what it takes is the cast's error; the failed cast's null value
+# went on to the op's builder and crashed the compiler (#438).
+set(builtin_cast_cases
+    "push|can't assign '[(][)] => s32' to '[(][)] => number'"
+    "unshift|can't assign '[(][)] => s32' to '[(][)] => number'"
+    "splice|can't assign '[(][)] => s32' to '[(][)] => number'"
+    "push_string|invalid cast from [(][)] => s32 to string"
+    "parse_int|invalid cast from [(][)] => s32 to string"
+    "is_nan|invalid cast from [(][)] => s32 to number")
+foreach(builtin_cast_case ${builtin_cast_cases})
+    string(FIND "${builtin_cast_case}" "|" builtin_cast_sep)
+    string(SUBSTRING "${builtin_cast_case}" 0 ${builtin_cast_sep} builtin_cast_name)
+    math(EXPR builtin_cast_sep "${builtin_cast_sep} + 1")
+    string(SUBSTRING "${builtin_cast_case}" ${builtin_cast_sep} -1 builtin_cast_message)
+    add_test(NAME test-compile-builtin-cast-${builtin_cast_name}
+             COMMAND $<TARGET_FILE:tslang> --emit=obj --no-default-lib -mm=none
+                     "${PROJECT_SOURCE_DIR}/test/tester/builtin-cast/${builtin_cast_name}.ts"
+                     -o "${CMAKE_CURRENT_BINARY_DIR}/builtin-cast-${builtin_cast_name}.obj")
+    set_tests_properties(test-compile-builtin-cast-${builtin_cast_name}
+                         PROPERTIES PASS_REGULAR_EXPRESSION "${builtin_cast_message}"
+                                    FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed")
+endforeach()
+
 # The shared-component tier under the other two models. A shared library records the model
 # it was built under, so both halves of a pair are built with the same flag - which is what
 # these run. The file pairs are the default model's, verbatim.

tslang/test/tester/builtin-cast/is_nan.ts

@@ -0,0 +1,5 @@
+// A value the builtin cannot cast to what it takes crashed the compiler (#438): the failed cast
+// was a null value, which went on to the op's builder.
+function main() {
+    print(isNaN(() => 1));
+}

tslang/test/tester/builtin-cast/parse_int.ts

@@ -0,0 +1,5 @@
+// A value the builtin cannot cast to what it takes crashed the compiler (#438): the failed cast
+// was a null value, which went on to the op's builder.
+function main() {
+    print(parseInt(() => 1));
+}

tslang/test/tester/builtin-cast/push.ts

@@ -0,0 +1,7 @@
+// A value the builtin cannot cast to what it takes crashed the compiler (#438): the failed cast
+// was a null value, which went on to the op's builder.
+function main() {
+    let h: (() => number)[] = [];
+    h.push(() => 1);
+    print(h.length);
+}

tslang/test/tester/builtin-cast/push_string.ts

@@ -0,0 +1,7 @@
+// A value the builtin cannot cast to what it takes crashed the compiler (#438): the failed cast
+// was a null value, which went on to the op's builder.
+function main() {
+    let s: string[] = [];
+    s.push(() => 1);
+    print(s.length);
+}

tslang/test/tester/builtin-cast/splice.ts

@@ -0,0 +1,7 @@
+// A value the builtin cannot cast to what it takes crashed the compiler (#438): the failed cast
+// was a null value, which went on to the op's builder.
+function main() {
+    let h: (() => number)[] = [];
+    h.splice(0, 0, () => 1);
+    print(h.length);
+}

tslang/test/tester/builtin-cast/unshift.ts

@@ -0,0 +1,7 @@
+// A value the builtin cannot cast to what it takes crashed the compiler (#438): the failed cast
+// was a null value, which went on to the op's builder.
+function main() {
+    let h: (() => number)[] = [];
+    h.unshift(() => 1);
+    print(h.length);
+}

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

@@ -0,0 +1,12 @@
+// Functions pushed, unshifted and spliced into an array of functions (#438): each is cast to the
+// element type, and one that cannot be is a compile error, not a crash (builtin-cast/*.ts).
+function main() {
+    let h: (() => number)[] = [];
+    let n: number = 0;
+    h.push(() => n);
+    h.unshift(() => n + 1);
+    h.splice(1, 0, () => n + 2);
+    assert(h.length == 3, "three functions");
+    assert(h[0]() == 1 && h[1]() == 2 && h[2]() == 0, "each in its place");
+    print("done.");
+}