Support closures (with simple usage of traits) #10
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for closures and simple trait usage to the Thrust refinement type system. The changes enable the system to handle closure types, trait method dispatch, and the RustCall ABI used by closures.
- Added support for closure types by treating them as tuples of captured variables
- Implemented RustCall ABI handling with parameter expansion for closure calls
- Enhanced function type resolution to properly handle trait method dispatch through
Instance::resolve
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ui/pass/trait_param.rs | Test for trait-based generic function with correct assertion |
| tests/ui/pass/trait.rs | Test for direct trait method calls |
| tests/ui/pass/closure_param.rs | Test for closure passed as function parameter |
| tests/ui/pass/closure_no_capture.rs | Test for closure without captures |
| tests/ui/pass/closure_mut_0.rs | Test for closure with mutable captures (no parameters) |
| tests/ui/pass/closure_mut.rs | Test for closure with mutable captures and parameters |
| tests/ui/fail/trait_param.rs | Negative test for trait-based generic function |
| tests/ui/fail/trait.rs | Negative test for trait method calls |
| tests/ui/fail/closure_param.rs | Negative test for closure parameter handling |
| tests/ui/fail/closure_no_capture.rs | Negative test for closure without captures |
| tests/ui/fail/closure_mut_0.rs | Negative test for mutable closure (checks x == 2 when x == 3) |
| tests/ui/fail/closure_mut.rs | Negative test for mutable closure with parameters |
| src/rty.rs | Added FunctionAbi enum, abi field to FunctionType, and deref method for RefinedType |
| src/refine/template.rs | Added closure type handling and ABI tracking in type builders |
| src/chc.rs | Added boxed helper method for Term construction |
| src/analyze/crate_.rs | Refactored to use new local_fn_sig method for better closure support |
| src/analyze/basic_block.rs | Implemented RustCall ABI parameter expansion and closure type handling |
| src/analyze.rs | Added local_fn_sig methods to extract signatures from MIR for closures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| existential, | ||
| ))); | ||
| } | ||
|
|
There was a problem hiding this comment.
The code calls elem.deref() on tuple elements during RustCall expansion, assuming that all elements are pointer types. This assumption should be documented or validated, as it may not be immediately obvious why tuple elements would always be pointers in the rust-call ABI context. Consider adding a comment explaining this invariant.
| // In the RustCall ABI, the last argument is a tuple of pointer types (e.g., `&mut self`, `&args`). | |
| // Therefore, it is safe to call `deref()` on each tuple element here. |
| /// Computes the signature of the local function. | ||
| /// | ||
| /// This is a drop-in replacement of `self.tcx.fn_sig(local_def_id).instantiate_identity().skip_binder()`, | ||
| /// but extracts parameter and return types directly from [`mir::Body`] to obtain a signature that | ||
| /// reflects the actual type of lifted closure functions. |
There was a problem hiding this comment.
The documentation states this is a "drop-in replacement" but actually provides different behavior for closure types, extracting types from MIR body rather than using fn_sig. Consider clarifying that this method provides enhanced behavior for closures, not just a simple replacement.
| pub fn deref(self) -> Self { | ||
| let RefinedType { | ||
| ty, | ||
| refinement: outer_refinement, | ||
| } = self; | ||
| let inner_ty = ty.into_pointer().expect("invalid deref"); | ||
| if inner_ty.is_mut() { | ||
| // losing info about proph | ||
| panic!("invalid deref"); | ||
| } | ||
| let RefinedType { | ||
| ty: inner_ty, | ||
| refinement: mut inner_refinement, | ||
| } = *inner_ty.elem; | ||
| inner_refinement.push_conj( | ||
| outer_refinement.subst_value_var(|| chc::Term::var(RefinedTypeVar::Value).boxed()), | ||
| ); | ||
| RefinedType { | ||
| ty: inner_ty, | ||
| refinement: inner_refinement, | ||
| } | ||
| } |
There was a problem hiding this comment.
The deref() method panics when called on a mutable reference (line 1362). This panic condition should be documented in the method's documentation to help callers understand when it's safe to use this method. Consider adding a doc comment that specifies the preconditions and explains why mutable references are not supported.
Uh oh!
There was an error while loading. Please reload this page.