rc: a const read out of an element keeps a count of its own

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -1731,6 +1731,25 @@ class MLIRGenImpl
             return loadOp && loadOp.getReference().getDefiningOp<mlir_ts::PropertyRefOp>();
         }
 
+        // A value read out of an element, through any casts: what `a[0]` is in `const c = a[0]`, and
+        // what `b` is in `const [b] = a`, read there only when `a` is long enough (the `then` of an if).
+        static bool readsElement(mlir::Value value)
+        {
+            while (auto castOp = value ? value.getDefiningOp<mlir_ts::CastOp>() : mlir_ts::CastOp())
+            {
+                value = castOp.getIn();
+            }
+
+            if (auto ifOp = value ? value.getDefiningOp<mlir_ts::IfOp>() : mlir_ts::IfOp())
+            {
+                auto resultOp = dyn_cast<mlir_ts::ResultOp>(ifOp.getThenRegion().front().getTerminator());
+                return resultOp && readsElement(resultOp.getOperand(mlir::cast<mlir::OpResult>(value).getResultNumber()));
+            }
+
+            auto loadOp = value ? value.getDefiningOp<mlir_ts::LoadOp>() : mlir_ts::LoadOp();
+            return loadOp && loadOp.getReference().getDefiningOp<mlir_ts::ElementRefOp>();
+        }
+
         // A record read out whole from the temporary slot an object literal is built in (one that
         // owns nothing of its own), with a field that owns a block and that only the literal's
         // receiver counts - a `let` does, with RetainSlot - so a `const` of it needs storage like a
@@ -1861,11 +1880,9 @@ class MLIRGenImpl
             // interface or a bound method made over such a read (`const i: I = a`, `const m = a.get`):
             // it holds the block as `b` would.
             //
-            // Under rc, so does a const read out of a field (`const c = h.c`): folded, `h.c = d` freed
-            // what `c` still read. Under own the folded read is a borrow, and a use after the overwrite
-            // is an error there. Not yet an element: a destructuring past the end of an array
-            // (`const [a, b, c] = pair`) reads whatever follows its data, and a count taken of that
-            // faults, as it does for a `let` today.
+            // Under rc, so does a const read out of a field (`const c = h.c`) or an element (`const c =
+            // a[0]`, `const [c] = a`): folded, `h.c = d` or `a[0] = d` freed what `c` still read. Under
+            // own the folded read is a borrow, and a use after the overwrite is an error there.
             //
             // And a const record whose fields own blocks (`const o = { s: p.child }`): the literal
             // hands its fields over unretained - its receiver is the one that retains them, as a
@@ -1884,7 +1901,8 @@ class MLIRGenImpl
                 || MLIRTypeHelper::isConstTupleHoldingArray(type)
                 || ((isa<mlir_ts::ArrayType>(type) || MLIRTypeHelper::isSharedHandleType(type)
                      || ((readsLocalSlot(initial) || viewsLocalSlot(initial)) && mth.ownsHeapMemory(location, type))
-                     || (compileOptions.isRefCounted() && readsField(initial) && mth.ownsHeapMemory(location, type))
+                     || (compileOptions.isRefCounted() && (readsField(initial) || readsElement(initial))
+                         && mth.ownsHeapMemory(location, type))
                      || (readsLiteralSlot(mth, location, initial) && isa<mlir_ts::TupleType>(type)
                          && mth.ownsHeapMemory(location, type))
                      || (initial && initial.getDefiningOp<mlir_ts::CreateTupleOp>() && isa<mlir_ts::TupleType>(type)

tslang/lib/TypeScript/MLIRGenVariables.cpp

@@ -415,6 +415,12 @@ namespace mlirgen
                         return V(mlirGenArrayRestCopy(location, mlir::cast<mlir_ts::ArrayType>(arrayType), arrayValue, index, genContext));
                     }
 
+                    // past the end: the default of the element type, null for a reference (see below)
+                    if (index >= constArrayType.getSize())
+                    {
+                        return V(builder.create<mlir_ts::DefaultOp>(location, constArrayType.getElementType()));
+                    }
+
                     // TODO: unify it with ElementAccess
                     auto constIndex = builder.create<mlir_ts::ConstantOp>(location, builder.getI32Type(),
                                                                         builder.getI32IntegerAttr(index));
@@ -429,12 +435,30 @@ namespace mlirgen
                         return V(mlirGenArrayRestCopy(location, arrayType, init, index, genContext));
                     }
 
-                    // TODO: unify it with ElementAccess
-                    auto constIndex = builder.create<mlir_ts::ConstantOp>(location, builder.getI32Type(),
-                                                                        builder.getI32IntegerAttr(index));
-                    auto elemRef = builder.create<mlir_ts::ElementRefOp>(
-                        location, mlir_ts::RefType::get(arrayType.getElementType()), init, constIndex);
-                    return V(builder.create<mlir_ts::LoadOp>(location, arrayType.getElementType(), elemRef));
+                    // An element past the end of the array (`const [a, b, c] = pair`) is the default
+                    // of its type - null for a reference, where TS has undefined - not whatever follows
+                    // the array's data: a read of that was undefined behaviour, and under rc a count
+                    // taken of it faulted.
+                    auto lengthValue = builder.create<mlir_ts::LengthOfOp>(location, builder.getIndexType(), init);
+                    auto indexValue = builder.create<mlir_ts::ConstantOp>(location, mth.getIndexType(),
+                                                                          mth.getIndexAttrValue(index));
+                    auto inBoundsValue = V(builder.create<mlir_ts::LogicalBinaryOp>(location, getBooleanType(),
+                        builder.getI32IntegerAttr((int)SyntaxKind::GreaterThanToken), lengthValue, indexValue));
+
+                    auto result = conditionalValue(location, inBoundsValue,
+                        [&]() {
+                            // TODO: unify it with ElementAccess
+                            auto constIndex = builder.create<mlir_ts::ConstantOp>(location, builder.getI32Type(),
+                                                                                builder.getI32IntegerAttr(index));
+                            auto elemRef = builder.create<mlir_ts::ElementRefOp>(
+                                location, mlir_ts::RefType::get(arrayType.getElementType()), init, constIndex);
+                            return ValueOrLogicalResult(
+                                V(builder.create<mlir_ts::LoadOp>(location, arrayType.getElementType(), elemRef)));
+                        },
+                        [&](mlir::Type elementType) {
+                            return ValueOrLogicalResult(V(builder.create<mlir_ts::DefaultOp>(location, elementType)));
+                        });
+                    return result.failed_or_no_value() ? mlir::Value() : V(result);
                 })
                 .Default([&](auto type) { 
 

tslang/test/tester/CMakeLists.txt

@@ -398,6 +398,7 @@ tslang_add_test(NAME test-compile-00-string-view-copy COMMAND test-runner "${PRO
 tslang_add_test(NAME test-compile-00-global-copy-counted COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_copy_counted.ts")
 tslang_add_test(NAME test-compile-00-c-string-copy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00c_string_copy.ts")
 tslang_add_test(NAME test-compile-00-const-field-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_field_read.ts")
+tslang_add_test(NAME test-compile-00-const-element-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_element_read.ts")
 tslang_add_test(NAME test-compile-00-array-data-string-alias COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_data_string_alias.ts")
 tslang_add_test(NAME test-compile-00-arrays7 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts")
 tslang_add_test(NAME test-compile-00-arrays8-tuple-spread COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array8_tuple_spread.ts")
@@ -970,6 +971,7 @@ tslang_add_test(NAME test-jit-00-string-view-copy COMMAND test-runner -jit "${PR
 tslang_add_test(NAME test-jit-00-global-copy-counted COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_copy_counted.ts")
 tslang_add_test(NAME test-jit-00-c-string-copy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00c_string_copy.ts")
 tslang_add_test(NAME test-jit-00-const-field-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_field_read.ts")
+tslang_add_test(NAME test-jit-00-const-element-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_element_read.ts")
 tslang_add_test(NAME test-jit-00-array-data-string-alias COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_data_string_alias.ts")
 tslang_add_test(NAME test-jit-00-arrays7 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts")
 tslang_add_test(NAME test-jit-00-arrays8-tuple-spread COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array8_tuple_spread.ts")
@@ -1932,6 +1934,7 @@ set(TSLANG_CORPUS
     00global_copy_counted.ts
     00c_string_copy.ts
     00const_field_read.ts
+    00const_element_read.ts
     00array_data_string_alias.ts
     00array7.ts
     00array8_tuple_spread.ts

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

@@ -0,0 +1,75 @@
+// A `const` read out of an element keeps what it read alive when the element is given a new value.
+// Folded, the const was the read itself and held no reference of its own: under rc the overwrite
+// freed the block it still read. Allocations of the same size reuse anything freed before each
+// read. A destructuring past the end of an array gives the element type's default (null for a
+// reference), not whatever follows the array's data.
+class C {
+    constructor(public x: number) {}
+}
+
+class Junk {
+    a = -1;
+    b = -2;
+}
+
+function churn() {
+    let keep: Junk[] = [];
+    for (let i = 0; i < 1000; i++) keep.push(new Junk());
+    return keep.length;
+}
+
+function element(i: number) {
+    const arr: C[] = [new C(0), new C(0)];
+    arr[0] = new C(i);
+    const c = arr[0];
+    arr[0] = new C(9);
+    churn();
+    return c.x == i && arr[0].x == 9;
+}
+
+function destructured(i: number) {
+    const arr: C[] = [new C(0), new C(i)];
+    const [a, c] = arr;
+    arr[1] = new C(9);
+    churn();
+    return c.x == i && a.x == 0 && arr[1].x == 9;
+}
+
+function stringElement(i: number) {
+    const words: string[] = ["a", "b"];
+    words[0] = "w" + i;
+    const w = words[0];
+    words[0] = "x" + i;
+    churn();
+    return w == "w" + i;
+}
+
+function pastTheEnd() {
+    const pair: C[] = [new C(1), new C(2)];
+    const [a, b, c] = pair;
+    let [d, e, f] = pair;
+    assert(a.x == 1 && b.x == 2 && d.x == 1 && e.x == 2, "in bounds");
+    assert(c == null && f == null, "a class element past the end is null");
+
+    const words: string[] = ["a", "b"];
+    const [w0, w1, w2] = words;
+    assert(w0 == "a" && w1 == "b" && w2 == null, "a string element past the end is null");
+
+    const nums: number[] = [1, 2];
+    const [n0, n1, n2] = nums;
+    assert(n0 == 1 && n1 == 2 && n2 == 0, "a number element past the end is 0");
+
+    const [k0, k1] = [7];
+    assert(k0 == 7 && k1 == 0, "past the end of an array literal");
+}
+
+function main() {
+    for (let i = 1; i < 4; i++) {
+        assert(element(i), "an element's overwrite frees what a const read of it holds");
+        assert(destructured(i), "an element's overwrite frees what a const destructured from it holds");
+        assert(stringElement(i), "a string element");
+    }
+
+    pastTheEnd();
+    print("done.");
+}