Conversation
internal_ty_contains_var scanned only a closure type's param_tys and
ret_ty, omitting captured_types — unlike every other closure-type walk
(conform_ty, the solver's can_conform check, and the canonical sub-type
enumeration), which all include it.
As a result, an inference variable appearing solely in captured_types
passed the occurs-check, so it could be bound to a closure type that
transitively contains itself. The resulting infinite type overflows the
stack and crashes the compiler:
fn unify<T, +Drop<T>>(_a: T, _b: T) {}
fn foo() {
let x = Default::default(); // x: ?T
let f = || { let _s = @x; }; // captured_types = [@?T]
unify(x, f); // binds ?T := closure containing @?T
}
Scan captured_types too, so the cyclic binding is rejected and the
program reports a normal type mismatch (E2041) instead of crashing.
Adds a regression test exercising the capture-only path.
|
This change is |
PR SummaryLow Risk Overview Adds a semantic regression test in closure diagnostics: Reviewed by Cursor Bugbot for commit d15963e. Bugbot is set up for automated code reviews on this repo. Configure here. |
eytan-starkware
left a comment
There was a problem hiding this comment.
Does pub fn type_dependencies<'db> reveal a similar problem?
@eytan-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
orizi
left a comment
There was a problem hiding this comment.
no - as type_dependencies is used for finding the dependencies of a type for sierra-gen purposes - so there - it is about the types the closure actually holds (so basically the closure itself as a struct) - the params and return types are completely irrelevant there.
@orizi made 1 comment.
Reviewable status:
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed all commit messages and made 1 comment.
Reviewable status:
Summary
Fixes a stack-overflow (occurs-check infinite loop) that occurred when a closure capturing a variable of inferred type was unified with that same variable. The
internal_ty_contains_varcheck forTypeLongId::Closurewas not includingcaptured_typesin its occurs-check traversal, meaning the inference engine could not detect the cyclic type constraint and would recurse indefinitely.The fix extends the occurs-check to also iterate over
closure.captured_typesalongsideparam_tysandret_ty, usingitertools::chain!to unify the three into a single iterator.Type of change
Please check one:
Why is this change needed?
When a closure captured a variable whose type was still being inferred, and that closure was then passed to a function expecting the same inferred type, the occurs-check (
internal_ty_contains_var) failed to detect the cycle because it did not inspectcaptured_types. This caused the compiler to stack-overflow instead of emitting a proper type mismatch diagnostic.What was the behavior or documentation before?
Compiling code like:
would cause a stack overflow in the compiler.
What is the behavior or documentation after?
The compiler now correctly detects the cyclic type constraint and emits a proper diagnostic:
Related issue or discussion (if any)
Occurs-check regression for closures capturing inferred-type variables.
Additional context
A regression test has been added to
crates/cairo-lang-semantic/src/expr/test_data/closurecovering this exact scenario.