Fix new correctness bugs found in a fresh lowering-passes audit

A follow-up static audit of LowerToAffineLoops.cpp, LowerToLLVM.cpp, and
the TypeScriptExceptionPass files (beyond the 10 findings already closed
in PRs #230/#232/#233/#234/#235) found and fixes 7 more confirmed bugs:

- SwitchStateOpLowering: generator yield/resume state labels were
  collected into a SmallPtrSet<Operation*,16>, whose iteration order is
  only insertion order below 16 entries; past that it silently falls back
  to pointer-hash order, scrambling the positional switch dispatch used
  by generator resume. Replaced with an order-preserving SmallVector.
- StringConcatOpLowering: the stack-allocation path sized its Alloca
  using a pointer type instead of i8, over-allocating by sizeof(ptr) for
  every multi-argument string concatenation (e.g. console.log with 2+
  args).
- ArraySpliceOpLowering: mixed genuine MLIR index-dialect ops with
  already-LLVM-converted operands, a type mismatch that made every
  Array.prototype.splice() call fail to lower correctly (the only
  .splice() call in the test suite was commented out because of this).
  Reworked to stay consistently in the LLVM-converted domain, matching
  every sibling array-mutation lowering.
- GlobalOpLowering: the side-effect enumeration deciding whether a global
  initializer can be inlined as a constant vs. must run as a real
  constructor was missing several ops with real side effects (array
  push/unshift/splice/pop/shift, delete, set-length, string concat/
  char-to-string).
- LandingPadFixPass: MadeChange was set unconditionally for every
  landingpad visited, even when nothing was rewritten, causing needless
  analysis invalidation on every EH-containing function on every compile.
- Win32ExceptionPass: the PHI-user removal loop erased only the first PHI
  user found then unconditionally erased the underlying value, violating
  LLVM's no-remaining-uses invariant when more than one PHI referenced the
  same to-be-removed value (own TODO comment flagged this as incomplete).
- Win32ExceptionPass: getThrowFn's fallback path created a stray
  _CxxThrowException Function without attaching it to the module.

Added 00array_splice.ts (JIT + AOT) covering shrink/grow/equal-size
splice cases, since this path had zero prior test coverage. Full ctest
suite green: 692/692.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: ASDAlexander77

tslang/lib/TypeScript/LowerToAffineLoops.cpp

@@ -2002,13 +2002,17 @@ class SwitchStateOpLowering : public TsPattern<mlir_ts::SwitchStateOp>
 
         SmallVector<mlir::Block *> caseDestinations;
 
-        SmallPtrSet<Operation *, 16> stateLabels;
+        // Collect state labels in program (walk) order: dispatch below is positional
+        // (case N -> caseDestinations[N-1]), so an order-scrambling container here
+        // (e.g. SmallPtrSet, which only preserves insertion order below its inline
+        // capacity) would silently corrupt generator resume dispatch.
+        SmallVector<Operation *> stateLabels;
 
         // select all states
         auto visitorAllStateLabels = [&](Operation *op) {
             if (auto stateLabelOp = dyn_cast_or_null<mlir_ts::StateLabelOp>(op))
             {
-                stateLabels.insert(op);
+                stateLabels.push_back(op);
             }
         };
 

tslang/lib/TypeScript/LowerToLLVM.cpp

@@ -584,7 +584,7 @@ class StringConcatOpLowering : public TsLlvmPattern<mlir_ts::StringConcatOp>
         auto allocInStack = op.getAllocInStack().has_value() && op.getAllocInStack().value()
                             && !isInsidePresplitCoroutine(op);
 
-        mlir::Value newStringValue = allocInStack ? ch.Alloca(i8PtrTy, size, true)
+        mlir::Value newStringValue = allocInStack ? ch.Alloca(th.getI8Type(), size, true)
                                                   : ch.MemoryAlloc(size);
 
         // copy
@@ -2682,54 +2682,61 @@ struct ArraySpliceOpLowering : public TsLlvmPattern<mlir_ts::ArraySpliceOp>
 
         auto incSizeAsLLVMType = clh.createIndexConstantOf(llvmIndexType, transformed.getItems().size());
 
-        mlir::Value newCountAsIndexType = rewriter.create<mlir::index::SubOp>(loc, indexType, ValueRange{countAsIndexType, decSizeAsIndexType});
-        newCountAsIndexType = rewriter.create<mlir::index::AddOp>(loc, indexType, ValueRange{newCountAsIndexType, incSizeAsLLVMType});
+        // Keep all arithmetic in the already-LLVM-converted domain (llvmIndexType), matching
+        // every sibling array-mutation lowering (ArrayPushOp/ArrayUnshiftOp/ArrayShiftOp) --
+        // mlir::index::*Op ops require genuinely `index`-typed operands, but countAsIndexType
+        // (despite its name) is loaded as llvmIndexType, and incSizeAsLLVMType is already
+        // LLVM-typed, so mixing them into index:: ops here was a dialect-operand mismatch.
+        mlir::Value newCountAsLLVMType = rewriter.create<LLVM::SubOp>(loc, llvmIndexType, ValueRange{countAsIndexType, decSizeAsLLVMType});
+        newCountAsLLVMType = rewriter.create<LLVM::AddOp>(loc, llvmIndexType, ValueRange{newCountAsLLVMType, incSizeAsLLVMType});
 
-        auto sizeOfTypeValue = rewriter.create<mlir_ts::SizeOfOp>(loc, indexType, elementType);
+        auto sizeOfTypeValueMLIR = rewriter.create<mlir_ts::SizeOfOp>(loc, indexType, elementType);
+        auto sizeOfTypeValue = rewriter.create<mlir_ts::DialectCastOp>(loc, llvmIndexType, sizeOfTypeValueMLIR);
 
         auto multSizeOfTypeValue =
-            rewriter.create<mlir::index::MulOp>(loc, indexType, ValueRange{sizeOfTypeValue, newCountAsIndexType});
+            rewriter.create<LLVM::MulOp>(loc, llvmIndexType, ValueRange{sizeOfTypeValue, newCountAsLLVMType});
 
         auto increaseArrayFunc = [&](OpBuilder &builder, Location location) -> mlir::Value {
             auto allocated = ch.MemoryRealloc(currentPtr, multSizeOfTypeValue);
 
-            auto moveCountAsIndexType =
-                rewriter.create<mlir::index::SubOp>(loc, indexType, ValueRange{countAsIndexType, startIndexAsIndexType});
-            moveCountAsIndexType =
-                rewriter.create<mlir::index::SubOp>(loc, indexType, ValueRange{moveCountAsIndexType, decSizeAsIndexType});
+            auto moveCountAsLLVMType =
+                rewriter.create<LLVM::SubOp>(loc, llvmIndexType, ValueRange{countAsIndexType, startIndexAsLLVMType});
+            moveCountAsLLVMType =
+                rewriter.create<LLVM::SubOp>(loc, llvmIndexType, ValueRange{moveCountAsLLVMType, decSizeAsLLVMType});
 
             // realloc
             auto offsetStart = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, allocated, ValueRange{startIndexAsLLVMType});
             auto offsetFrom = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, offsetStart, ValueRange{decSizeAsLLVMType});
             auto offsetTo = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, offsetStart, ValueRange{incSizeAsLLVMType});
-            auto moveCountAsIndexTypeAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, indexType, moveCountAsIndexType);
+            auto moveCountAsIndexTypeAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, indexType, moveCountAsLLVMType);
             rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetTo, offsetFrom, moveCountAsIndexTypeAdapt);
 
             return allocated;
         };
 
         auto decreaseArrayFunc = [&](OpBuilder &builder, Location location) -> mlir::Value {
 
-            auto moveCountAsIndexType =
-                rewriter.create<mlir::index::SubOp>(loc, indexType, ValueRange{countAsIndexType, startIndexAsIndexType});
-            moveCountAsIndexType =
-                rewriter.create<mlir::index::SubOp>(loc, indexType, ValueRange{moveCountAsIndexType, decSizeAsIndexType});
+            auto moveCountAsLLVMType =
+                rewriter.create<LLVM::SubOp>(loc, llvmIndexType, ValueRange{countAsIndexType, startIndexAsLLVMType});
+            moveCountAsLLVMType =
+                rewriter.create<LLVM::SubOp>(loc, llvmIndexType, ValueRange{moveCountAsLLVMType, decSizeAsLLVMType});
 
             // realloc
             auto offsetStart = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, currentPtr, ValueRange{startIndexAsLLVMType});
             auto offsetFrom = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, offsetStart, ValueRange{decSizeAsLLVMType});
             auto offsetTo = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, offsetStart, ValueRange{incSizeAsLLVMType});
 
-            rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetTo, offsetFrom, moveCountAsIndexType);
+            auto moveCountAsIndexTypeAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, indexType, moveCountAsLLVMType);
+            rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetTo, offsetFrom, moveCountAsIndexTypeAdapt);
 
             auto allocated = ch.MemoryRealloc(currentPtr, multSizeOfTypeValue);
             return allocated;
         };
 
-        auto cond = rewriter.create<arith::CmpIOp>(loc, th.getLLVMBoolType(), arith::CmpIPredicateAttr::get(rewriter.getContext(), arith::CmpIPredicate::ugt), incSizeAsLLVMType, decSizeAsIndexType);
+        auto cond = rewriter.create<LLVM::ICmpOp>(loc, LLVM::ICmpPredicate::ugt, incSizeAsLLVMType, decSizeAsLLVMType);
         auto allocated = clh.conditionalExpressionLowering(loc, ptrType, cond, increaseArrayFunc, decreaseArrayFunc);
 
-        mlir::Value index = startIndexAsIndexType;
+        mlir::Value indexAsLLVMType = startIndexAsLLVMType;
         auto next = false;
         mlir::Value value1;
         for (auto itemPair : llvm::zip(transformed.getItems(), spliceOp.getItems()))
@@ -2744,11 +2751,10 @@ struct ArraySpliceOpLowering : public TsLlvmPattern<mlir_ts::ArraySpliceOp>
                     value1 = clh.createIndexConstantOf(llvmIndexType, 1);
                 }
 
-                index = rewriter.create<mlir::index::AddOp>(loc, indexType, ValueRange{index, value1});
+                indexAsLLVMType = rewriter.create<LLVM::AddOp>(loc, llvmIndexType, ValueRange{indexAsLLVMType, value1});
             }
 
             // save new element
-            auto indexAsLLVMType = rewriter.create<mlir::index::CastUOp>(loc, llvmIndexType, index);
             auto offset = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, allocated, ValueRange{indexAsLLVMType});
 
             auto effectiveItem = item;
@@ -2766,7 +2772,6 @@ struct ArraySpliceOpLowering : public TsLlvmPattern<mlir_ts::ArraySpliceOp>
         }
 
         rewriter.create<LLVM::StoreOp>(loc, allocated, currentPtrPtr);
-        auto newCountAsLLVMType = rewriter.create<mlir::index::CastUOp>(loc, llvmIndexType, newCountAsIndexType);
         rewriter.create<LLVM::StoreOp>(loc, newCountAsLLVMType, countAsIndexTypePtr);
 
         rewriter.replaceOp(spliceOp, ValueRange{newCountAsLLVMType});
@@ -3495,8 +3500,12 @@ struct GlobalOpLowering : public TsLlvmPattern<mlir_ts::GlobalOp>
                 isa<mlir_ts::SymbolCallInternalOp>(op) || isa<mlir_ts::CallInternalOp>(op) ||
                 isa<mlir_ts::CallHybridInternalOp>(op) || isa<mlir_ts::VariableOp>(op) || isa<mlir_ts::AllocaOp>(op) ||
                 isa<mlir_ts::CreateArrayOp>(op) || isa<mlir_ts::NewEmptyArrayOp>(op) || isa<mlir_ts::CreateTupleOp>(op) ||
-                isa<mlir_ts::LoadOp>(op) || isa<mlir_ts::StoreOp>(op) || 
-                isa<mlir_ts::LoadLibraryPermanentlyOp>(op) || isa<mlir_ts::SearchForAddressOfSymbolOp>(op))
+                isa<mlir_ts::LoadOp>(op) || isa<mlir_ts::StoreOp>(op) ||
+                isa<mlir_ts::LoadLibraryPermanentlyOp>(op) || isa<mlir_ts::SearchForAddressOfSymbolOp>(op) ||
+                isa<mlir_ts::ArrayPushOp>(op) || isa<mlir_ts::ArrayUnshiftOp>(op) || isa<mlir_ts::ArraySpliceOp>(op) ||
+                isa<mlir_ts::ArrayPopOp>(op) || isa<mlir_ts::ArrayShiftOp>(op) || isa<mlir_ts::DeleteOp>(op) ||
+                isa<mlir_ts::SetLengthOfOp>(op) || isa<mlir_ts::SetStringLengthOp>(op) ||
+                isa<mlir_ts::StringConcatOp>(op) || isa<mlir_ts::CharToStringOp>(op))
             {
                 createAsGlobalConstructor = true;
             }

tslang/lib/TypeScriptExceptionPass/LandingPadFixPass.cpp

@@ -51,9 +51,9 @@ struct LandingPadFixPassCode
                 newLandingPad->setCleanup(true);
                 LPI->replaceAllUsesWith(newLandingPad);
                 LPI->eraseFromParent();
-            }
 
-            MadeChange = true;
+                MadeChange = true;
+            }
         }
 
         LLVM_DEBUG(llvm::dbgs() << "\n!! LANDFIX Change: " << MadeChange;);

tslang/lib/TypeScriptExceptionPass/Win32ExceptionPass.cpp

@@ -541,22 +541,18 @@ struct Win32ExceptionPassCode
         // remove
         for (auto CI : toRemoveWorkSet)
         {
-            // TODO: we need to fix issue wit PHI node after inline works
-            if (CI->getNumUses() > 0)
+            // Erase every PHI-node user first (there can be more than one, e.g. after
+            // inlining merges control flow through several PHIs referencing the same
+            // to-be-removed value); eraseFromParent() below requires no uses remain.
+            while (!CI->use_empty())
             {
-                for (auto &U : CI->uses())
+                auto UI = llvm::find_if(CI->users(), [](llvm::User *U) { return isa<PHINode>(U); });
+                if (UI == CI->user_end())
                 {
-                    if (U.getUser() && isa<PHINode>(U.getUser()))
-                    {
-                        // Instruction *UserI = cast<Instruction>(U.getUser());
-                        PHINode *UserPHI = cast<PHINode>(U.getUser());
-                        if (UserPHI)
-                        {
-                            UserPHI->eraseFromParent();
-                            break;
-                        }
-                    }
+                    break;
                 }
+
+                cast<PHINode>(*UI)->eraseFromParent();
             }
 
             CI->eraseFromParent();
@@ -631,7 +627,7 @@ struct Win32ExceptionPassCode
         // which describes the exception.
         llvm::Type *Args[] = {PointerType::get(Ctx, 0), PointerType::get(Ctx, 0)};
         auto *FTy = llvm::FunctionType::get(Type::getVoidTy(Ctx), Args, /*isVarArg=*/false);
-        auto Throw = Function::Create(FTy, llvm::GlobalValue::LinkageTypes::InternalLinkage, "_CxxThrowException");
+        auto Throw = Function::Create(FTy, llvm::GlobalValue::LinkageTypes::ExternalLinkage, "_CxxThrowException", module);
         /*
         // _CxxThrowException is stdcall on 32-bit x86 platforms.
         if (CGM.getTarget().getTriple().getArch() == llvm::Triple::x86)

tslang/test/tester/CMakeLists.txt

@@ -164,6 +164,7 @@ add_test(NAME test-compile-00-arrays2 COMMAND test-runner "${PROJECT_SOURCE_DIR}
 add_test(NAME test-compile-00-arrays3 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array3.ts")
 add_test(NAME test-compile-00-arrays4-push-pop COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array4_push_pop.ts")
 add_test(NAME test-compile-00-array-shift COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_shift.ts")
+add_test(NAME test-compile-00-array-splice COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_splice.ts")
 add_test(NAME test-compile-00-arrays5-deconstruct COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array5_deconst.ts")
 add_test(NAME test-compile-00-arrays6 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array6.ts")
 add_test(NAME test-compile-00-arrays7 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts")
@@ -502,6 +503,7 @@ add_test(NAME test-jit-00-arrays2 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR
 add_test(NAME test-jit-00-arrays3 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array3.ts")
 add_test(NAME test-jit-00-arrays4-push-pop COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array4_push_pop.ts")
 add_test(NAME test-jit-00-array-shift COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_shift.ts")
+add_test(NAME test-jit-00-array-splice COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_splice.ts")
 add_test(NAME test-jit-00-arrays5-deconstruct COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array5_deconst.ts")
 add_test(NAME test-jit-00-arrays6 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array6.ts")
 add_test(NAME test-jit-00-arrays7 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts")

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

@@ -0,0 +1,29 @@
+function main() {
+    // shrink: delete more than inserted
+    let a: number[] = [1, 2, 3, 4, 5];
+    a.splice(1, 2);
+    assert(a.length == 3, "shrink len");
+    assert(a[0] == 1, "shrink 0");
+    assert(a[1] == 4, "shrink 1");
+    assert(a[2] == 5, "shrink 2");
+
+    // grow: insert more than deleted
+    let b: number[] = [1, 2, 3];
+    b.splice(1, 1, 10, 20, 30);
+    assert(b.length == 5, "grow len");
+    assert(b[0] == 1, "grow 0");
+    assert(b[1] == 10, "grow 1");
+    assert(b[2] == 20, "grow 2");
+    assert(b[3] == 30, "grow 3");
+    assert(b[4] == 3, "grow 4");
+
+    // equal: same count deleted as inserted
+    let c: number[] = [1, 2, 3, 4];
+    c.splice(1, 2, 99);
+    assert(c.length == 3, "equal len");
+    assert(c[0] == 1, "equal 0");
+    assert(c[1] == 99, "equal 1");
+    assert(c[2] == 4, "equal 2");
+
+    print("done.");
+}