rc: a global takes the reference of an owned producer through a widening view (#459)

tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp

@@ -123,7 +123,7 @@ class OwnedReturnConsumptionPass
         // back even when no function here is classified as returning owned.
         if (returnsOwned.empty())
         {
-            claimHandleGlobalInitializers(module);
+            claimGlobalInitializers(mth, module);
             releaseDiscardedTemporaries(mth, module);
             return;
         }
@@ -236,29 +236,31 @@ class OwnedReturnConsumptionPass
 
         consumeConditionalResults(mth, module);
 
-        claimHandleGlobalInitializers(module);
+        claimGlobalInitializers(mth, module);
 
         releaseDiscardedTemporaries(mth, module);
     }
 
-    // A handle global (own spec 23.1) is born holding the reference its initializer carries.
+    // A global is born holding the reference its initializer carries.
     //
     // The initializer region ends in `ts.GlobalResult`, a terminator, so a value given to it
-    // straight is never released as a discarded temporary (§9.30): `let g: Shared<T> = mk()`
-    // keeps the call's +1. A value given to it through a view of the global's type is: `ts.Cast`
-    // to `Shared<T> | null`, `ts.OptionalValue` to `Shared<T> | undefined`, `ts.CreateUnionInstance`
-    // - the view is the one use the producer has, and it is releasable, so the producer's
-    // reference was given back at the end of the region and the global was born at count 0.
+    // straight is never released as a discarded temporary (§9.30): `let g = mk()` keeps the call's
+    // +1. A value given to it through a view of the global's type is: `ts.Cast` to `C | null` or
+    // `Shared<T> | null`, `ts.OptionalValue` to `C | undefined`, `ts.CreateUnionInstance` - the view
+    // is the one use the producer has, and it is releasable, so the producer's reference was given
+    // back at the end of the region and the global was born at count 0, pointing at freed memory
+    // (#459; own spec 23.1 for handles).
     //
-    // The producer is anything that carries an owned reference by now: `new Shared(..)`, marked
-    // where MLIRGen builds it, or a call, marked above once its callee is classified - which is
-    // why MLIRGen alone cannot see it. A handle read out of a field or an element carries none,
-    // and gets a retain of its own. Keyed on the global's type: no other global changes. A read of another
-    // global is left alone (#446: it reads null today in every model).
-    void claimHandleGlobalInitializers(mlir::ModuleOp module)
+    // The producer is anything that carries an owned reference by now: `new Shared(..)` or a fresh
+    // string, marked where MLIRGen builds it, or a call, marked above once its callee is classified
+    // - which is why MLIRGen alone cannot see it. A handle read out of a field or an element
+    // carries none, and gets a retain of its own (handles only: under own a retain left in a
+    // global's region is an error for any other type). A read of another global is left alone
+    // (#446: it reads null today in every model).
+    void claimGlobalInitializers(MLIRTypeHelper &mth, mlir::ModuleOp module)
     {
         module.walk([&](mlir_ts::GlobalOp globalOp) {
-            if (!MLIRTypeHelper::isSharedHandleType(globalOp.getType()))
+            if (!mth.ownsHeapMemory(globalOp.getLoc(), globalOp.getType()))
             {
                 return;
             }

tslang/test/tester/CMakeLists.txt

@@ -472,6 +472,7 @@ tslang_add_test(NAME test-compile-00-instanceof-union COMMAND test-runner "${PRO
 tslang_add_test(NAME test-compile-00-nullable-to-any COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00nullable_to_any.ts")
 tslang_add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
 tslang_add_test(NAME test-compile-00-class-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts")
+tslang_add_test(NAME test-compile-00-global-union-from-call COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_union_from_call.ts")
 tslang_add_test(NAME test-compile-00-capture-in-new-arguments COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts")
 tslang_add_test(NAME test-compile-01-class-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/01class_new.ts")
 tslang_add_test(NAME test-compile-00-class-stack COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_stack.ts")
@@ -956,6 +957,7 @@ tslang_add_test(NAME test-jit-00-instanceof-union COMMAND test-runner -jit "${PR
 tslang_add_test(NAME test-jit-00-nullable-to-any COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00nullable_to_any.ts")
 tslang_add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
 tslang_add_test(NAME test-jit-00-class-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts")
+tslang_add_test(NAME test-jit-00-global-union-from-call COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_union_from_call.ts")
 tslang_add_test(NAME test-jit-00-capture-in-new-arguments COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts")
 tslang_add_test(NAME test-jit-01-class-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/01class_new.ts")
 tslang_add_test(NAME test-jit-00-class-stack COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_stack.ts")
@@ -1766,6 +1768,7 @@ set(TSLANG_CORPUS
     00in_method_names.ts
     00in.ts
     00infer.ts
+    00global_union_from_call.ts
     00instanceof.ts
     00instanceof_namespace_narrowing.ts
     00instanceof_union.ts

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

@@ -0,0 +1,43 @@
+// A module-level `Node | null` (or `Node | undefined`) initialised from a call keeps the object the
+// call made (#459). Under rc the call's reference was released at the end of the initializer, since
+// the global's union cast hid it, and the global pointed at freed memory. Each global is read after
+// allocations of the same size have reused anything freed.
+class Node {
+    v = 0;
+    name = "";
+}
+
+function mk(name: string) {
+    const n = new Node();
+    n.name = name;
+    return n;
+}
+
+function churn() {
+    let keep: Node[] = [];
+    for (let i = 0; i < 1000; i++) {
+        const n = new Node();
+        n.v = -i;
+        n.name = "junk" + i;
+        keep.push(n);
+    }
+
+    return keep.length;
+}
+
+let G: Node | null = mk("made" + 1);
+const C: Node | null = mk("const" + 2);
+let U: Node | undefined = mk("optional" + 3);
+
+function main() {
+    assert(churn() == 1000);
+    assert(G !== null && G.name == "made1", "a union global from a call keeps its object");
+    assert(C !== null && C.name == "const2", "a union const from a call keeps its object");
+    assert(U !== undefined && U.name == "optional3", "an optional global from a call keeps its object");
+
+    G = mk("next" + 5);
+    assert(churn() == 1000);
+    assert(G !== null && G.name == "next5", "a reassigned union global keeps the new object");
+
+    print("done.");
+}