-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Improve type inference for closures and function traits #21170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
0ca8419 to
dabc5d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR improves type inference for Rust closures by changing them from dyn FnOnce to dyn Fn, and adds support for type mentions using Fn(..) -> .. and FnMut(..) -> .. traits. This allows closures to be correctly used where Fn and FnMut traits are expected, and enables proper type inference when these traits are used in type parameter bounds.
Changes:
- Closures now have the type
dyn Fninstead ofdyn FnOnce, enabling them to satisfyFnandFnMuttrait bounds - Added support for
FnandFnMuttraits with proper type parameter handling - Updated type mention handling to work with all three function traits (
Fn,FnMut,FnOnce)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/tools/builtins/mentions.rs | Changed builtin mention from FnOnce to Fn to ensure the dyn Fn type is always available |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updated test expectations to reflect new type inference behavior with dyn Fn instead of dyn FnOnce |
| rust/ql/test/library-tests/type-inference/closure.rs | Added comprehensive test cases for FnMut and Fn trait bounds |
| rust/ql/lib/codeql/rust/internal/typeinference/TypeMention.qll | Updated type mention logic to handle all function traits generically using AnyFnTrait |
| rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll | Changed closure root type from dyn FnOnce to dyn Fn with updated type path handling |
| rust/ql/lib/codeql/rust/frameworks/stdlib/Stdlib.qll | Added AnyFnTrait, FnMutTrait, and FnTrait classes; refactored FnOnceTrait to extend AnyFnTrait |
| rust/ql/lib/change-notes/2026-01-16-type-inference-closures.md | Added change note documenting the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result = TDynTraitType(any(FnTrait t)) // always exists because of the mention in `builtins/mentions.rs` | ||
| } | ||
|
|
||
| /** Gets the path to a closure's return type. */ |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment for closureReturnPath() should clarify why it references FnTrait for the trait parameter but FnOnceTrait for the output type. This mixing of trait references is subtle and deserves explanation, as it relies on the fact that Fn is a subtrait of FnOnce and inherits the Output associated type.
| /** Gets the path to a closure's return type. */ | |
| /** | |
| * Gets the path to a closure's return type. | |
| * | |
| * Closures are modeled as `dyn Fn` trait object types (see `closureRootType`), | |
| * so the trait parameter here is an `FnTrait`. However, we obtain the return | |
| * type via the `Output` associated type of `FnOnceTrait`. This works because | |
| * `Fn` is a subtrait of `FnOnce` in Rust and inherits its `Output` associated | |
| * type, so using `FnOnceTrait` to access `Output` is valid even when the | |
| * underlying trait object is `FnTrait`. | |
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I think this is pretty clear if one understands how TDynTraitTypeParameter works.
rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll
Outdated
Show resolved
Hide resolved
| * [1]: https://doc.rust-lang.org/std/ops/trait.FnOnce.html | ||
| */ | ||
| class FnOnceTrait extends Trait { | ||
| class FnOnceTrait extends AnyFnTrait { |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FnOnceTrait class still has its own getOutputType() method while AnyFnTrait doesn't. Consider documenting why AnyFnTrait doesn't include getOutputType() when all three function traits have an Output associated type. This would clarify that only FnOnce directly defines Output, while Fn and FnMut inherit it.
Co-authored-by: Copilot <[email protected]>
This PR
dyn Fninstead ofdyn FnOnce. This means that closures can now be passed to places where the traitFnMutandFnis used.FnMut(..) -> ..andFn(..) -> ..work.Both of these changes rely on #21165 since
FnMutandFnuses associated types from their supertraitFnOnce.The DCA report seems fine, though things might've become a tiny bit slower.