A using in a switch case is disposed where the switch ends; break in an if in a case compiles

tslang/include/TypeScript/MLIRLogic/MLIRGenContext.h

@@ -230,6 +230,10 @@ struct GenContext
     // at the loop's own scope - and `isLoop` alone reads true for all of them, which is what
     // made a `break` written inside an `if` skip the lot.
     bool isLoopBodyScope = false;
+    // Set on the scope of a `switch` - the one its cases share - and cleared, as isLoopBodyScope
+    // is, for every scope nested in it. An unlabelled `break` walking outwards stops here; a
+    // `continue` passes through it to the loop.
+    bool isSwitchScope = false;
     // out-of-band cancellation signal; mutable so stop() stays callable through the const& threading
     mutable bool stopProcess = false;
     mlir::SmallVector<std::unique_ptr<mlir::Diagnostic>> *postponedMessages = nullptr;

tslang/lib/TypeScript/LowerToAffineLoops.cpp

@@ -850,6 +850,18 @@ struct SwitchOpLowering : public TsPattern<mlir_ts::SwitchOp>
 
         auto *casesRegion = &switchOp.getCasesRegion().front();
 
+        // An unlabelled `break` at the end of a case is already a branch to the merge block
+        // (mlirGenSwitchCase); one written further in - in an `if`, say - is a BreakOp, and had
+        // no target: lowering it crashed. It goes past the merge block, whose scope exit the
+        // `break` has already done on its way out. A labelled one belongs to its LabelOp.
+        visitBreakContinueInScope(switchOp.getCasesRegion(), mlir::StringAttr(), /*eligibleForUnlabeledBreak*/ true,
+                                   /*eligibleForUnlabeledContinue*/ false, [&](Operation *op, bool isBreak) {
+                                       if (isBreak)
+                                       {
+                                           tsContext->jumps[op] = continuation;
+                                       }
+                                   });
+
         auto *casesRegionWithMerge = &switchOp.getCasesRegion().back();
         for (auto &block : switchOp.getCasesRegion())
         {

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -124,7 +124,10 @@ enum class DisposeDepth
 {
     CurrentScope,
     CurrentScopeKeepAfterUse,
+    // a `continue`: up to the body of the loop it targets
     LoopScope,
+    // a `break`: as LoopScope, except that an unlabelled one stops at a `switch` too
+    BreakScope,
     FullStack
 };
 
@@ -469,9 +472,15 @@ class MLIRGenImpl
     // know a `using` was declared, the block's ops are already emitted at the current
     // insertion point and there is no clean way to wrap them retroactively.
     bool blockDeclaresUsing(ts::Block blockAST, int skipStatements = 0)
+    {
+        return blockDeclaresUsing(blockAST->statements, skipStatements);
+    }
+
+    // the same over a list of statements: a `switch`'s case clauses have no block of their own
+    bool blockDeclaresUsing(const NodeArray<Statement> &statements, int skipStatements = 0)
     {
         auto index = 0;
-        for (auto statement : blockAST->statements)
+        for (auto statement : statements)
         {
             if (index++ < skipStatements)
             {
@@ -504,9 +513,14 @@ class MLIRGenImpl
     // blockDeclaresUsing for why the check has to run before that. `new X(...)` is the
     // syntactic proxy, and it costs nothing to check.
     bool blockUsingInitializersAreAllNewExpr(ts::Block blockAST, int skipStatements = 0)
+    {
+        return blockUsingInitializersAreAllNewExpr(blockAST->statements, skipStatements);
+    }
+
+    bool blockUsingInitializersAreAllNewExpr(const NodeArray<Statement> &statements, int skipStatements = 0)
     {
         auto index = 0;
-        for (auto statement : blockAST->statements)
+        for (auto statement : statements)
         {
             if (index++ < skipStatements)
             {
@@ -558,6 +572,11 @@ class MLIRGenImpl
     // A block with nothing to release costs nothing: mlirGenScopeExit writes no operations into
     // the cleanup region, TryOpLowering sees an empty one, and the wrapping leaves no trace.
     bool blockHasTypedCatch(ts::Block blockAST, int skipStatements = 0)
+    {
+        return blockHasTypedCatch(blockAST->statements, skipStatements);
+    }
+
+    bool blockHasTypedCatch(const NodeArray<Statement> &statements, int skipStatements = 0)
     {
         auto found = false;
         ts::FilterVisitorSkipFuncsAST<CatchClause> visitor(SyntaxKind::CatchClause, [&](CatchClause catchClauseNode) {
@@ -568,7 +587,7 @@ class MLIRGenImpl
         });
 
         auto index = 0;
-        for (auto statement : blockAST->statements)
+        for (auto statement : statements)
         {
             if (index++ < skipStatements)
             {
@@ -726,7 +745,8 @@ class MLIRGenImpl
 
     // Whether a scope exit that has just finished with this scope still owes the scopes outside
     // it. `FullStack` - a `return` - owes all of them. A `break` or `continue` owes every scope
-    // up to and including the body scope of the loop it targets.
+    // up to and including the body scope of the loop it targets - or, for an unlabelled
+    // `break` in a case, the scope of the switch.
     //
     // The loop test belongs here, and on `isLoopBodyScope`. Written as
     // `disposeDepth == LoopScope && genContext->isLoop && genContext->loopLabel != loopLabel`
@@ -743,7 +763,7 @@ class MLIRGenImpl
             return true;
         }
 
-        if (disposeDepth != DisposeDepth::LoopScope)
+        if (disposeDepth != DisposeDepth::LoopScope && disposeDepth != DisposeDepth::BreakScope)
         {
             return false;
         }
@@ -758,9 +778,15 @@ class MLIRGenImpl
         // `continue outer` written in an `if` in an inner loop skipped the outer body's `using`.
         // A jump to a labelled block matched nothing and ran on to the function's scope,
         // disposing its `using` and releasing its locals there and then.
+        //
+        // An unlabelled `break` belongs to a `switch` as much as to a loop, and stops at the
+        // nearer of the two; a `continue` passes through a switch. Without the switch's scope the
+        // walk took a `break` out of a case on to the scopes around the switch - releasing their
+        // locals - and found nothing of the case's own.
         if (loopLabel.empty())
         {
-            return !genContext->isLoopBodyScope;
+            return !genContext->isLoopBodyScope &&
+                   !(disposeDepth == DisposeDepth::BreakScope && genContext->isSwitchScope);
         }
 
         auto isTarget = (genContext->isLoopBodyScope && genContext->loopBodyLabel == loopLabel) ||
@@ -4571,10 +4597,12 @@ class MLIRGenImpl
         auto statements = caseBlock->statements;
         // inline block
         // TODO: should I inline block as it is isolator of local vars?
+        // Not one that declares a `using`: that is disposed where the block ends, before a case
+        // falling through runs the next one, so the block keeps its scope (mlirGen(Block)).
         if (statements.size() == 1)
         {
             auto firstStatement = statements.front();
-            if ((SyntaxKind)firstStatement == SyntaxKind::Block)
+            if ((SyntaxKind)firstStatement == SyntaxKind::Block && !blockDeclaresUsing(firstStatement.as<ts::Block>()))
             {
                 statements = statements.front().as<ts::Block>()->statements;
             }

tslang/lib/TypeScript/MLIRGenStatements.cpp

@@ -187,6 +187,8 @@ namespace mlirgen
         genContextUsing.loopBodyLabel = genContext.loopLabel;
         // a scope of its own, not the labelled statement's: see labelledContext in mlirGen(LabeledStatement)
         genContextUsing.labelledScopeLabel.clear();
+        // nor the switch's, when this block is a case's
+        genContextUsing.isSwitchScope = false;
         genContextUsing.isLoop = false;
 
         DITableScopeT debugBlockScope(debugScope);
@@ -261,6 +263,7 @@ namespace mlirgen
             tryBodyGenContext.isLoopBodyScope = tryGenContext.isLoop;
             tryBodyGenContext.loopBodyLabel = tryGenContext.loopLabel;
             tryBodyGenContext.labelledScopeLabel.clear();
+            tryBodyGenContext.isSwitchScope = false;
             tryBodyGenContext.isLoop = false;
 
             auto usingVars = std::make_unique<SmallVector<ts::VariableDeclarationDOM::TypePtr>>();
@@ -686,6 +689,7 @@ namespace mlirgen
         branchContext.loopBodyLabel = genContext.loopLabel;
         branchContext.isLoop = false;
         branchContext.labelledScopeLabel.clear();
+        branchContext.isSwitchScope = false;
 
         auto usingVars = std::make_unique<SmallVector<ts::VariableDeclarationDOM::TypePtr>>();
         branchContext.usingVars = usingVars.get();
@@ -1071,6 +1075,7 @@ namespace mlirgen
         labelledContext.parentBlockContext = &genContext;
         labelledContext.labelledScopeLabel = label;
         labelledContext.isLoopBodyScope = false;
+        labelledContext.isSwitchScope = false;
 
         auto usingVars = std::make_unique<SmallVector<ts::VariableDeclarationDOM::TypePtr>>();
         labelledContext.usingVars = usingVars.get();
@@ -1129,7 +1134,7 @@ namespace mlirgen
         }
         label = resolveLoopLabelAlias(label);
 
-        EXIT_IF_FAILED(mlirGenScopeExit(location, DisposeDepth::LoopScope, label, &genContext));
+        EXIT_IF_FAILED(mlirGenScopeExit(location, DisposeDepth::BreakScope, label, &genContext));
 
         builder.create<mlir_ts::BreakOp>(location, builder.getStringAttr(label));
         return mlir::success();
@@ -1146,19 +1151,66 @@ namespace mlirgen
         EXIT_IF_FAILED_OR_NO_VALUE(result)
         auto switchValue = V(result);
 
+        auto &clauses = switchStatementAST->caseBlock->clauses;
+
+        // The cases' scope needs an unwind cleanup on the same terms a block does (mlirGen(Block)):
+        // without one, an exception after a case's `using` was stored reached the cleanup of an
+        // enclosing scope, which knows nothing of the switch's locals.
+        auto declaresUsing = false;
+        auto usingIsDisposable = true;
+        auto hasTypedCatch = false;
+        for (auto &clause : clauses)
+        {
+            declaresUsing |= blockDeclaresUsing(clause->statements);
+            usingIsDisposable &= blockUsingInitializersAreAllNewExpr(clause->statements);
+            hasTypedCatch |= !compileOptions.isWindows && blockHasTypedCatch(clause->statements);
+        }
+
+        mlir_ts::TryOp tryOp;
+        if (usingIsDisposable && (declaresUsing || hasTypedCatch) && !blockIsInsideCatchOrFinally())
+        {
+            mlir_ts::FuncOp funcOp = genContext.funcOp;
+            funcOp.setPersonalityAttr(builder.getBoolAttr(true));
+
+            tryOp = builder.create<mlir_ts::TryOp>(location);
+
+            SmallVector<mlir::Type, 0> types;
+            builder.createBlock(&tryOp.getBody(), {}, types);
+            builder.createBlock(&tryOp.getCleanup(), {}, types);
+            builder.createBlock(&tryOp.getCatches(), {}, types);
+            builder.createBlock(&tryOp.getFinally(), {}, types);
+
+            builder.setInsertionPointToStart(&tryOp.getBody().front());
+        }
+
         auto switchOp = builder.create<mlir_ts::SwitchOp>(location, switchValue);
 
         GenContext switchGenContext(genContext);
         switchGenContext.allocateVarsOutsideOfOperation = true;
-        switchGenContext.currentOperation = switchOp;
+        // in front of the TryOp when there is one, so that its cleanup region sees the storage
+        switchGenContext.currentOperation = tryOp ? tryOp.getOperation() : switchOp.getOperation();
         switchGenContext.insertIntoParentScope = true;
 
+        // The scope the cases share, made as mlirGen(Block) makes one. The cases used to be generated
+        // with a copy of the enclosing context, sharing its lists: a `using` in a case was disposed
+        // at the end of the enclosing scope, by a name the switch's symbol table scope no longer
+        // had, which crashed the compiler.
+        switchGenContext.parentBlockContext = &genContext;
+        switchGenContext.isSwitchScope = true;
+        switchGenContext.isLoopBodyScope = false;
+        switchGenContext.labelledScopeLabel.clear();
+        switchGenContext.isLoop = false;
+
+        auto usingVars = std::make_unique<SmallVector<ts::VariableDeclarationDOM::TypePtr>>();
+        switchGenContext.usingVars = usingVars.get();
+
+        auto ownedVars = std::make_unique<SmallVector<mlir::Value>>();
+        switchGenContext.ownedVars = ownedVars.get();
+
         // add merge block
         switchOp.addMergeBlock();
         auto *mergeBlock = switchOp.getMergeBlock();
 
-        auto &clauses = switchStatementAST->caseBlock->clauses;
-
         SmallVector<mlir::cf::CondBranchOp> pendingConditions;
         SmallVector<mlir::cf::BranchOp> pendingBranches;
         mlir::Operation *previousConditionOrFirstBranchOp = nullptr;
@@ -1174,14 +1226,18 @@ namespace mlirgen
             auto name = propertyAccessExpressionOp->name;
 
             safeCastLogic = [=, &switchGenContext](Expression caseExpr, mlir::Value constVal) {
+                // The narrowed binding is the case's own. This used to clear the flag on
+                // switchGenContext itself, so every later case declared its variables in its own
+                // symbol table scope too - gone by the time the switch's scope exit looks a
+                // `using` up by name.
                 GenContext safeCastGenContext(switchGenContext);
-                switchGenContext.insertIntoParentScope = false;
+                safeCastGenContext.insertIntoParentScope = false;
 
                 // Safe Cast
-                if (mlir::failed(checkSafeCastTypeOf(switchExpr, caseExpr, false, nullptr, switchGenContext)))
+                if (mlir::failed(checkSafeCastTypeOf(switchExpr, caseExpr, false, nullptr, safeCastGenContext)))
                 {
                     checkSafeCastPropertyAccessLogic(caseExpr, objAccessExpression, typeOfObject, name, constVal,
-                                                     false, nullptr, switchGenContext);
+                                                     false, nullptr, safeCastGenContext);
                 }
             };
         }
@@ -1201,6 +1257,41 @@ namespace mlirgen
             }
         }
 
+        // Every way out of a case but a jump ends in the merge block - a `break` at the end of a
+        // case, the last case falling off its end, and no case matching. A `break` written
+        // further in, a `continue` or a `return` has done its own share of this on the way out
+        // (scopeExitContinuesOutwards) and does not come here. A `using` in a case the path did
+        // not run is skipped: its storage is hoisted in front of the switch, and so is a guard
+        // that only the initializer's store arms (createLocalVariable). An owned local there is
+        // released as null, which is nothing to do (VariableOpLowering).
+        {
+            mlir::OpBuilder::InsertionGuard guard(builder);
+            builder.setInsertionPoint(mergeBlock->getTerminator());
+            EXIT_IF_FAILED(mlirGenScopeExit(location,
+                                            tryOp ? DisposeDepth::CurrentScopeKeepAfterUse : DisposeDepth::CurrentScope,
+                                            {}, &switchGenContext));
+        }
+
+        if (tryOp)
+        {
+            // as in mlirGenBlockWithUnwindCleanup: the cleanup does what the merge block does,
+            // reached only from the unwind edge; no catch clause, no finally
+            builder.setInsertionPointAfter(switchOp);
+            builder.create<mlir_ts::ResultOp>(location);
+
+            builder.setInsertionPointToStart(&tryOp.getCleanup().front());
+            EXIT_IF_FAILED(mlirGenScopeExit(location, DisposeDepth::CurrentScope, {}, &switchGenContext));
+            builder.create<mlir_ts::ResultOp>(location);
+
+            builder.setInsertionPointToStart(&tryOp.getCatches().front());
+            builder.create<mlir_ts::ResultOp>(location);
+
+            builder.setInsertionPointToStart(&tryOp.getFinally().front());
+            builder.create<mlir_ts::ResultOp>(location);
+
+            builder.setInsertionPointAfter(tryOp);
+        }
+
         LLVM_DEBUG(llvm::dbgs() << "\n!! SWITCH: " << switchOp << "\n");
 
         return mlir::success();
@@ -1372,6 +1463,7 @@ namespace mlirgen
             tryBodyGenContext.isLoopBodyScope = tryGenContext.isLoop;
             tryBodyGenContext.loopBodyLabel = tryGenContext.loopLabel;
             tryBodyGenContext.labelledScopeLabel.clear();
+            tryBodyGenContext.isSwitchScope = false;
             tryBodyGenContext.isLoop = false;
 
             auto usingVars = std::make_unique<SmallVector<ts::VariableDeclarationDOM::TypePtr>>();

tslang/lib/TypeScript/OwnershipVerifierPass.cpp

@@ -66,8 +66,27 @@ class OwnershipVerifierPass : public mlir::PassWrapper<OwnershipVerifierPass, Ty
             {
                 if (varOp->hasAttr(OWNED_LOCAL_CONSUMED_ATTR_NAME))
                 {
-                    acquisitions.emplace_back(varOp.getResult(), varOp.getOperation());
                     acquiredSlots.insert(varOp.getResult());
+                    if (varOp.getInitializer())
+                    {
+                        acquisitions.emplace_back(varOp.getResult(), varOp.getOperation());
+                    }
+                    else
+                    {
+                        // Storage hoisted out in front of a TryOp or a SwitchOp: the declaration
+                        // is the store that stayed behind, and the slot is null until it runs
+                        // (VariableOpLowering) - a release has nothing to give back on a path
+                        // that never reaches it. Blaming the hoisted slot itself reported a
+                        // `break` written ahead of the declaration, in a case or a try body.
+                        for (auto *user : varOp.getResult().getUsers())
+                        {
+                            auto storeOp = mlir::dyn_cast<mlir_ts::StoreOp>(user);
+                            if (storeOp && storeOp.getReference() == varOp.getResult())
+                            {
+                                acquisitions.emplace_back(varOp.getResult(), storeOp.getOperation());
+                            }
+                        }
+                    }
                 }
             }
         });