A temporary a finally reads is released after the finally

tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp

@@ -17,6 +17,9 @@
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Support/Debug.h"
 
+#include <set>
+#include <tuple>
+
 #define DEBUG_TYPE "pass"
 
 namespace mlir_ts = mlir::typescript;
@@ -1361,6 +1364,7 @@ class OwnedReturnConsumptionPass
         });
 
         mlir::OpBuilder builder(&getContext());
+        std::set<std::tuple<mlir::Operation *, mlir::Operation *, std::string>> placedAfterFinally;
         for (auto *op : discarded)
         {
             if (!allUsesReleasableInOwnBlock(op) || storedIntoUnownedLocalOutside(op))
@@ -1414,7 +1418,7 @@ class OwnedReturnConsumptionPass
                     auto leavesBlock = mlir::isa<mlir_ts::ReturnValOp, mlir_ts::ReturnOp>(nested) ||
                                        (mlir::isa<mlir_ts::BreakOp, mlir_ts::ContinueOp>(nested) &&
                                         jumpLeavesBlock(nested, block));
-                    if (leavesBlock)
+                    if (leavesBlock && !releaseAfterFinally(builder, op, nested, block, placedAfterFinally))
                     {
                         builder.setInsertionPoint(nested);
                         builder.create<mlir_ts::ReleaseOp>(op->getLoc(), op->getResult(0));
@@ -1446,6 +1450,85 @@ class OwnedReturnConsumptionPass
         }
     }
 
+    // A jump out of a `try` body or catch clause runs the `finally` first, and the finally may read
+    // the temporary: `const v = mv(); try { if (c) continue; } finally { s += v.get(); }` gave `v`
+    // back at the `continue`, before the finally read it. Or the finally may leave by a jump of its
+    // own, which releases the temporary in front of it too: `try { continue; } finally { break; }`
+    // gave it back twice. When a jump passes through a finally between it and the temporary's block,
+    // the release goes at the end of the outermost such finally instead - the last one the jump
+    // runs - inside a `ts.If` tagged with the jump, as MLIRGen does for the scopes the jump leaves
+    // (mlirGenScopeExitAfterFinally), so that TryOpLowering keeps it in the copy of the finally that
+    // jump runs and in no other. A finally that leaves on its own never reaches it. False when the
+    // jump passes through no finally.
+    //
+    // Every jump with one key runs one copy, so the release goes in once for each (`placed`).
+    static bool releaseAfterFinally(mlir::OpBuilder &builder, mlir::Operation *op, mlir::Operation *jump,
+                                    mlir::Block *block,
+                                    std::set<std::tuple<mlir::Operation *, mlir::Operation *, std::string>> &placed)
+    {
+        mlir_ts::TryOp lastTry;
+        for (auto *child = jump; child->getBlock() != block; child = child->getParentOp())
+        {
+            auto tryOp = mlir::dyn_cast<mlir_ts::TryOp>(child->getParentOp());
+            if (!tryOp || child->getParentRegion() == &tryOp.getFinally() ||
+                child->getParentRegion() == &tryOp.getCleanup())
+            {
+                continue;
+            }
+
+            auto &finally = tryOp.getFinally();
+            if (finally.empty() || &finally.front().front() == finally.front().getTerminator())
+            {
+                continue;
+            }
+
+            lastTry = tryOp;
+        }
+
+        if (!lastTry)
+        {
+            return false;
+        }
+
+        auto value = op->getResult(0);
+
+        auto isReturn = mlir::isa<mlir_ts::ReturnValOp, mlir_ts::ReturnOp>(jump);
+        std::string key;
+        if (!isReturn)
+        {
+            auto isBreak = mlir::isa<mlir_ts::BreakOp>(jump);
+            auto label = isBreak ? mlir::cast<mlir_ts::BreakOp>(jump).getLabelAttr()
+                                 : mlir::cast<mlir_ts::ContinueOp>(jump).getLabelAttr();
+            key = std::string(isBreak ? JUMP_ONLY_BREAK_KEY : JUMP_ONLY_CONTINUE_KEY) +
+                  (label ? label.getValue().str() : std::string());
+        }
+
+        if (!placed.insert({op, lastTry.getOperation(), isReturn ? std::string(RETURN_ONLY_ATTR_NAME) : key}).second)
+        {
+            return true;
+        }
+
+        auto location = op->getLoc();
+        auto &finallyEnd = lastTry.getFinally().back();
+        builder.setInsertionPoint(finallyEnd.getTerminator());
+        auto alwaysTrue = builder.create<mlir_ts::ConstantOp>(location, mlir_ts::BooleanType::get(builder.getContext()),
+                                                              builder.getBoolAttr(true));
+        auto ifOp = builder.create<mlir_ts::IfOp>(location, alwaysTrue, /*withElseRegion=*/false);
+        if (isReturn)
+        {
+            ifOp->setAttr(RETURN_ONLY_ATTR_NAME, builder.getUnitAttr());
+        }
+        else
+        {
+            ifOp->setAttr(JUMP_ONLY_ATTR_NAME, builder.getStringAttr(key));
+        }
+
+        mlir::OpBuilder::InsertionGuard guard(builder);
+        builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
+        builder.create<mlir_ts::ReleaseOp>(location, value);
+        return true;
+    }
+
     // Does `op` catch this jump - is it the loop, switch or labelled statement the jump names?
     //
     // An unlabelled `break` is caught by the nearest enclosing loop or `switch`; an unlabelled

tslang/test/tester/CMakeLists.txt

@@ -405,6 +405,7 @@ tslang_add_test(NAME test-compile-00-const-element-read COMMAND test-runner "${P
 tslang_add_test(NAME test-compile-00-const-global-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts")
 tslang_add_test(NAME test-compile-00-global-argument-reassigned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts")
 tslang_add_test(NAME test-compile-00-global-argument-view COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_view.ts")
+tslang_add_test(NAME test-compile-00-temporary-read-in-finally COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00temporary_read_in_finally.ts")
 tslang_add_test(NAME test-compile-00-unowned-local-store COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00unowned_local_store.ts")
 tslang_add_test(NAME test-compile-00-array-argument-held COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_argument_held.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")
@@ -986,6 +987,7 @@ tslang_add_test(NAME test-jit-00-const-element-read COMMAND test-runner -jit "${
 tslang_add_test(NAME test-jit-00-const-global-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts")
 tslang_add_test(NAME test-jit-00-global-argument-reassigned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts")
 tslang_add_test(NAME test-jit-00-global-argument-view COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_view.ts")
+tslang_add_test(NAME test-jit-00-temporary-read-in-finally COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00temporary_read_in_finally.ts")
 tslang_add_test(NAME test-jit-00-unowned-local-store COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00unowned_local_store.ts")
 tslang_add_test(NAME test-jit-00-array-argument-held COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_argument_held.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")
@@ -1957,6 +1959,7 @@ set(TSLANG_CORPUS
     00const_global_read.ts
     00global_argument_reassigned.ts
     00global_argument_view.ts
+    00temporary_read_in_finally.ts
     00unowned_local_store.ts
     00array_argument_held.ts
     00array_data_string_alias.ts

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

@@ -0,0 +1,177 @@
+// A `const` holding a call's result (a folded const, released where its block ends) stays alive
+// when a `break`, `continue` or `return` leaves its block through a `try` whose `finally` reads
+// it, or leaves by a jump of its own. Under rc the release went in front of the jump, and the
+// finally then ran on a freed block, or released it a second time in front of its own jump.
+// Each read follows allocations of the same size, so a freed block is reused.
+class V {
+    tag = "";
+    constructor(public x: number) {
+        this.tag = "v" + x;
+    }
+
+    get() {
+        return this.x;
+    }
+}
+
+function mv(x: number) {
+    return new V(x);
+}
+
+function churn() {
+    const junk: V[] = [];
+    for (let i = 0; i < 64; i++) {
+        junk.push(new V(999));
+    }
+}
+
+function viaContinue() {
+    let s: number = 0;
+    for (let i = 0; i < 4; i++) {
+        const v = mv(i);
+        try {
+            churn();
+            if (i % 2 == 0) {
+                continue;
+            }
+        } finally {
+            churn();
+            s += v.get();
+        }
+    }
+
+    return s;
+}
+
+function viaBreak() {
+    let s: number = 0;
+    for (let i = 0; i < 4; i++) {
+        const v = mv(i + 10);
+        try {
+            if (i == 1) {
+                break;
+            }
+        } finally {
+            churn();
+            s += v.x;
+        }
+    }
+
+    return s;
+}
+
+function viaTwoJumps() {
+    let s: number = 0;
+    for (let i = 0; i < 6; i++) {
+        const v = mv(i);
+        try {
+            if (i % 3 == 0) {
+                continue;
+            }
+
+            if (i % 3 == 1) {
+                continue;
+            }
+        } finally {
+            churn();
+            s += v.get();
+        }
+    }
+
+    return s;
+}
+
+function viaLabelledContinue() {
+    let s: number = 0;
+    outer: for (let i = 0; i < 3; i++) {
+        const v = mv(i + 1);
+        for (let j = 0; j < 3; j++) {
+            try {
+                try {
+                    if (j == 1) {
+                        continue outer;
+                    }
+                } finally {
+                    churn();
+                    s += v.get();
+                }
+            } finally {
+                churn();
+                s += v.x * 100;
+            }
+        }
+    }
+
+    return s;
+}
+
+function viaReturn(x: number) {
+    const v = mv(x);
+    try {
+        if (x > 0) {
+            return x;
+        }
+    } finally {
+        churn();
+        assert(v.x == x && v.tag == "v" + x, "read in the finally of a return");
+    }
+
+    return -1;
+}
+
+function jumpInFinally() {
+    let s: number = 0;
+    for (let i = 0; i < 4; i++) {
+        const v = mv(i);
+        try {
+            try {
+                if (i % 2 == 0) {
+                    continue;
+                }
+            } finally {
+                churn();
+                if (i == 2) {
+                    break;
+                }
+            }
+        } finally {
+        }
+
+        s += v.x;
+    }
+
+    churn();
+    return s;
+}
+
+function callInFinally(v: V) {
+    churn();
+    return v.get();
+}
+
+function viaCallInFinally() {
+    let s: number = 0;
+    for (let i = 0; i < 3; i++) {
+        const v = mv(i + 5);
+        try {
+            if (v.x > 2) {
+                continue;
+            }
+        } finally {
+            s += callInFinally(v);
+        }
+    }
+
+    return s;
+}
+
+function main() {
+    assert(viaContinue() == 6, "continue through a finally");
+    assert(viaBreak() == 21, "break through a finally");
+    assert(viaTwoJumps() == 15, "two continues through one finally");
+    assert(viaLabelledContinue() == 1212, "a labelled continue through two finallys");
+    assert(viaReturn(3) == 3, "return through a finally");
+    assert(viaCallInFinally() == 18, "a call in the finally");
+    assert(jumpInFinally() == 1, "a continue through a finally that breaks");
+    print("done.");
+}