mirror of
https://github.com/roc-lang/roc.git
synced 2025-12-23 08:48:03 +00:00
Merge pull request #8669 from roc-lang/fix-issue-8666
Fix panic when using polymorphic numeric as list index
This commit is contained in:
commit
12dac2cec7
12 changed files with 141 additions and 36 deletions
|
|
@ -1258,7 +1258,7 @@ fn checkDef(self: *Self, def_idx: CIR.Def.Idx, env: *Env) std.mem.Allocator.Erro
|
|||
_ = try self.checkExpr(def.expr, env, .no_expectation);
|
||||
}
|
||||
|
||||
// Now that we are existing the scope, we must generalize then pop this rank
|
||||
// Now that we are exiting the scope, we must generalize then pop this rank
|
||||
try self.generalizer.generalize(self.gpa, &env.var_pool, env.rank());
|
||||
|
||||
// Check any accumulated static dispatch constraints
|
||||
|
|
@ -2898,9 +2898,28 @@ fn checkExpr(self: *Self, expr_idx: CIR.Expr.Idx, env: *Env, expected: Expected)
|
|||
}
|
||||
|
||||
const pat_var = ModuleEnv.varFrom(lookup.pattern_idx);
|
||||
const resolved_pat = self.types.resolveVar(pat_var).desc;
|
||||
const resolved_pat = self.types.resolveVar(pat_var);
|
||||
|
||||
if (resolved_pat.rank == Rank.generalized) {
|
||||
// Check if this is a generalized var that should NOT be instantiated.
|
||||
// Numeric literals with from_numeral constraints should unify directly
|
||||
// so that the concrete type propagates back to the definition site.
|
||||
// This fixes GitHub issue #8666 where polymorphic numerics defaulted to Dec.
|
||||
const should_instantiate = blk: {
|
||||
if (resolved_pat.desc.rank != Rank.generalized) break :blk false;
|
||||
// Don't instantiate if this has a from_numeral constraint
|
||||
if (resolved_pat.desc.content == .flex) {
|
||||
const flex = resolved_pat.desc.content.flex;
|
||||
const constraints = self.types.sliceStaticDispatchConstraints(flex.constraints);
|
||||
for (constraints) |constraint| {
|
||||
if (constraint.origin == .from_numeral) {
|
||||
break :blk false;
|
||||
}
|
||||
}
|
||||
}
|
||||
break :blk true;
|
||||
};
|
||||
|
||||
if (should_instantiate) {
|
||||
const instantiated = try self.instantiateVar(pat_var, env, .use_last_var);
|
||||
_ = try self.unify(expr_var, instantiated, env);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -200,3 +200,57 @@ test "numeric literal in comparison unifies with typed operand" {
|
|||
}
|
||||
try testing.expect(found_result);
|
||||
}
|
||||
|
||||
test "polymorphic numeric in list used as List.get index unifies to U64 - regression #8666" {
|
||||
// When a numeric literal is stored in an unannotated list and later used as
|
||||
// an index to List.get (which takes U64), the type should unify to U64.
|
||||
// This is a regression test for GitHub issue #8666 where the type remained
|
||||
// as a flex var, causing the interpreter to default it to Dec layout.
|
||||
const source =
|
||||
\\list = [10, 20, 30]
|
||||
\\index = 0
|
||||
\\result = List.get(list, index)
|
||||
;
|
||||
|
||||
var test_env = try TestEnv.init("Test", source);
|
||||
defer test_env.deinit();
|
||||
|
||||
// First verify no type errors
|
||||
try test_env.assertNoErrors();
|
||||
|
||||
// The key assertion: `index` should be U64 after unification with List.get's parameter.
|
||||
// Find the `index` definition and check its type.
|
||||
const ModuleEnv = @import("can").ModuleEnv;
|
||||
const defs_slice = test_env.module_env.store.sliceDefs(test_env.module_env.all_defs);
|
||||
var found_index = false;
|
||||
for (defs_slice) |def_idx| {
|
||||
const def = test_env.module_env.store.getDef(def_idx);
|
||||
const ptrn = test_env.module_env.store.getPattern(def.pattern);
|
||||
if (ptrn == .assign) {
|
||||
const def_name = test_env.module_env.getIdentStoreConst().getText(ptrn.assign.ident);
|
||||
if (std.mem.eql(u8, def_name, "index")) {
|
||||
found_index = true;
|
||||
|
||||
// Get the type from the expression (the literal 0)
|
||||
const expr_var = ModuleEnv.varFrom(def.expr);
|
||||
try test_env.type_writer.write(expr_var);
|
||||
const expr_type = test_env.type_writer.get();
|
||||
|
||||
// After unification with List.get's U64 parameter, should be U64
|
||||
try testing.expectEqualStrings("U64", expr_type);
|
||||
|
||||
// Also verify the pattern has the same type
|
||||
const pattern_var = ModuleEnv.varFrom(def.pattern);
|
||||
try test_env.type_writer.write(pattern_var);
|
||||
const pattern_type = test_env.type_writer.get();
|
||||
try testing.expectEqualStrings("U64", pattern_type);
|
||||
|
||||
// Verify the pattern is NOT generalized (numeric literals shouldn't be)
|
||||
const resolved_pat = test_env.module_env.types.resolveVar(pattern_var);
|
||||
try testing.expect(resolved_pat.desc.rank != types.Rank.generalized);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
try testing.expect(found_index);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1353,10 +1353,10 @@ test "check type - expect" {
|
|||
\\ x
|
||||
\\}
|
||||
;
|
||||
// Inside lambdas, numeric flex vars ARE generalized (to support polymorphic functions).
|
||||
// Each use of `x` gets a fresh instance, so constraints from `x == 1` don't
|
||||
// propagate to the generalized type. Only `from_numeral` from the def is captured.
|
||||
try checkTypesModule(source, .{ .pass = .last_def }, "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]");
|
||||
// Numeric literals with from_numeral constraints are NOT generalized (GitHub #8666).
|
||||
// This means constraints from `x == 1` (the is_eq constraint) DO propagate back
|
||||
// to the definition of x, along with the original from_numeral constraint.
|
||||
try checkTypesModule(source, .{ .pass = .last_def }, "a where [a.is_eq : a, a -> Bool, a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]");
|
||||
}
|
||||
|
||||
test "check type - expect not bool" {
|
||||
|
|
|
|||
|
|
@ -1428,6 +1428,26 @@ test "List.len returns proper U64 nominal type for method calls - regression" {
|
|||
, "3", .no_trace);
|
||||
}
|
||||
|
||||
test "List.get with polymorphic numeric index - regression #8666" {
|
||||
// Regression test for GitHub issue #8666: interpreter panic when using
|
||||
// a polymorphic numeric type as a list index.
|
||||
//
|
||||
// The bug occurred because numeric literals with from_numeral constraints
|
||||
// were being generalized, causing each use to get a fresh instantiation.
|
||||
// This meant the concrete U64 type from List.get didn't propagate back
|
||||
// to the original definition, leaving it as a flex var that defaulted to Dec.
|
||||
//
|
||||
// The fix: don't generalize vars with from_numeral constraints, and don't
|
||||
// instantiate them during lookup, so constraint propagation works correctly.
|
||||
try runExpectInt(
|
||||
\\{
|
||||
\\ list = [10, 20, 30]
|
||||
\\ index = 0
|
||||
\\ match List.get(list, index) { Ok(v) => v, _ => 0 }
|
||||
\\}
|
||||
, 10, .no_trace);
|
||||
}
|
||||
|
||||
test "for loop element type extracted from list runtime type - regression #8664" {
|
||||
// Regression test for InvalidMethodReceiver when calling methods on elements
|
||||
// from a for loop over a list passed to an untyped function parameter.
|
||||
|
|
|
|||
|
|
@ -205,18 +205,17 @@ pub const Generalizer = struct {
|
|||
if (@intFromEnum(resolved.desc.rank) < rank_to_generalize_int) {
|
||||
// Rank was lowered during adjustment - variable escaped
|
||||
try var_pool.addVarToRank(resolved.var_, resolved.desc.rank);
|
||||
} else if (rank_to_generalize_int == @intFromEnum(Rank.top_level) and self.hasNumeralConstraint(resolved.desc.content)) {
|
||||
// Flex var with numeric constraint at TOP LEVEL - don't generalize.
|
||||
} else if (self.hasNumeralConstraint(resolved.desc.content)) {
|
||||
// Flex var with numeric constraint - don't generalize at ANY rank.
|
||||
// This ensures numeric literals like `x = 15` stay monomorphic so that
|
||||
// later usage like `I64.to_str(x)` can constrain x to I64.
|
||||
// later usage like `List.get(list, x)` can constrain x to U64.
|
||||
// Without this, let-generalization would create a fresh copy at each use,
|
||||
// leaving the original as an unconstrained flex var that defaults to Dec.
|
||||
// leaving the original as an unconstrained flex var that defaults to Dec
|
||||
// at runtime, causing panics when used as integer indices (GitHub #8666).
|
||||
//
|
||||
// However, at rank > top_level (inside lambdas OR inside nested blocks),
|
||||
// we DO generalize numeric literals. This allows:
|
||||
// - Polymorphic functions like `|a| a + 1` to work correctly
|
||||
// - Numeric literals in blocks like `{ n = 42; use_as_i64(n); use_as_dec(n) }`
|
||||
// to be used polymorphically within that block's scope.
|
||||
// Note: Polymorphic functions like `|a| a + 1` still work correctly because
|
||||
// the numeric literal `1` inside the lambda body gets its own type variable
|
||||
// that will be instantiated fresh for each call to the function.
|
||||
try var_pool.addVarToRank(resolved.var_, resolved.desc.rank);
|
||||
} else {
|
||||
// Rank unchanged - safe to generalize
|
||||
|
|
|
|||
|
|
@ -1076,12 +1076,12 @@ main = |_| {
|
|||
(patt (type "{ value: a, wrapper: List(a) } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "{ value: Str, wrapper: List(Str) }"))
|
||||
(patt (type "{ value: a, wrapper: List(a) } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "{ level1: { collection: List(_a), level2: { items: List(b), level3: { data: List(_c), value: d } } }, results: List({ data: List(e), tag: Str }) } where [b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)]), d.from_numeral : Numeral -> Try(d, [InvalidNumeral(Str)]), e.from_numeral : Numeral -> Try(e, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "{ level1: { collection: List(_a), level2: { items: List(b), level3: { data: List(_c), value: b } } }, results: List({ data: List(d), tag: Str }) } where [b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)]), d.from_numeral : Numeral -> Try(d, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "List(a) where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "{ base: a, derived: List(b) } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)]), b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "{ computations: { from_frac: a, from_num: b, list_from_num: List(c) }, empty_lists: { in_list: List(List(_d)), in_record: { data: List(_e) }, raw: List(_f) }, numbers: { float: g, list: List(h), value: i }, strings: { list: List(Str), value: Str } } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)]), b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)]), c.from_numeral : Numeral -> Try(c, [InvalidNumeral(Str)]), g.from_numeral : Numeral -> Try(g, [InvalidNumeral(Str)]), h.from_numeral : Numeral -> Try(h, [InvalidNumeral(Str)]), i.from_numeral : Numeral -> Try(i, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "{ base: a, derived: List(a) } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "{ computations: { from_frac: a, from_num: b, list_from_num: List(b) }, empty_lists: { in_list: List(List(_c)), in_record: { data: List(_d) }, raw: List(_e) }, numbers: { float: a, list: List(b), value: b }, strings: { list: List(Str), value: Str } } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)]), b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "_arg -> a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]")))
|
||||
(expressions
|
||||
(expr (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
|
|
@ -1105,11 +1105,11 @@ main = |_| {
|
|||
(expr (type "{ value: a, wrapper: List(a) } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "{ value: Str, wrapper: List(Str) }"))
|
||||
(expr (type "{ value: a, wrapper: List(a) } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "{ level1: { collection: List(_a), level2: { items: List(b), level3: { data: List(_c), value: d } } }, results: List({ data: List(e), tag: Str }) } where [b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)]), d.from_numeral : Numeral -> Try(d, [InvalidNumeral(Str)]), e.from_numeral : Numeral -> Try(e, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "{ level1: { collection: List(_a), level2: { items: List(b), level3: { data: List(_c), value: b } } }, results: List({ data: List(d), tag: Str }) } where [b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)]), d.from_numeral : Numeral -> Try(d, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "List(a) where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "{ base: a, derived: List(b) } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)]), b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "{ computations: { from_frac: a, from_num: b, list_from_num: List(c) }, empty_lists: { in_list: List(List(_d)), in_record: { data: List(_e) }, raw: List(_f) }, numbers: { float: g, list: List(h), value: i }, strings: { list: List(Str), value: Str } } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)]), b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)]), c.from_numeral : Numeral -> Try(c, [InvalidNumeral(Str)]), g.from_numeral : Numeral -> Try(g, [InvalidNumeral(Str)]), h.from_numeral : Numeral -> Try(h, [InvalidNumeral(Str)]), i.from_numeral : Numeral -> Try(i, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "{ base: a, derived: List(a) } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "{ computations: { from_frac: a, from_num: b, list_from_num: List(b) }, empty_lists: { in_list: List(List(_c)), in_record: { data: List(_d) }, raw: List(_e) }, numbers: { float: a, list: List(b), value: b }, strings: { list: List(Str), value: Str } } where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)]), b.from_numeral : Numeral -> Try(b, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "_arg -> a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))))
|
||||
~~~
|
||||
|
|
|
|||
|
|
@ -145,7 +145,7 @@ deepType = C
|
|||
~~~clojure
|
||||
(inferred-types
|
||||
(defs
|
||||
(patt (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "U64"))
|
||||
(patt (type "U64"))
|
||||
(patt (type "Foo.Level1.Level2.Level3")))
|
||||
(type_decls
|
||||
|
|
@ -158,7 +158,7 @@ deepType = C
|
|||
(nominal (type "Foo.Level1.Level2.Level3")
|
||||
(ty-header (name "Foo.Level1.Level2.Level3"))))
|
||||
(expressions
|
||||
(expr (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "U64"))
|
||||
(expr (type "U64"))
|
||||
(expr (type "Foo.Level1.Level2.Level3"))))
|
||||
~~~
|
||||
|
|
|
|||
|
|
@ -76,12 +76,12 @@ useBar = Foo.bar
|
|||
~~~clojure
|
||||
(inferred-types
|
||||
(defs
|
||||
(patt (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "U64"))
|
||||
(patt (type "U64")))
|
||||
(type_decls
|
||||
(nominal (type "Foo")
|
||||
(ty-header (name "Foo"))))
|
||||
(expressions
|
||||
(expr (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "U64"))
|
||||
(expr (type "U64"))))
|
||||
~~~
|
||||
|
|
|
|||
|
|
@ -111,7 +111,7 @@ myNum = Foo.Bar.baz
|
|||
~~~clojure
|
||||
(inferred-types
|
||||
(defs
|
||||
(patt (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "U64"))
|
||||
(patt (type "Foo.Bar"))
|
||||
(patt (type "U64")))
|
||||
(type_decls
|
||||
|
|
@ -120,7 +120,7 @@ myNum = Foo.Bar.baz
|
|||
(nominal (type "Foo.Bar")
|
||||
(ty-header (name "Foo.Bar"))))
|
||||
(expressions
|
||||
(expr (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "U64"))
|
||||
(expr (type "Foo.Bar"))
|
||||
(expr (type "U64"))))
|
||||
~~~
|
||||
|
|
|
|||
|
|
@ -97,14 +97,14 @@ result = myBar
|
|||
~~~clojure
|
||||
(inferred-types
|
||||
(defs
|
||||
(patt (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(patt (type "U64"))
|
||||
(patt (type "U64"))
|
||||
(patt (type "U64")))
|
||||
(type_decls
|
||||
(nominal (type "Foo")
|
||||
(ty-header (name "Foo"))))
|
||||
(expressions
|
||||
(expr (type "a where [a.from_numeral : Numeral -> Try(a, [InvalidNumeral(Str)])]"))
|
||||
(expr (type "U64"))
|
||||
(expr (type "U64"))
|
||||
(expr (type "U64"))))
|
||||
~~~
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
# META
|
||||
~~~ini
|
||||
description=Numeric let-generalization inside nested block (rank > top_level)
|
||||
description=Numeric without let-generalization gives type error (only lambdas get let-generalization)
|
||||
type=expr
|
||||
~~~
|
||||
# SOURCE
|
||||
|
|
@ -13,9 +13,22 @@ type=expr
|
|||
}
|
||||
~~~
|
||||
# EXPECTED
|
||||
NIL
|
||||
TYPE MISMATCH - numeric_let_generalize_in_block.md:4:20:4:21
|
||||
# PROBLEMS
|
||||
NIL
|
||||
**TYPE MISMATCH**
|
||||
The first argument being passed to this function has the wrong type:
|
||||
**numeric_let_generalize_in_block.md:4:20:4:21:**
|
||||
```roc
|
||||
b = Dec.to_str(n)
|
||||
```
|
||||
^
|
||||
|
||||
This argument has the type:
|
||||
_I64_
|
||||
|
||||
But the function needs the first argument to be:
|
||||
_Dec_
|
||||
|
||||
# TOKENS
|
||||
~~~zig
|
||||
OpenCurly,
|
||||
|
|
@ -87,5 +100,5 @@ EndOfFile,
|
|||
~~~
|
||||
# TYPES
|
||||
~~~clojure
|
||||
(expr (type "Str"))
|
||||
(expr (type "Error"))
|
||||
~~~
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
# META
|
||||
~~~ini
|
||||
description=Numeric without annotation, multiple uses with different types (each use gets fresh type)
|
||||
description=Numeric without annotation, later use gives type error (no let-generalization for non-lambdas)
|
||||
type=repl
|
||||
~~~
|
||||
# SOURCE
|
||||
|
|
@ -17,6 +17,6 @@ assigned `a`
|
|||
---
|
||||
assigned `b`
|
||||
---
|
||||
"4242.0"
|
||||
TYPE MISMATCH
|
||||
# PROBLEMS
|
||||
NIL
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue