Fix ===/any coercion bugs, in-operator crash, and add regression coverage

docs/bugs/00generator_manual_next.ts

@@ -0,0 +1,48 @@
+// regression test: calling .next() manually on a real `function*` generator (as opposed
+// to driving it via `for...of`) used to never advance the generator's internal state.
+//
+// root cause: a `const` binding was stored in the symbol table as a bare SSA value with
+// no backing stack storage. Each `g.next()` property access needed a ref to recover
+// `this` for the bound ".next" method, and without real storage it fell back to
+// allocating a fresh temporary copy of `g` -- re-seeded from the pristine, never-mutated
+// original -- on every single call site. So every manual `.next()` call restarted the
+// generator instead of resuming it. `for...of` happened to work because its lowering
+// materializes the generator object into one persistent local up front and reuses it.
+//
+// fix: a const whose value is a tuple with a bound-method field (e.g. a generator or
+// closure object) now gets real stack storage, matching what for...of already relied on.
+
+function* gen() {
+    for (let i = 0; i < 5; i++) {
+        yield i;
+    }
+}
+
+function main() {
+    const g = gen();
+
+    let r = g.next();
+    assert(!r.done);
+    assert(r.value == 0);
+
+    r = g.next();
+    assert(!r.done);
+    assert(r.value == 1);
+
+    r = g.next();
+    assert(!r.done);
+    assert(r.value == 2);
+
+    r = g.next();
+    assert(!r.done);
+    assert(r.value == 3);
+
+    r = g.next();
+    assert(!r.done);
+    assert(r.value == 4);
+
+    r = g.next();
+    assert(r.done);
+
+    print("done.");
+}

tslang/include/TypeScript/LowerToLLVM/CodeLogicHelper.h

@@ -114,19 +114,25 @@ class CodeLogicHelper
         // then block
         auto *thenBlock = rewriter.createBlock(continuationBlock);
         auto thenValue = thenBuilder(rewriter, loc);
+        // thenBuilder may itself branch into further blocks (e.g. a nested
+        // conditionalExpressionLowering call) -- always branch to the result block
+        // from wherever the insertion point actually ended up, not from the
+        // (possibly stale) block handle captured before the builder ran.
+        auto *thenEndBlock = rewriter.getInsertionBlock();
 
         // else block
         auto *elseBlock = rewriter.createBlock(continuationBlock);
         auto elseValue = elseBuilder(rewriter, loc);
+        auto *elseEndBlock = rewriter.getInsertionBlock();
 
         // result block
         auto *resultBlock = rewriter.createBlock(continuationBlock, TypeRange{type}, {loc});
         rewriter.create<LLVM::BrOp>(loc, ValueRange{}, continuationBlock);
 
-        rewriter.setInsertionPointToEnd(thenBlock);
+        rewriter.setInsertionPointToEnd(thenEndBlock);
         rewriter.create<LLVM::BrOp>(loc, ValueRange{thenValue}, resultBlock);
 
-        rewriter.setInsertionPointToEnd(elseBlock);
+        rewriter.setInsertionPointToEnd(elseEndBlock);
         rewriter.create<LLVM::BrOp>(loc, ValueRange{elseValue}, resultBlock);
 
         // Generate assertion test.

tslang/include/TypeScript/MLIRLogic/MLIRTypeHelper.h

@@ -2275,6 +2275,8 @@ class MLIRTypeHelper
     {
         LLVM_DEBUG(llvm::dbgs() << "!! get type of field '" << fieldName << "' of '" << srcType << "'\n";);
 
+        srcType = stripLiteralType(srcType);
+
         if (auto constTupleType = dyn_cast<mlir_ts::ConstTupleType>(srcType))
         {
             auto index = constTupleType.getIndex(fieldName);

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -3647,7 +3647,15 @@ class MLIRGenImpl
 
         NodeFactory nf(NodeFactoryFlags::None);
 
-        if (auto hasLength = evaluateProperty(binaryExpressionAST->right, LENGTH_FIELD_NAME, genContext))
+        // the length-based numeric-index rewrite below only makes sense when the left
+        // side is actually a number (e.g. `i in arr`); a string-literal left side (e.g.
+        // `"length" in arr` or `"push" in arr`) must fall through to the general
+        // field-lookup path further down instead, otherwise we'd cast a string to an
+        // index/int type and generate invalid IR.
+        auto leftIsStringLiteral = binaryExpressionAST->left == SyntaxKind::StringLiteral
+            || binaryExpressionAST->left == SyntaxKind::NoSubstitutionTemplateLiteral;
+
+        if (!leftIsStringLiteral && evaluateProperty(binaryExpressionAST->right, LENGTH_FIELD_NAME, genContext))
         {
             auto cond1 = nf.createBinaryExpression(
                 binaryExpressionAST->left, nf.createToken(SyntaxKind::LessThanToken),
@@ -4600,6 +4608,37 @@ class MLIRGenImpl
         return mlir::IntegerType::get(builder.getContext(), width, mlir::IntegerType::Signed);
     }
 
+    // JS/TS `===`/`!==` never coerce: operands of clearly different primitive kinds
+    // (boolean vs number, boolean vs string, string vs number) are simply unequal,
+    // full stop. Everything downstream of this function (adjustTypesForBinaryOp's
+    // numeric-sync loop, LogicalBinaryOpLowering) is shared with `==`/`!=`, which DO
+    // coerce, so without this check `1 === true` was being widened to a common
+    // numeric type just like `1 == true` and wrongly evaluated to `true`.
+    bool isDefinitelyMismatchedForStrictEquals(mlir::Type leftType, mlir::Type rightType)
+    {
+        auto isBoolean = [](mlir::Type type) { return isa<mlir_ts::BooleanType>(type); };
+        auto isString = [](mlir::Type type) { return isa<mlir_ts::StringType>(type); };
+        auto isNumeric = [](mlir::Type type) { return type.isIntOrIndexOrFloat() && !isa<mlir_ts::BooleanType>(type); };
+
+        auto leftIsBoolean = isBoolean(leftType);
+        auto rightIsBoolean = isBoolean(rightType);
+        auto leftIsString = isString(leftType);
+        auto rightIsString = isString(rightType);
+        auto leftIsNumeric = isNumeric(leftType);
+        auto rightIsNumeric = isNumeric(rightType);
+
+        // only fire when BOTH sides are known, unambiguous primitives (not any/union/
+        // object/etc., which may still need the general coercion/toPrimitive machinery)
+        auto leftIsKnownPrimitive = leftIsBoolean || leftIsString || leftIsNumeric;
+        auto rightIsKnownPrimitive = rightIsBoolean || rightIsString || rightIsNumeric;
+        if (!leftIsKnownPrimitive || !rightIsKnownPrimitive)
+        {
+            return false;
+        }
+
+        return (leftIsBoolean != rightIsBoolean) || (leftIsString != rightIsString) || (leftIsNumeric != rightIsNumeric);
+    }
+
     // TODO: review it, seems like big hack
     mlir::LogicalResult adjustTypesForBinaryOp(mlir::Location location, SyntaxKind opCode, mlir::Value &leftExpressionValue,
                                                mlir::Value &rightExpressionValue, const GenContext &genContext)
@@ -4609,6 +4648,15 @@ class MLIRGenImpl
             return mlir::success();
         }
 
+        if ((opCode == SyntaxKind::EqualsEqualsEqualsToken || opCode == SyntaxKind::ExclamationEqualsEqualsToken)
+            && isDefinitelyMismatchedForStrictEquals(leftExpressionValue.getType(), rightExpressionValue.getType()))
+        {
+            auto result = opCode == SyntaxKind::ExclamationEqualsEqualsToken;
+            leftExpressionValue = builder.create<mlir_ts::ConstantOp>(location, getBooleanType(), builder.getBoolAttr(result));
+            rightExpressionValue = builder.create<mlir_ts::ConstantOp>(location, getBooleanType(), builder.getBoolAttr(true));
+            return mlir::success();
+        }
+
         if (MLIRTypeCore::canHaveToPrimitiveMethod(leftExpressionValue.getType())
             && evaluateProperty(location, leftExpressionValue, SYMBOL_TO_PRIMITIVE, genContext)
             && !isa<mlir_ts::UndefinedType>(rightExpressionValue.getType())
@@ -4671,8 +4719,64 @@ class MLIRGenImpl
             }
 
             break;
-        case SyntaxKind::AsteriskToken:
+        case SyntaxKind::PlusToken:
+        {
+            // this is exactly the untyped default: case below (left/right type sync,
+            // string-preferring) -- PlusToken used to fall through to it unconditionally.
+            // Preserved as-is so string concat (`"fo" + 1`) and ordinary numeric-literal
+            // widening (`numberParam + 1`) keep working exactly like before.
+            auto leftType = leftExpressionValue.getType();
+            if (isa<mlir_ts::StringType>(rightExpressionValue.getType()))
+            {
+                leftType = rightExpressionValue.getType();
+                if (leftType != leftExpressionValue.getType())
+                {
+                    CAST(leftExpressionValue, location, leftType, leftExpressionValue, genContext);
+                }
+            }
+
+            auto rightType = rightExpressionValue.getType();
+            if (leftType != rightType)
+            {
+                CAST(rightExpressionValue, location, leftType, rightExpressionValue, genContext);
+            }
+
+            // additionally: when neither side is a string (so this isn't concat) and
+            // both sides already had the SAME boolean type, the sync above was a no-op
+            // (leftType == rightType already), so booleans reached
+            // ArithmeticBinaryOpLowering as raw i1 and wrapped (`true + true` -> false
+            // instead of 2). Widen them to number in that case.
+            if (!isa<mlir_ts::StringType>(leftExpressionValue.getType()) && !isa<mlir_ts::StringType>(rightExpressionValue.getType()))
+            {
+                if (isa<mlir_ts::BooleanType>(leftExpressionValue.getType()))
+                {
+                    CAST(leftExpressionValue, location, getNumberType(), leftExpressionValue, genContext);
+                }
+
+                if (isa<mlir_ts::BooleanType>(rightExpressionValue.getType()))
+                {
+                    CAST(rightExpressionValue, location, getNumberType(), rightExpressionValue, genContext);
+                }
+            }
+
+            break;
+        }
         case SyntaxKind::MinusToken:
+            // unlike PlusToken, MinusToken never does string concat, so it's safe to
+            // widen booleans here and then fall through to the same cross-type sync
+            // used by the other arithmetic/comparison operators below (e.g. `any - number`).
+            if (isa<mlir_ts::BooleanType>(leftExpressionValue.getType()))
+            {
+                CAST(leftExpressionValue, location, getNumberType(), leftExpressionValue, genContext);
+            }
+
+            if (isa<mlir_ts::BooleanType>(rightExpressionValue.getType()))
+            {
+                CAST(rightExpressionValue, location, getNumberType(), rightExpressionValue, genContext);
+            }
+
+            [[fallthrough]];
+        case SyntaxKind::AsteriskToken:
         case SyntaxKind::EqualsEqualsToken:
         case SyntaxKind::EqualsEqualsEqualsToken:
         case SyntaxKind::ExclamationEqualsToken:

tslang/test/tester/CMakeLists.txt

@@ -121,6 +121,8 @@ add_test(NAME test-compile-00-enums-multiple COMMAND test-runner "${PROJECT_SOUR
 add_test(NAME test-compile-01-enums COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/01enum.ts")
 add_test(NAME test-compile-00-numbers COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00numbers.ts")
 add_test(NAME test-compile-00-equals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00equals.ts")
+add_test(NAME test-compile-00-bool-arith-ops COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00bool_arith_ops.ts")
+add_test(NAME test-compile-00-mixed-type-ops COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00mixed_type_ops.ts")
 add_test(NAME test-compile-00-funcs COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs.ts")
 add_test(NAME test-compile-00-funcs-capture COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_capture.ts")
 add_test(NAME test-compile-00-funcs-vararg COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_vararg.ts")
@@ -216,6 +218,8 @@ add_test(NAME test-compile-02-sizeof COMMAND test-runner "${PROJECT_SOURCE_DIR}/
 add_test(NAME test-compile-00-new-delete COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00new_delete.ts")
 add_test(NAME test-compile-00-void COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00void.ts")
 add_test(NAME test-compile-00-in COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00in.ts")
+add_test(NAME test-compile-00-in-method-names COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00in_method_names.ts")
+add_test(NAME test-compile-00-class-structural-extends COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_structural_extends.ts")
 add_test(NAME test-compile-00-instanceof COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof.ts")
 add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
 add_test(NAME test-compile-00-class-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts")
@@ -461,6 +465,8 @@ add_test(NAME test-jit-00-enums-multiple COMMAND test-runner -jit "${PROJECT_SOU
 add_test(NAME test-jit-01-enums COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/01enum.ts")
 add_test(NAME test-jit-00-numbers COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00numbers.ts")
 add_test(NAME test-jit-00-equals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00equals.ts")
+add_test(NAME test-jit-00-bool-arith-ops COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00bool_arith_ops.ts")
+add_test(NAME test-jit-00-mixed-type-ops COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00mixed_type_ops.ts")
 add_test(NAME test-jit-00-funcs COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs.ts")
 add_test(NAME test-jit-00-funcs-capture COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_capture.ts")
 add_test(NAME test-jit-00-funcs-vararg COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_vararg.ts")
@@ -555,6 +561,8 @@ add_test(NAME test-jit-02-sizeof COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}
 add_test(NAME test-jit-00-new-delete COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00new_delete.ts")
 add_test(NAME test-jit-00-void COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00void.ts")
 add_test(NAME test-jit-00-in COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00in.ts")
+add_test(NAME test-jit-00-in-method-names COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00in_method_names.ts")
+add_test(NAME test-jit-00-class-structural-extends COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_structural_extends.ts")
 add_test(NAME test-jit-00-instanceof COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof.ts")
 add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
 add_test(NAME test-jit-00-class-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts")

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

@@ -0,0 +1,42 @@
+// regression test: boolean operands in arithmetic/comparison binary ops must coerce to
+// number the same way JS/TS does (true -> 1, false -> 0), not operate on the raw i1
+// representation.
+//
+// bugs found and fixed:
+// - `+`/`-` had no case forcing numeric coercion when both operands were already the
+//   same type (boolean), unlike `/`/`%`/`**` which always force getNumberType(). So
+//   `true + true` computed 1+1 as a wrapping i1 add (result: false) instead of 2, and
+//   `true - false` similarly wrapped instead of giving 1.
+// - ordering comparisons (`> >= < <=`) picked their icmp predicate via
+//   Type::isUnsignedInteger(), which is false for the custom BooleanType (a signless
+//   i1), so they fell to the *signed* predicate -- where i1 `true` (bit pattern 1)
+//   reads as -1, making `true > false` compare as `-1 > 0` (false) instead of true.
+
+function main() {
+    assert(true + true == 2);
+    assert(true + false == 1);
+    assert(false + false == 0);
+
+    assert(true - false == 1);
+    assert(false - true == -1);
+    assert(true - true == 0);
+
+    assert(true * 2 == 2);
+    assert(false * 2 == 0);
+
+    assert(true > false);
+    assert(!(false > true));
+    assert(false < true);
+    assert(!(true < false));
+    assert(true >= true);
+    assert(true <= true);
+    assert(true >= false);
+    assert(!(false >= true));
+
+    assert(true == true);
+    assert(true != false);
+    assert(true === true);
+    assert(true !== false);
+
+    print("done.");
+}

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

@@ -0,0 +1,29 @@
+// regression test for #238: getFieldTypeByFieldName's ClassType branch looked up
+// class field/method info in the *interface* registry (getInterfaceInfoByFullName)
+// instead of the class registry (getClassInfoByFullName). Since classes and
+// interfaces are registered in separate tables, this lookup silently failed for
+// every field/method on a real class -- breaking both the `in` operator and
+// structural generic constraints (`T extends { length: number }`) matched against
+// a class instance.
+
+class Box {
+    length: number;
+
+    constructor(length: number) {
+        this.length = length;
+    }
+}
+
+function getLength<T extends { length: number }>(x: T): number {
+    return x.length;
+}
+
+function main() {
+    const b = new Box(42);
+
+    assert("length" in b);
+
+    assert(getLength(b) == 42);
+
+    print("done.");
+}

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

@@ -0,0 +1,49 @@
+// regression test for #238: getFieldTypeByFieldName used to hit llvm_unreachable
+// when the `in` operator (or structural type checks that reuse the same lookup)
+// was asked about a name that is a method/extension-function name rather than a
+// real data field -- e.g. "push" on an array, "fromCharCode"/"charAt" on a string,
+// or a method name on a class instance. All five branches (Array, ConstArray,
+// String, Interface, Class) must return "not found" instead of aborting.
+
+class Point {
+    x: number;
+    y: number;
+
+    constructor(x: number, y: number) {
+        this.x = x;
+        this.y = y;
+    }
+
+    dist(): number {
+        return this.x * this.x + this.y * this.y;
+    }
+}
+
+function main() {
+    const trees = ["redwood", "bay", "cedar"];
+
+    // real data fields/indices are found
+    assert("length" in trees);
+    assert(0 in trees);
+
+    // method/extension names are not data fields, must resolve to false, not abort
+    assert(!("push" in trees));
+    assert(!("pop" in trees));
+    assert(!("entries" in trees));
+    assert(!("madeUpName" in trees));
+
+    const s = "hello";
+
+    assert("length" in s);
+    assert(!("charAt" in s));
+    assert(!("fromCharCode" in s));
+
+    const p = new Point(3, 4);
+
+    assert("x" in p);
+    assert("y" in p);
+    assert("dist" in p);
+    assert(!("madeUpField" in p));
+
+    print("done.");
+}