Arrays in an inferred tuple or record literal are heap copies (#479)

tslang/include/TypeScript/LowerToLLVM/ArrayLayout.h

@@ -106,11 +106,11 @@ class ArrayLayout : public LLVMCodeHelperBase
     // MemoryRealloc, which keeps an rc data block's count across the move and births one grown
     // from null with the header's reference.
     //
-    // A static header (makeStatic) is broken under the ops that change an array in place (#479):
-    // it lives in a constant global with capacity = length over static data, and pop, shift,
-    // splice and `length =` store the new length into that constant global and zero the
-    // vacated slots in the static data - undefined behaviour, which today reads back silently
-    // wrong values. Nothing here guards against it.
+    // A static header (makeStatic) must never reach the ops that change an array in place: it lives
+    // in a constant global with capacity = length over static data, and pop, shift, splice and
+    // `length =` would store into that global and zero slots of the static data. MLIRGen copies
+    // a literal's arrays to the heap before anything can change them (#479); nothing here guards
+    // against it.
     mlir::Value ensureCapacity(mlir_ts::ArrayType arrayType, mlir::Value header, mlir::Value needed)
     {
         TypeHelper th(rewriter);

tslang/include/TypeScript/MLIRLogic/MLIRTypeHelper.h

@@ -435,6 +435,33 @@ class MLIRTypeHelper
         return mlir::Attribute();
     }
 
+    // A tuple literal folded to a constant (`[1, [6, 7]]`, `{ items: [1, 2] }`) keeps each array
+    // in it as a static header over constant data (#479): such a tuple has to be rebuilt with
+    // heap copies before anything can change one of its arrays.
+    static bool constTupleHoldsArray(mlir::Type type)
+    {
+        ArrayRef<mlir_ts::FieldInfo> fields;
+        if (auto constTupleType = dyn_cast<mlir_ts::ConstTupleType>(type))
+        {
+            fields = constTupleType.getFields();
+        }
+        else if (auto tupleType = dyn_cast<mlir_ts::TupleType>(type))
+        {
+            fields = tupleType.getFields();
+        }
+
+        return llvm::any_of(fields, [](auto &field) {
+            return isa<mlir_ts::ArrayType>(field.type) || constTupleHoldsArray(field.type);
+        });
+    }
+
+    // a `const` of such a tuple gets storage and is widened to a tuple, as a `const` of a constant
+    // array is widened to an array (processConstRef, adjustLocalVariableType)
+    static bool isConstTupleHoldingArray(mlir::Type type)
+    {
+        return isa<mlir_ts::ConstTupleType>(type) && constTupleHoldsArray(type);
+    }
+
     mlir::Type wideStorageType(mlir::Type type)
     {
         auto actualType = type;

tslang/lib/TypeScript/MLIRGenCast.cpp

@@ -223,6 +223,54 @@ namespace mlirgen
         return mlir::success();
     }    
 
+    // A tuple literal folded to a constant keeps each array in it as a static header over constant
+    // data, so `pop`, `length =` and element writes changed a constant global and `push` reallocated
+    // one (#479). It is rebuilt here as a tuple whose arrays are heap copies, made from the
+    // literal's attributes the way castConstArrayToArray makes a nested array literal; a nested
+    // tuple literal holding an array is rebuilt in turn. Returns no value for anything else.
+    ValueOrLogicalResult MLIRGenImpl::copyArraysOfConstTuple(mlir::Location location, mlir::Value value,
+        mlir_ts::ConstTupleType constTupleType, const GenContext &genContext)
+    {
+        auto constOp = value.getDefiningOp<mlir_ts::ConstantOp>();
+        auto fieldAttrs = constOp ? dyn_cast<mlir::ArrayAttr>(constOp.getValue()) : mlir::ArrayAttr();
+        if (!fieldAttrs || fieldAttrs.size() != constTupleType.size() || !MLIRTypeHelper::constTupleHoldsArray(constTupleType))
+        {
+            return mlir::Value();
+        }
+
+        SmallVector<mlir::Value> values;
+        for (auto [index, fieldInfo] : enumerate(constTupleType.getFields()))
+        {
+            auto fieldAttr = dyn_cast<mlir::ArrayAttr>(fieldAttrs[index]);
+            mlir::Value literal;
+            if (auto arrayType = dyn_cast<mlir_ts::ArrayType>(fieldInfo.type); arrayType && fieldAttr)
+            {
+                literal = builder.create<mlir_ts::ConstantOp>(
+                    location, getConstArrayType(arrayType.getElementType(), fieldAttr.size()), fieldAttr);
+            }
+            else if (MLIRTypeHelper::constTupleHoldsArray(fieldInfo.type) && fieldAttr)
+            {
+                literal = builder.create<mlir_ts::ConstantOp>(
+                    location, mth.convertTupleTypeToConstTupleType(fieldInfo.type), fieldAttr);
+            }
+
+            if (!literal)
+            {
+                MLIRPropertyAccessCodeLogic cl(compileOptions, builder, location, value, builder.getI32IntegerAttr(index));
+                auto fieldValue = cl.Tuple(constTupleType, true);
+                VALIDATE(fieldValue, location)
+                values.push_back(fieldValue);
+                continue;
+            }
+
+            CAST_A(copied, location, mth.convertConstTupleTypeToTupleType(fieldInfo.type), literal, genContext);
+            values.push_back(copied);
+        }
+
+        auto tupleType = mlir::cast<mlir_ts::TupleType>(mth.convertConstTupleTypeToTupleType(constTupleType));
+        return V(builder.create<mlir_ts::CreateTupleOp>(location, tupleType, values));
+    }
+
     ValueOrLogicalResult MLIRGenImpl::castTupleToTuple(mlir::Location location, mlir::Value value, mlir_ts::TupleType srcTupleType, 
         ArrayRef<mlir_ts::FieldInfo> fields, const GenContext &genContext, bool errorAsWarning)
     {
@@ -1042,7 +1090,8 @@ namespace mlirgen
         // copies only the outer data to the heap, and the inner arrays kept pointing at the literal's
         // constant data, so `nested[1].push(4)` reallocated a global and `nested[1][0] = 9` wrote one
         if (constArrayType.getElementType() == arrayType.getElementType()
-            && !isa<mlir_ts::ArrayType>(arrayType.getElementType()))
+            && !isa<mlir_ts::ArrayType>(arrayType.getElementType())
+            && !MLIRTypeHelper::constTupleHoldsArray(arrayType.getElementType()))
         {
             return std::nullopt;
         }
@@ -1072,7 +1121,14 @@ namespace mlirgen
         for (auto elementAttr : elementAttrs)
         {
             mlir::Value element;
-            if (auto nestedAttrs = dyn_cast<mlir::ArrayAttr>(elementAttr))
+            if (auto nestedAttrs = dyn_cast<mlir::ArrayAttr>(elementAttr);
+                nestedAttrs && MLIRTypeHelper::constTupleHoldsArray(sourceElementType))
+            {
+                // a tuple holding an array (`[[1, [2]]]`): rebuilt with copies by its cast (#479)
+                element = builder.create<mlir_ts::ConstantOp>(
+                    location, mth.convertTupleTypeToConstTupleType(sourceElementType), nestedAttrs);
+            }
+            else if (auto nestedAttrs = dyn_cast<mlir::ArrayAttr>(elementAttr))
             {
                 if (!nestedElementType)
                 {
@@ -1183,6 +1239,22 @@ namespace mlirgen
         // const tuple to tuple
         if (auto srcConstTupleType = dyn_cast<mlir_ts::ConstTupleType>(valueType))
         {
+            if (isa<mlir_ts::TupleType, mlir_ts::ConstTupleType, mlir_ts::ClassType>(type))
+            {
+                // the copy is a tuple of the type the casts below read the constant as
+                auto copied = copyArraysOfConstTuple(location, value, srcConstTupleType, genContext);
+                EXIT_IF_FAILED(copied)
+                if (auto copiedValue = V(copied))
+                {
+                    if (copiedValue.getType() == type)
+                    {
+                        return copied;
+                    }
+
+                    value = copiedValue;
+                }
+            }
+
             ::llvm::ArrayRef<::mlir::typescript::FieldInfo> fields;
             if (auto tupleType = dyn_cast<mlir_ts::TupleType>(type))
             {

tslang/lib/TypeScript/MLIRGenExpressions.cpp

@@ -1821,12 +1821,6 @@ namespace mlirgen
             return mlir::failure();
         }
 
-        auto constTupleTypeWithReplacedThis = getConstTupleType(oli.fieldInfos);
-
-        auto arrayAttr = mlir::ArrayAttr::get(builder.getContext(), oli.values);
-        auto constantVal =
-            builder.create<mlir_ts::ConstantOp>(location, constTupleTypeWithReplacedThis, arrayAttr);
-
         // box any literal with a method/accessor as a reference-typed ObjectType, not just
         // synthetic wrappers carrying the explicit flag (see docs/object-literal-boxing-design.md
         // §3 gap 4): a method can only mutate its object through `this`, and predicting which
@@ -1838,6 +1832,49 @@ namespace mlirgen
             (objectLiteral->internalFlags & InternalFlags::BoxAsObject) == InternalFlags::BoxAsObject ||
             !oli.methodInfos.empty() || !oli.methodInfosWithCaptures.empty();
 
+        // A literal built in a slot (a field known only at run time, or boxed) starts from the
+        // constant below, and an array kept in it would be a static header over constant data,
+        // which `push`, `pop` and element writes change (#479): such a field is set in the slot as a
+        // heap copy instead. A literal that stays a constant is copied where it is widened to a
+        // tuple (copyArraysOfConstTuple).
+        if (!oli.fieldsToSet.empty() || boxAsObject)
+        {
+            for (auto [index, fieldInfo] : enumerate(oli.fieldInfos))
+            {
+                auto fieldAttr = dyn_cast<mlir::ArrayAttr>(oli.values[index]);
+                if (!fieldAttr)
+                {
+                    continue;
+                }
+
+                mlir::Value literal;
+                if (auto arrayType = dyn_cast<mlir_ts::ArrayType>(fieldInfo.type))
+                {
+                    literal = builder.create<mlir_ts::ConstantOp>(
+                        location, getConstArrayType(arrayType.getElementType(), fieldAttr.size()), fieldAttr);
+                }
+                else if (MLIRTypeHelper::constTupleHoldsArray(fieldInfo.type))
+                {
+                    literal = builder.create<mlir_ts::ConstantOp>(
+                        location, mth.convertTupleTypeToConstTupleType(fieldInfo.type), fieldAttr);
+                }
+                else
+                {
+                    continue;
+                }
+
+                CAST_A(copied, location, fieldInfo.type, literal, genContext);
+                oli.fieldsToSet.push_back({fieldInfo.id, copied});
+                oli.values[index] = builder.getUnitAttr();
+            }
+        }
+
+        auto constTupleTypeWithReplacedThis = getConstTupleType(oli.fieldInfos);
+
+        auto arrayAttr = mlir::ArrayAttr::get(builder.getContext(), oli.values);
+        auto constantVal =
+            builder.create<mlir_ts::ConstantOp>(location, constTupleTypeWithReplacedThis, arrayAttr);
+
         if (oli.fieldsToSet.empty() && !boxAsObject)
         {
             return V(constantVal);

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -1587,8 +1587,13 @@ class MLIRGenImpl
             // hands its fields over unretained - its receiver is the one that retains them, as a
             // `let` does - so folded, `o` held none of them, and `p.child = d` freed what `o.s`
             // still read (#460).
+            //
+            // And a tuple literal folded to a constant with an array in it (`const st = [1, [6, 7]]`,
+            // `const o = { items: [1, 2] }`): its arrays are static headers over constant data, so
+            // it is widened to a tuple of heap copies once, here, like a const array literal (#479).
             MLIRTypeHelper mth(builder.getContext(), compileOptions);
             needsIdentityStorage = mth.hasBoundMethodField(type) || isa<mlir_ts::ConstArrayType>(type)
+                || MLIRTypeHelper::isConstTupleHoldingArray(type)
                 || ((isa<mlir_ts::ArrayType>(type) || MLIRTypeHelper::isSharedHandleType(type)
                      || (readsLocalSlot(initial) && mth.ownsHeapMemory(location, type))
                      || (readsLiteralSlot(mth, location, initial) && isa<mlir_ts::TupleType>(type)
@@ -1806,10 +1811,11 @@ class MLIRGenImpl
             // write, ...) would still `ts.Cast` a fresh, disposable !ts.array<T> copy
             // out of it instead of sharing one heap array through the slot. Every
             // other const type (generator wrapper tuples, plain values) keeps the
-            // early-return: this widening is only valid/needed for const_array. See
+            // early-return: this widening is only valid/needed for const_array (and a constant
+            // tuple holding an array, which castTupleLikeVariants rebuilds with copies, #479). See
             // docs/const-let-storage-design.md and the const-array note above
             // processConstRef.
-            if (isa<mlir_ts::ConstArrayType>(type) && variableDeclarationInfo.needsIdentityStorage)
+            if ((isa<mlir_ts::ConstArrayType>(type) || MLIRTypeHelper::isConstTupleHoldingArray(type)) && variableDeclarationInfo.needsIdentityStorage)
             {
                 auto actualType = mth.removeConstType(type);
                 if (variableDeclarationInfo.initial && actualType != type)
@@ -1858,7 +1864,7 @@ class MLIRGenImpl
             // <T,N>` snapshot, or every mutating method/element write still operates
             // on a disposable ts.Cast copy or the read-only original. See the note
             // above processConstRef / const-let-storage-design.md.
-            if (isa<mlir_ts::ConstArrayType>(variableDeclarationInfo.type))
+            if (isa<mlir_ts::ConstArrayType>(variableDeclarationInfo.type) || MLIRTypeHelper::isConstTupleHoldingArray(variableDeclarationInfo.type))
             {
                 auto type = variableDeclarationInfo.type;
                 auto actualType = mth.removeConstType(type);
@@ -9634,8 +9640,16 @@ class MLIRGenImpl
         SmallVector<mlir_ts::FieldInfo> fieldInfos;
         for (auto val : values)
         {
-            fieldInfos.push_back({mlir::Attribute(), val.value.getType(), false, mlir_ts::AccessLevel::Public});
-            arrayValues.push_back(val.value);
+            auto value = val.value;
+            // a constant tuple literal inside (`[x, [2, [3, 4]]]`) holds its arrays as static
+            // headers: it becomes a tuple of heap copies (#479)
+            if (MLIRTypeHelper::isConstTupleHoldingArray(value.getType()))
+            {
+                CAST(value, location, mth.convertConstTupleTypeToTupleType(value.getType()), value, genContext);
+            }
+
+            fieldInfos.push_back({mlir::Attribute(), value.getType(), false, mlir_ts::AccessLevel::Public});
+            arrayValues.push_back(value);
         }
 
         return V(builder.create<mlir_ts::CreateTupleOp>(location, getTupleType(fieldInfos), arrayValues));
@@ -12063,6 +12077,9 @@ class MLIRGenImpl
     ValueOrLogicalResult mapTupleToFields(mlir::Location location, SmallVector<mlir::Value> &values, mlir::Value value, mlir_ts::TupleType srcTupleType, 
         ::llvm::ArrayRef<::mlir::typescript::FieldInfo> fields, bool filterSpecialCases, const GenContext &genContext, bool errorAsWarning = false);
 
+    ValueOrLogicalResult copyArraysOfConstTuple(mlir::Location location, mlir::Value value, mlir_ts::ConstTupleType constTupleType,
+        const GenContext &genContext);
+
 
     ValueOrLogicalResult castTupleToTuple(mlir::Location location, mlir::Value value, mlir_ts::TupleType srcTupleType, 
         ArrayRef<mlir_ts::FieldInfo> fields, const GenContext &genContext, bool errorAsWarning = false);

tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp

@@ -304,6 +304,12 @@ class OwnedReturnConsumptionPass
                 return;
             }
 
+            if (bornUncounted(value))
+            {
+                retainForGlobal(resultOp, resultOp->getOperand(0));
+                return;
+            }
+
             // An optional widened into a union (`let g: C | null | undefined = mkU()`, #462) is a
             // ts.If over the call made before it: each branch yields that call's value through
             // views, or a value that holds nothing (the empty optional). The global takes the call's
@@ -405,6 +411,34 @@ class OwnedReturnConsumptionPass
         });
     }
 
+    // An array or tuple built in a global's region (`const a = [1, 2]`, `const t = [1, [6, 7]]`, or
+    // a field of `const o = { items: [1, 2] }`) is born at count 0 under rc: a local takes its first
+    // count with RetainSlot, and nothing took one for the global, so the first local that read it
+    // and let go freed it. A global is a root and holds a count of its own (§9.49). Under own the
+    // global owns what it holds without a count, and a retain there is an error.
+    // A constant array reaches the global through its cast to an array, which copies it to the heap;
+    // any other constant (a string literal, an RTTI name) is static and immortal, and its global
+    // stays a constant initializer.
+    static bool bornUncounted(mlir::Value value)
+    {
+        auto *definingOp = value.getDefiningOp();
+        return definingOp && (mlir::isa<mlir_ts::CreateArrayOp, mlir_ts::NewArrayOp, mlir_ts::NewEmptyArrayOp,
+                                        mlir_ts::CreateTupleOp>(definingOp) ||
+                              mlir::isa<mlir_ts::ConstArrayType>(value.getType()));
+    }
+
+    void retainForGlobal(mlir::Operation *before, mlir::Value value)
+    {
+        if (!compileOptions.isRefCounted())
+        {
+            return;
+        }
+
+        mlir::OpBuilder builder(before->getContext());
+        builder.setInsertionPoint(before);
+        builder.create<mlir_ts::RetainOp>(before->getLoc(), value);
+    }
+
     // Each owned producer stored into a field of an object literal's temporary slot, through the
     // views of claimGlobalInitializers, is taken over by whoever takes the literal; a field that is
     // itself a literal read out of its own temporary slot is followed into.
@@ -452,6 +486,10 @@ class OwnedReturnConsumptionPass
                     stored.getDefiningOp()->setAttr(OWNED_RESULT_CONSUMED_ATTR_NAME,
                                                     mlir::UnitAttr::get(&getContext()));
                 }
+                else if (bornUncounted(stored))
+                {
+                    retainForGlobal(storeOp, storeOp.getValue());
+                }
                 else if (auto innerLoad = stored.getDefiningOp<mlir_ts::LoadOp>())
                 {
                     if (auto innerSlot = innerLoad.getReference().getDefiningOp<mlir_ts::VariableOp>();