Skip to content
Closed
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
76 commits
Select commit Hold shift + click to select a range
221ef89
Add desugaring for minus
rtfeldman Nov 10, 2025
64f2657
Desugar other arithmetic operators
rtfeldman Nov 10, 2025
63aa24d
Improve a test
rtfeldman Nov 10, 2025
924727f
Fix some operator precedence
rtfeldman Nov 10, 2025
843f150
Consolidate some operator string stuff
rtfeldman Nov 10, 2025
cdeef94
Use a hash map for operator names
rtfeldman Nov 10, 2025
c56d31d
Update a comment
rtfeldman Nov 10, 2025
1279c22
rem and pow
rtfeldman Nov 10, 2025
6b76350
Fix size expectation
rtfeldman Nov 10, 2025
4fbfe53
s/div_by/div
rtfeldman Nov 10, 2025
dc64579
Fix some debug stuff for wasm
rtfeldman Nov 10, 2025
32800bf
Improve unification for numbers
rtfeldman Nov 10, 2025
aec008d
Remove unnecessary addVarToRank
rtfeldman Nov 10, 2025
4b9c2ee
Add a test
rtfeldman Nov 11, 2025
7525844
Merge remote-tracking branch 'origin/main' into desugar-arithmetic
rtfeldman Nov 11, 2025
39a70c8
Use desc in panic
rtfeldman Nov 11, 2025
afb4fc4
Eliminate Num
rtfeldman Nov 11, 2025
e1d3cc7
Fix placeholder behavior
rtfeldman Nov 11, 2025
18b90c4
Fix a type checker bug
rtfeldman Nov 12, 2025
10914d9
Add some fallbacks
rtfeldman Nov 12, 2025
4f50c91
panic with a descriptive error on bug
rtfeldman Nov 12, 2025
ec0d249
Introduce more debug panicking
rtfeldman Nov 12, 2025
285216e
Reproduce type-checking bug
rtfeldman Nov 12, 2025
b40e647
describe bug and fix
rtfeldman Nov 12, 2025
ad1aca3
Fix literals
rtfeldman Nov 12, 2025
317d4b5
Fix more type checking stuff
rtfeldman Nov 12, 2025
1e7d58a
More type checking fixes
rtfeldman Nov 12, 2025
5ea1338
Unskip some tests
rtfeldman Nov 12, 2025
4e65b87
Fix infinite recursion
rtfeldman Nov 12, 2025
7515cbf
More type checker fixes
rtfeldman Nov 12, 2025
3f04beb
In a surprise twist, more type-checking fixes
rtfeldman Nov 12, 2025
808b582
Update .rules
rtfeldman Nov 12, 2025
c541f43
Allow builtin shadowing in Builtin.roc
rtfeldman Nov 13, 2025
18d165a
Update type printing for numbers
rtfeldman Nov 13, 2025
501d373
Re-skip some tests that origin/main skips
rtfeldman Nov 13, 2025
bb94a11
Update tests
rtfeldman Nov 13, 2025
3e984fb
wip
rtfeldman Nov 13, 2025
2f54fb8
Revise builtin stuff more
rtfeldman Nov 13, 2025
0f39cab
Remove evalArithmeticBinop
rtfeldman Nov 13, 2025
29ce2a6
Fix desugaring bug
rtfeldman Nov 13, 2025
84aee27
Revert "Fix desugaring bug"
rtfeldman Nov 13, 2025
205d7d9
Remove incorrect special-casing
rtfeldman Nov 13, 2025
8c25ade
Fix a constraint bug
rtfeldman Nov 13, 2025
4b3ecb7
Revert "Fix a constraint bug"
rtfeldman Nov 14, 2025
2ddd072
Partial fix
rtfeldman Nov 14, 2025
58ef395
Revert "Partial fix"
rtfeldman Nov 14, 2025
928f7a9
Skip some tests
rtfeldman Nov 14, 2025
cbf6f29
Update snapshots
rtfeldman Nov 14, 2025
e498d1e
Merge remote-tracking branch 'origin/main' into desugar-arithmetic
rtfeldman Nov 14, 2025
4014e31
Merge remote-tracking branch 'origin/main' into desugar-arithmetic
rtfeldman Nov 14, 2025
60791cf
Fix missing .list unification
rtfeldman Nov 14, 2025
06f4207
Clean up stuff
rtfeldman Nov 14, 2025
ddd7c7e
Add a new test
rtfeldman Nov 14, 2025
2984b54
Add .num_unbound_if_builtin
rtfeldman Nov 14, 2025
41e308d
Fix eval for .plus
rtfeldman Nov 14, 2025
bdbebab
Fix `+` desugaring
rtfeldman Nov 15, 2025
cfab46f
Update some tests
rtfeldman Nov 15, 2025
51658cf
Clean up some type checking tests
rtfeldman Nov 15, 2025
7adb4fd
wip trying stuff
rtfeldman Nov 15, 2025
2ff418a
Get number tests passing
rtfeldman Nov 15, 2025
ce4e14f
Delete obsolete test
rtfeldman Nov 15, 2025
18afe73
Get rid of the runtime type store
rtfeldman Nov 15, 2025
3b6551b
Wire in Builtin.roc into more tests
rtfeldman Nov 15, 2025
4761d4e
Don't skip low level tests anymore
rtfeldman Nov 15, 2025
37e09b4
wip
rtfeldman Nov 15, 2025
0b86661
Remove int_unbound
rtfeldman Nov 15, 2025
cc6d869
Fix some tests
rtfeldman Nov 15, 2025
f32a6f6
Fix a unification regression
rtfeldman Nov 16, 2025
398d71b
wip
rtfeldman Nov 16, 2025
8321038
Fix some constraint unification
rtfeldman Nov 16, 2025
fbf61f8
Before eliminating frac_unbound
rtfeldman Nov 16, 2025
6d5972a
Remove frac_unbound and frac_poly
rtfeldman Nov 16, 2025
97b36eb
Eliminate the *_poly numeric primitives
rtfeldman Nov 16, 2025
e077179
Fix some tests
rtfeldman Nov 16, 2025
5f1c48b
Remove "Num" from some tests
rtfeldman Nov 16, 2025
131b4b8
Fix missing from_dec_digits constraints
rtfeldman Nov 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions BUG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# Type Checker Bug: Missing Static Dispatch Constraints on Binary Operation Results

## Summary

When the type checker processes binary operations (like `x + x`), it creates a flex type variable for the result but **fails to add the corresponding static dispatch constraint**. This causes runtime failures when the interpreter tries to compute layouts, as it has no information to infer a default numeric type.

## Failing Test Case

**Test**: `test "simple lambdas"` in `/Users/rtfeldman/code/roc4/src/eval/test/eval_test.zig:394`

**Specific failing expression**: `(|x| x + x)(7)` (expected result: 14)

## Detailed Analysis

### What's Happening

**1. Compile-Time Type:**
- The result of the `x + x` expression has type: **`flex` with ZERO constraints**
- Specifically: `.{ .flex = .{ .name = null, .constraints = .{ .start = @enumFromInt(0), .count = 0 } } }`

**2. Runtime Type:**
- After `translateTypeVar`, it remains: **`flex` with ZERO constraints**
- The `translateTypeVar` function correctly copies the (empty) constraint list from compile-time to runtime

**3. Layout Computation:**
- When trying to compute the layout for this flex var, we hit `/Users/rtfeldman/code/roc4/src/layout/store.zig:1359`
- Error: `LayoutError.BugUnboxedFlexVar` because the flex var has no constraints to infer a default type from

### What Constraints Should It Have

For the expression `(|x| x + x)(7)`:

**Expected constraints on the result**:
- The result of `x + x` should have a **`plus` constraint** from the `+` operator
- When `x` is unified with `7` (an integer literal), `x` should get a **`from_int_digits` constraint**
- Therefore, the result should also inherit numeric constraints

**What we're getting**:
- **ZERO constraints** on the result flex var

### Why Is This Wrong?

When the type checker processes `|x| x + x`:
1. `x` is a parameter with initially unconstrained flex type
2. The `+` operator should add a `plus` static dispatch constraint to the result type
3. When called with `(7)`, the literal `7` should add `from_int_digits` constraint
4. These constraints should flow through to the result

**The type checker is failing to add the `plus` constraint to the result of binary operations.**

## Consequences

**Without constraints**:
- The layout computation has NO information about what type this is
- It can't default to I128 or Dec because there's no hint about numeric operations
- We return `BugUnboxedFlexVar` error
- The test panics

**With constraints (expected behavior)**:
- If the flex var had a `plus` constraint (or `from_int_digits` from the literal), we would:
- Detect it's a numeric operation
- Default to I128 for integer operations
- Compute the layout successfully
- Execute the code and return 14

## Root Cause Location

**The bug is in the type checker** (likely in constraint generation during binary operation type-checking).

When processing binary operations like `x + x`, the type checker should:
1. Create a fresh flex var for the result
2. Add a static dispatch constraint (e.g., `plus`) to that flex var
3. This would signal "this is the result of an addition, so it should be a numeric type"

**Expected**: Result type = `flex` with `plus` constraint
**Actual**: Result type = `flex` with NO constraints

This is a **type checker bug**, not an interpreter or layout bug. The fix belongs in the constraint generation phase of type checking.

## Test Coverage

A reproduction test has been added at:
- `/Users/rtfeldman/code/roc4/src/check/test/binop_constraint_test.zig`

This test verifies that binary operations add the appropriate static dispatch constraints to their result type variables.

## Workaround

The layout computation code in `/Users/rtfeldman/code/roc4/src/layout/store.zig` has been enhanced to:
1. Check flex vars for numeric constraints
2. Default to Dec if `from_dec_digits` constraint is present
3. Default to I128 for any other numeric constraint (like `plus`, `from_int_digits`, etc.)
4. Return `LayoutError.BugUnboxedFlexVar` only for truly unconstrained flex vars

This workaround allows constrained flex vars to work, but the root cause (missing constraints) still needs to be fixed in the type checker.
260 changes: 260 additions & 0 deletions FIX.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
# Fix Plan: Add Static Dispatch Constraints to Binary Operation Results

## Problem Summary

When the type checker processes binary operations (e.g., `x + y`), it creates static dispatch constraints for the operands but **fails to add the constraint to the result type**. This means the result gets an unconstrained flex variable, which causes runtime failures when the layout computation has no information to infer a default type.

## Root Cause

**Location**: `/Users/rtfeldman/code/roc4/src/check/Check.zig:3719-3750`

In the `checkBinopExpr` function's static dispatch path:

```zig
// Line 3724: Create return type - just a fresh unconstrained variable
const ret_var = try self.fresh(env, expr_region);

// Lines 3727-3731: Create constraint function type
const constraint_fn_var = try self.freshFromContent(.{ .structure = .{ .fn_unbound = Func{
.args = args_range, // [lhs_var, rhs_var]
.ret = ret_var, // ← PROBLEM: unconstrained!
.needs_instantiation = false,
} } }, env, expr_region);

// Lines 3734-3739: Create and add constraint
const constraint = StaticDispatchConstraint{
.fn_name = method_name, // e.g., "plus"
.fn_var = constraint_fn_var,
.origin = .desugared_binop,
};
const constraint_range = try self.types.appendStaticDispatchConstraints(&.{constraint});

// Lines 3742-3747: Add constraint to LHS only
const constrained_var = try self.freshFromContent(
.{ .flex = Flex{ .name = null, .constraints = constraint_range } },
env,
expr_region,
);
_ = try self.unify(constrained_var, lhs_var, env);

// Line 3750: Unify result with unconstrained return var
_ = try self.unify(expr_var, ret_var, env); // ← BUG: ret_var has no constraints!
```

**The Issue**: For a binary operation like `a + b`, the proper static dispatch constraint should be:
```
a.plus : a, a -> a
```

This means ALL THREE type variables (left operand, right operand, and result) should reference the same constrained type `a`. However, currently:
- ✅ LHS (`a`) gets the constraint
- ❌ RHS (`b`) doesn't explicitly get the constraint (but might get it via unification with LHS)
- ❌ **Result has NO constraint** - it's just a fresh flex variable

## The Fix

### Step 1: Add Constraint to Return Type Variable

**File**: `/Users/rtfeldman/code/roc4/src/check/Check.zig`
**Function**: `checkBinopExpr`
**Lines**: ~3724-3750

Instead of creating an unconstrained `ret_var`, create a constrained return variable with the same constraint as the LHS.

**Before**:
```zig
// Line 3724: Create return type - just a fresh unconstrained variable
const ret_var = try self.fresh(env, expr_region);

// ... create constraint ...

// Add constraint only to LHS
const constrained_var = try self.freshFromContent(
.{ .flex = Flex{ .name = null, .constraints = constraint_range } },
env,
expr_region,
);
_ = try self.unify(constrained_var, lhs_var, env);

// Unify result with unconstrained return var
_ = try self.unify(expr_var, ret_var, env);
```

**After**:
```zig
// Create the static dispatch constraint FIRST (before creating ret_var)
const constraint = StaticDispatchConstraint{
.fn_name = method_name,
.fn_var = undefined, // Will be filled in after creating constraint_fn_var
.origin = .desugared_binop,
};

// Create constrained return variable with the SAME constraint
const ret_var = try self.freshFromContent(
.{ .flex = Flex{ .name = null, .constraints = constraint_range } },
env,
expr_region,
);

// Now create the constraint function type with constrained return
const args_range = try self.types.appendVars(&.{ lhs_var, rhs_var });
const constraint_fn_var = try self.freshFromContent(.{ .structure = .{ .fn_unbound = Func{
.args = args_range,
.ret = ret_var, // Now constrained!
.needs_instantiation = false,
} } }, env, expr_region);

// Update the constraint with the function var
// (This requires making constraint mutable or reconstructing it)
const final_constraint = StaticDispatchConstraint{
.fn_name = method_name,
.fn_var = constraint_fn_var,
.origin = .desugared_binop,
};
const constraint_range = try self.types.appendStaticDispatchConstraints(&.{final_constraint});

// Add constraint to LHS
const constrained_lhs_var = try self.freshFromContent(
.{ .flex = Flex{ .name = null, .constraints = constraint_range } },
env,
expr_region,
);
_ = try self.unify(constrained_lhs_var, lhs_var, env);

// Unify result with constrained return var
_ = try self.unify(expr_var, ret_var, env); // Now ret_var has the constraint!
```

**WAIT** - there's a circular dependency issue here. The constraint needs the `constraint_fn_var`, but the `constraint_fn_var` needs the `ret_var`, but the `ret_var` needs the constraint. Let me think about this differently...

### Alternative Approach: Add Constraint After Creating Function Type

Actually, looking at the code more carefully, I think the issue is that we need to create THREE constrained variables (lhs, rhs, ret) all with the SAME constraint range. Here's the correct approach:

```zig
// Step 1: Create unconstrained return var (temporarily)
const ret_var = try self.fresh(env, expr_region);

// Step 2: Create the constraint function type
const args_range = try self.types.appendVars(&.{ lhs_var, rhs_var });
const constraint_fn_var = try self.freshFromContent(.{ .structure = .{ .fn_unbound = Func{
.args = args_range,
.ret = ret_var,
.needs_instantiation = false,
} } }, env, expr_region);

// Step 3: Create the constraint
const constraint = StaticDispatchConstraint{
.fn_name = method_name,
.fn_var = constraint_fn_var,
.origin = .desugared_binop,
};
const constraint_range = try self.types.appendStaticDispatchConstraints(&.{constraint});

// Step 4: Add constraint to LHS (existing code)
const constrained_lhs_var = try self.freshFromContent(
.{ .flex = Flex{ .name = null, .constraints = constraint_range } },
env,
expr_region,
);
_ = try self.unify(constrained_lhs_var, lhs_var, env);

// Step 5: **NEW** - Add the SAME constraint to the return variable
const constrained_ret_var = try self.freshFromContent(
.{ .flex = Flex{ .name = null, .constraints = constraint_range } },
env,
expr_region,
);
_ = try self.unify(constrained_ret_var, ret_var, env);

// Step 6: Unify result with the constrained return var
_ = try self.unify(expr_var, constrained_ret_var, env);
```

This approach:
1. Creates the constraint with an unconstrained return type
2. Creates constrained flex variables for both LHS and return type
3. Unifies them with their respective type variables
4. This propagates the constraint to both the LHS and the result

### Step 2: Test the Fix

Run the tests in `/Users/rtfeldman/code/roc4/src/check/test/binop_constraint_test.zig`:

```bash
zig build test
```

Expected results:
- ✅ `addition result should have 'plus' constraint` - PASS
- ✅ `subtraction result should have 'minus' constraint` - PASS
- ✅ `multiplication result should have 'mul' constraint` - PASS
- ✅ `division result should have 'div' or 'div_trunc' constraint` - PASS
- ✅ `chained operations should accumulate constraints` - PASS
- ✅ `integer literal should have 'from_int_digits' constraint` - PASS (or fail if literals also need fixing)
- ✅ `decimal literal should have 'from_dec_digits' constraint` - PASS (or fail if literals also need fixing)

### Step 3: Verify Interpreter Tests Pass

The failing interpreter tests should now pass:

```bash
zig build test
```

Expected:
- `test "simple lambdas"` in `/Users/rtfeldman/code/roc4/src/eval/test/eval_test.zig:394` should PASS
- Specifically `(|x| x + x)(7)` should evaluate to `14` without panicking

### Step 4: Handle Numeric Literals (If Needed)

The last two tests check that numeric literals (`42` and `3.14`) have the appropriate constraints. If these tests fail, we need to also fix the numeric literal handling.

**Location**: `/Users/rtfeldman/code/roc4/src/check/Check.zig` around line 2189 (`.e_num` case)

The numeric literal handling should add `from_int_digits` or `from_dec_digits` constraints to the type variable. Currently it creates `Num.num_unbound` types but may not be adding the constraint properly.

If the tests fail, investigate how numeric literals are supposed to get their constraints and ensure they're being added.

## Testing Strategy

1. **Unit Tests**: The `binop_constraint_test.zig` tests will verify that constraints are present after type checking
2. **Integration Tests**: The existing `eval_test.zig` tests will verify that the interpreter can execute code that previously panicked
3. **Manual Testing**: Try expressions like:
- `|x, y| x + y` should type as `a, a -> a where [a.plus : a, a -> a]`
- `(|x| x + x)(7)` should evaluate to `14`
- `add = |x, y| x + y; add(10, 5)` should evaluate to `15`

## Potential Issues and Solutions

### Issue 1: RHS Constraint

**Question**: Should the RHS also get an explicit constraint?

**Answer**: In the current implementation, the RHS doesn't get an explicit constraint added, but it gets constrained through unification with the LHS (via the function signature). This should be sufficient, but if tests show otherwise, we can add an explicit constraint to RHS as well using the same pattern.

### Issue 2: Constraint Ordering

**Question**: Does the order of unifications matter?

**Answer**: Yes, potentially. We should unify the constrained vars with the actual vars BEFORE unifying expr_var with ret_var, to ensure constraints propagate properly.

### Issue 3: Multiple Constraints

**Question**: What if a type variable already has constraints and we're adding more?

**Answer**: The unification process should merge constraints. If there are issues, we may need to check if the variable already has constraints and merge them appropriately.

## Success Criteria

✅ All 7 tests in `binop_constraint_test.zig` pass
✅ All interpreter tests pass (no panics, correct results)
✅ Build passes with no errors
✅ The test count returns to ~1411/1426 or better

## Code Locations Reference

- **Main fix location**: `/Users/rtfeldman/code/roc4/src/check/Check.zig:3719-3750` (function `checkBinopExpr`)
- **Test file**: `/Users/rtfeldman/code/roc4/src/check/test/binop_constraint_test.zig`
- **Bug documentation**: `/Users/rtfeldman/code/roc4/BUG.md`
- **Integration tests**: `/Users/rtfeldman/code/roc4/src/eval/test/eval_test.zig`
10 changes: 10 additions & 0 deletions src/base/Ident.zig
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ pub const FROM_INT_DIGITS_METHOD_NAME = "from_int_digits";
pub const FROM_DEC_DIGITS_METHOD_NAME = "from_dec_digits";
/// Method name for addition - used by + operator desugaring
pub const PLUS_METHOD_NAME = "plus";
/// Method name for subtraction - used by - operator desugaring
pub const MINUS_METHOD_NAME = "minus";
/// Method name for multiplication - used by * operator desugaring
pub const TIMES_METHOD_NAME = "times";
/// Method name for division - used by / operator desugaring
pub const DIV_METHOD_NAME = "div";
/// Method name for truncating division - used by // operator desugaring
pub const DIV_TRUNC_METHOD_NAME = "div_trunc";
/// Method name for remainder - used by % operator desugaring
pub const REM_METHOD_NAME = "rem";

/// The original text of the identifier.
raw_text: []const u8,
Expand Down
Loading