own: a value pushed where not null; literal unions as views; moves on every path; constant literal globals (TSLANG 67-70); string literal union type check

TSLANG_ISSUES.md

@@ -479,6 +479,54 @@ ports 25 to 6), and no source has more. Regression tests `tslang/test/tester/tes
 (every model; loops, a class-typed `let`, a throw), `own/own_err_split_let_stored.ts` and
 `own/own_err_split_let_field_overwritten.ts`.
 
+67 (a value pushed only where it is not null, then used: `const view = constantBuffer.createConstantView(offset,
+size); if (view) this.constantViews.push(view); return view;` in `examples/nri/rt_shadows_nri.ts` and its
+NRI ports, refused "is used here after its value was moved") is fixed. TSLANG 52 made each use the push
+dominates a read of the array's last element; one after the paths meet again is now one too. The block the
+test's false edge goes to, entered otherwise only from blocks the push dominates, takes the element read
+back on those paths and null on the false edge, and the uses there read it. A function returning it returns
+a borrow of `this`, as one returning the element read does. The array popped before the use is still
+refused, and so is a push under a test that is not the value's own truth (`if (view && keep)`), where the
+value is not null on the other path. A `let` pushed this way (`let pipeline = null; if (loaded) pipeline =
+create(); if (pipeline) this.pipelines.push(pipeline); return pipeline;` in `msaa_nri.ts`) is not handled
+yet. In the sweep, 6 errors less; their sources fail for others too. Regression tests
+`tslang/test/tester/tests/00tslang_issue_67.ts` (every model; it holds 68 too) and
+`own/own_err_pushed_or_null_popped.ts`.
+
+68 (a union of string literals read as a `string`: `ImGui.text(`primitiveUnderestimation: ${this.yesNo(
+p[3])}`)` in `examples/conservative_rasterization.ts`, with `yesNo` returning `value != 0.0 ? "yes" : "no"`,
+refused "takes a second reference") is fixed. `"no" | "yes"` is one pointer, like a `string`, and the cast
+between them lowers to nothing, but -mm=own took it for a conversion making a new string, so the call
+result it was cast from was released while the string was used. A cast between a string, a string literal
+and a union of them (and null) that needs no tag is now a view. In the sweep, 2 sources more compile
+(`conservative_rasterization.ts`, `multi_draw_indirect.ts`) and 20 errors less. A union of string literals
+also allows only its literals now, under every model: `let a: "no" | "yes" = "maybe"`, a `string` given
+to it, or `a = "maybe"` after `let a = yesNo(v)`, compiled and held "maybe"; each is now refused, as in
+TypeScript (TS2322, and TS2345 for an argument). The check found a parameter property typed by its
+default left unwidened: `constructor(private source: string, private flags = "")` in the default library's
+`RegExp` made a field of type `""`; it is a `string` now, as a declared field and a parameter already were.
+Regression tests
+`tslang/test/tester/tests/assign_string_literal_union_error.ts`, `assign_string_to_literal_union_error.ts`
+and `argument_string_literal_union_error.ts`.
+
+69 (a value moved on every path, by a different move on each: `const node = new SceneNode(...); if
+(SCENE_ORDER[o] == TESSELLATED) this.tessNodes.push(node); else { ... this.baselineNodes.push(node); }` in
+`examples/nri/extended_dynamic_state2_nri.ts`, refused "is moved here on some paths only; -mm=own cannot
+release it on the others yet") is fixed. Each move was judged alone: the value's release where the paths meet
+was past this move on some paths, and on the others past a move it did not look at. A release no path reaches
+from the value's making without passing one of its moves is past a move on each, and goes. A path that moves
+it nowhere (`if ... else if ...` with no `else`) is still refused, and so is a use after the moves. In the
+sweep, 4 errors less. Regression tests `tslang/test/tester/tests/00tslang_issue_69.ts` (every model; it holds
+70 too) and `own/own_err_moved_on_two_of_three_paths.ts`.
+
+70 (a constant global of a string literal's type given to a call that keeps it: `this.loadTexture(device,
+FLAME_TEXTURE_PATH)` in `examples/nri/ray_tracing_extended_nri.ts`, with `const FLAME_TEXTURE_PATH = "..."` in
+its namespace and `loadTexture` pushing the path into `textureNames`, refused "is given to ..., which keeps
+it, but -mm=own cannot move it here: this function does not own it") is fixed. The global is a constant whose
+type is the literal, so it holds that literal and nothing else, in read-only data: given to a keeper, it is the
+literal itself, as `"..."` written in place is. 69 exposed it: `loadTexture` is now found to keep its path. In the sweep, the 2 errors it gave
+(in `ray_tracing_extended_nri.ts` and `ray_tracing_invocation_reorder_nri.ts`) are gone.
+
 For native_ts_graphics, from the same sweep (its code, not the compiler):
 - `core/ray_tracing_extended_scene.ts`: the exported classes' constructors keep their array parameters
   (`this.origin = origin`), which an exported constructor has to say: `@keeps("origin", "direction")`
@@ -489,7 +537,7 @@ For native_ts_graphics, from the same sweep (its code, not the compiler):
   takes it; the next release won't. `desc.setDepthClip(clip != 0)` fixes it.
 - Under `-mm=own`, 3 `core/` modules do not compile yet (`nri_donut_textures.ts`,
   `ray_tracing_extended_scene.ts`, `rtxpt_game.ts`), and nor do many examples,
-  which #310 did not port: with every facts object built and 50-66 in, 118 of the 276 sources fail (664 errors). The
+  which #310 did not port: with every facts object built and 50-70 in, 116 of the 276 sources fail (635 errors). The
   largest group left is "borrows a field but is used here after it may be released or overwritten" (396,
-  93 of them `this.sample.run` callbacks), then "used here after its value was moved" (58: mostly a value a pushed object or an
+  93 of them `this.sample.run` callbacks), then "used here after its value was moved" (52: mostly a value a pushed object or an
   earlier call keeps, then returned or used).

tslang/lib/TypeScript/MLIRGenClasses.cpp

@@ -2686,6 +2686,13 @@ namespace mlirgen
                 return mlir::failure();
             }
 
+            // `constructor(private flags = "")` is a field of the parameter's type: the default's,
+            // widened - a `string`, not the literal `""` (no other value could be given to it)
+            if (!typeProvided)
+            {
+                type = mth.wideStorageType(type);
+            }
+
             // `constructor(public m?: T)` is a field `m?: T`: T | undefined, as a declared one
             // (mlirGenClassDataFieldMember) - `new Error().message` is undefined
             if (parameter->questionToken)

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -3278,8 +3278,84 @@ class MLIRGenImpl
         auto to = stringOrNumber(destType);
         // a partial resolve may stand in a call's first overload for the one it will pick (an @cpp
         // class's `write(s32)` for `write(string)`): the full pass checks the one picked
-        return from != StringOrNumber::Neither && to != StringOrNumber::Neither && from != to
-               && !untypedParametersInScope(genContext) && !genContext.allowPartialResolve;
+        if (untypedParametersInScope(genContext) || genContext.allowPartialResolve)
+        {
+            return false;
+        }
+
+        return (from != StringOrNumber::Neither && to != StringOrNumber::Neither && from != to)
+               || isStringLiteralMismatch(valueType, destType);
+    }
+
+    // The string literals a type allows when it allows only string literals (`"no" | "yes"`, with
+    // null or undefined beside them), or none.
+    static bool stringLiteralsOf(mlir::Type type, llvm::SmallVectorImpl<mlir::Type> &literals)
+    {
+        auto isStringLiteral = [](mlir::Type member) {
+            auto literalType = dyn_cast<mlir_ts::LiteralType>(member);
+            return literalType && isa<mlir_ts::StringType>(literalType.getElementType());
+        };
+
+        if (auto optionalType = dyn_cast<mlir_ts::OptionalType>(type))
+        {
+            type = optionalType.getElementType();
+        }
+
+        if (isStringLiteral(type))
+        {
+            literals.push_back(type);
+            return true;
+        }
+
+        auto unionType = dyn_cast<mlir_ts::UnionType>(type);
+        if (!unionType)
+        {
+            return false;
+        }
+
+        for (auto member : unionType.getTypes())
+        {
+            if (isStringLiteral(member))
+            {
+                literals.push_back(member);
+            }
+            else if (!isa<mlir_ts::NullType, mlir_ts::UndefinedType>(member))
+            {
+                return false;
+            }
+        }
+
+        return !literals.empty();
+    }
+
+    // `let answer: "no" | "yes" = "maybe"`, or a `string` given to it: TypeScript rejects them (TS2322),
+    // the values it allows being those literals only. A literal, or a union of literals, it allows
+    // each of passes.
+    bool isStringLiteralMismatch(mlir::Type valueType, mlir::Type destType)
+    {
+        llvm::SmallVector<mlir::Type> allowed;
+        if (!stringLiteralsOf(destType, allowed))
+        {
+            return false;
+        }
+
+        if (auto optionalType = dyn_cast<mlir_ts::OptionalType>(valueType))
+        {
+            valueType = optionalType.getElementType();
+        }
+
+        if (isa<mlir_ts::StringType>(valueType))
+        {
+            return true;
+        }
+
+        llvm::SmallVector<mlir::Type> given;
+        if (!stringLiteralsOf(valueType, given))
+        {
+            return false;
+        }
+
+        return llvm::any_of(given, [&](mlir::Type literal) { return !llvm::is_contained(allowed, literal); });
     }
 
     // `x = "1"` with `x: number`, `let s: string = 5`: TypeScript rejects them (TS2322). Converted,

tslang/lib/TypeScript/OwnershipFacts.h

@@ -366,6 +366,8 @@ inline llvm::SmallVector<mlir::Value> valuesStoredInto(mlir_ts::VariableOp varOp
 // interned, nobody's block). rc's retain or release of
 // either is one of the block, so the pass looks through them. A cast that converts (a number
 // printed into a string) or boxes (into `any`) makes a new block and is not one.
+inline bool isCopyableString(mlir::Type type);
+
 inline bool isView(mlir::Operation *op)
 {
     // (`ts.ValueOrDefault` of a class: the optional's object, or null - the same block or none, spec 77)
@@ -407,6 +409,13 @@ inline bool isView(mlir::Operation *op)
     auto out = castOp.getType();
     auto isNullableView = (isStringOrArray(in) && holdsAsIs(out, in, false)) ||
                           (isStringOrArray(out) && holdsAsIs(in, out, true));
+    // ...or text seen as other text, each one pointer: a string, or a union of strings and string
+    // literals (and null) that needs no tag - `"yes" | "no"` read as a `string` (TSLANG 68). An optional
+    // is `{ptr, i1}`, not one of them; a cast of a literal is a constant's read, which stays one.
+    auto isOnePointerText = [](mlir::Type type) {
+        return mlir::isa<mlir_ts::StringType, mlir_ts::UnionType>(type) && isCopyableString(type);
+    };
+    isNullableView = isNullableView || (isOnePointerText(in) && isOnePointerText(out));
     // a `@value` value seen as the tuple it is laid out as, or a tuple seen as the value: the same
     // tuple (the cast lowers to nothing)
     auto isValueStorage = [](mlir::Type value, mlir::Type tuple) {

tslang/lib/TypeScript/OwnershipInferenceMoves.cpp

@@ -381,8 +381,28 @@ namespace own_inference
         }
 
         auto castOp = rootOf(value).getDefiningOp<mlir_ts::CastOp>();
-        return castOp && mlir::isa<mlir_ts::StringType>(castOp.getType()) &&
-               castOp.getIn().getDefiningOp<mlir_ts::ConstantOp>();
+        if (!castOp || !mlir::isa<mlir_ts::StringType>(castOp.getType()))
+        {
+            return false;
+        }
+
+        if (castOp.getIn().getDefiningOp<mlir_ts::ConstantOp>())
+        {
+            return true;
+        }
+
+        // ...or a read of a constant global of a string literal's type (`const PATH = "flame.dds"`,
+        // TSLANG 70): it holds that literal, in read-only data, and nothing else
+        auto loadOp = castOp.getIn().getDefiningOp<mlir_ts::LoadOp>();
+        auto addressOfOp = loadOp ? loadOp.getReference().getDefiningOp<mlir_ts::AddressOfOp>() : mlir_ts::AddressOfOp();
+        auto literalType = mlir::dyn_cast<mlir_ts::LiteralType>(castOp.getIn().getType());
+        if (!addressOfOp || !literalType || !mlir::isa<mlir_ts::StringType>(literalType.getElementType()))
+        {
+            return false;
+        }
+
+        auto globalOp = mlir::SymbolTable::lookupNearestSymbolFrom<mlir_ts::GlobalOp>(addressOfOp, addressOfOp.getGlobalNameAttr());
+        return globalOp && globalOp.getConstant();
     }
 
     bool OwnershipInferencePass::isBorrow(mlir::Operation *user, mlir::Value value)
@@ -1016,9 +1036,12 @@ namespace own_inference
                     continue;
                 }
 
-                // moved on some ways only: on the others it must be empty (TSLANG 49)
+                // moved on some ways only: on the others it must be empty (TSLANG 49) - or moved by
+                // another taker: `if (c) this.a.push(node); else this.b.push(node);` reaches the
+                // release only past one move or the other (TSLANG 69)
                 if (!dominance->properlyDominates(acquire, release) &&
-                    !emptyWhereNotMoved(value, definition, acquire, release))
+                    !emptyWhereNotMoved(value, definition, acquire, release) &&
+                    !(definition && takers.size() > 1 && !reachableAfter(definition, release, takers)))
                 {
                     reportMovedOnSomePaths(taker, release, name);
                     return false;

tslang/lib/TypeScript/OwnershipSignatureFieldLocals.cpp

@@ -134,6 +134,99 @@ namespace own_signature
                               loadOp.getReference().getDefiningOp<mlir_ts::PropertyRefOp>());
         }
 
+        // The truthiness test of `value` a branch makes: `cond_br(DialectCast(Cast(value -> boolean)))`.
+        static bool testsTruthOf(mlir::cf::CondBranchOp branch, mlir::Value value)
+        {
+            auto dialectCast = branch.getCondition().getDefiningOp<mlir_ts::DialectCastOp>();
+            auto cast = dialectCast ? dialectCast->getOperand(0).getDefiningOp<mlir_ts::CastOp>() : mlir_ts::CastOp();
+            return cast && mlir::isa<mlir_ts::BooleanType>(cast.getType()) && cast->getOperand(0) == value;
+        }
+
+        // `if (value) array.push(value);` and uses of `value` after it (TSLANG 67): the block the test's
+        // false edge goes to, reached otherwise only through the push, gets an argument - the element
+        // read back after the push (`read`) on those paths, null on the false edge - and the uses of
+        // `value` that block dominates read it.
+        void mergePushedOrNull(mlir::Operation *pushOp, mlir::Value value, mlir::Value read, mlir::DominanceInfo &dominance)
+        {
+            auto *pushBlock = pushOp->getBlock();
+            mlir::cf::CondBranchOp test;
+            for (auto *user : value.getUsers())
+            {
+                auto cast = mlir::dyn_cast<mlir_ts::CastOp>(user);
+                if (!cast)
+                {
+                    continue;
+                }
+
+                for (auto *castUser : cast->getUsers())
+                {
+                    for (auto *dialectUser : castUser->getUsers())
+                    {
+                        auto branch = mlir::dyn_cast<mlir::cf::CondBranchOp>(dialectUser);
+                        if (branch && testsTruthOf(branch, value) && dominance.dominates(branch.getTrueDest(), pushBlock))
+                        {
+                            test = branch;
+                        }
+                    }
+                }
+            }
+
+            if (!test || test.getFalseDest() == test.getTrueDest())
+            {
+                return;
+            }
+
+            auto *merge = test.getFalseDest();
+            for (auto *pred : merge->getPredecessors())
+            {
+                if (pred == test->getBlock())
+                {
+                    continue;
+                }
+
+                // every other way in has pushed it, and goes in by a plain branch
+                if (!mlir::isa<mlir::cf::BranchOp>(pred->getTerminator()) || !dominance.dominates(pushOp, pred->getTerminator()))
+                {
+                    return;
+                }
+            }
+
+            llvm::SmallVector<mlir::OpOperand *> uses;
+            for (auto &use : value.getUses())
+            {
+                auto *user = use.getOwner();
+                if (!mlir::isa<mlir_ts::ReleaseOp>(user) && user->getBlock() != test->getBlock() &&
+                    dominance.dominates(merge, user->getBlock()))
+                {
+                    uses.push_back(&use);
+                }
+            }
+
+            if (uses.empty())
+            {
+                return;
+            }
+
+            auto location = pushOp->getLoc();
+            auto argument = merge->addArgument(value.getType(), location);
+            mlir::OpBuilder builder(test);
+            mlir::Value null = builder.create<mlir_ts::NullOp>(location, mlir_ts::NullType::get(builder.getContext()));
+            null = builder.create<mlir_ts::CastOp>(location, value.getType(), null);
+            test.getFalseDestOperandsMutable().append(null);
+            for (auto *pred : merge->getPredecessors())
+            {
+                if (auto branch = mlir::dyn_cast<mlir::cf::BranchOp>(pred->getTerminator()))
+                {
+                    branch.getDestOperandsMutable().append(read);
+                }
+            }
+
+            for (auto *use : uses)
+            {
+                use->set(argument);
+            }
+        }
+
         // `value` seen through the views that keep its block: an optional made of it or opened, a
         // widening or narrowing cast that is a view.
         mlir::Value stripViews(mlir::Value value)
@@ -848,6 +941,11 @@ namespace own_signature
                 }
 
                 movedTo[root].push_back({pushOp.getOperation(), read});
+
+                // ...and pushed only where it is not null (TSLANG 67): `const view = make(); if (view)
+                // this.views.push(view); return view;`. Where the paths meet again it is the element
+                // read back, or the null it was: a merge of the two, which each use there reads.
+                mergePushedOrNull(pushOp, root, read, dominance);
             }
 
             followFieldStores();