-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rust: Restrict type propagation into arguments #20683
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
9097870 to
a5e8fdb
Compare
e05b287 to
fb8aabc
Compare
fb8aabc to
2d5aecf
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 introduces context-based typing for Rust function calls to improve type inference accuracy and reduce combinatorial explosions. The changes implement a mechanism to track when function return types depend on context (like Default::default() or Vec::new()) rather than their arguments.
Key Changes
- Introduced a new
TContextTypetype to mark context-typed expressions - Added
ContextTypingmodule to identify and handle context-typed calls - Modified type inference to prevent type information from flowing into arguments unless they are context-typed
- Updated test expectations to reflect improved type inference results
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
rust/ql/lib/codeql/rust/internal/Type.qll |
Added new TContextType and ContextType class for context-based typing |
rust/ql/lib/codeql/rust/internal/TypeInference.qll |
Implemented ContextTyping module and integrated context checking into method and non-method call type inference |
rust/ql/lib/codeql/rust/internal/typeinference/FunctionType.qll |
Filtered out TContextType in argument substitution |
rust/ql/lib/codeql/rust/internal/typeinference/BlanketImplementation.qll |
Filtered out TContextType in blanket implementation type resolution |
rust/ql/test/library-tests/type-inference/type-inference.ql |
Added import and filter to exclude TContextType from test output |
rust/ql/test/library-tests/type-inference/main.rs |
Added test cases for context-typed calls with trait implementations |
Various *.expected files |
Updated expected test results reflecting improved type inference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I need to better understand how this relates to the existing certain type machinery and what we want to do going forward. There clearly is some overlap between the two things. With certain types we can find places where we need to do speculative inference with: And with context types we can find places where we need to do speculative inference with: In the former case we're marking places with certainty and in the latter we're marking places with uncertainty. In the former case, too few marking leads to too many types (over approximation) and in the latter case too few markings leads to too few types (under approximation). It seems that certain type inference can block spurious types in ways that context types cannot: struct MyA {}
struct MyB {}
impl Default for MyA { fn default() -> Self { MyA {} } }
impl Deref for MyA {
type Target = MyB;
fn deref(&self) -> &Self::Target { &MyB {} }
}
fn take_a(_b: &MyA) {}
fn take_b(_b: &MyB) {}
fn test() {
let x: MyA = Default::default(); // Certain types prevent `x : MyB`
take_a(&x);
take_b(&x);
let y = Default::default(); // Context types can not prevent `x : MyB`
take_a(&y);
take_b(&y);
}So maybe we could instead expand on the existing certain type inference? Or are there things that context type can prevent that certain types cannot? Having an example of such a thing as a test would be great. We should also infer context types for:
|
Anything involving method calls will be difficult to support, since we cannot monotonically determine when a method call has a unique target and hence has a unique return type. |
Right now
Yes, your are right, I forgot about those. |
fca1614 to
d43133f
Compare
f7e5281 to
cdf6c86
Compare
cdf6c86 to
bd8665a
Compare
Type propagation into arguments is needed in cases like
Up until now, we have unconditionally propagated types into all arguments, which can sometimes lead to explosions in inferred types.
This PR circumvents such explosions by only propagating type information into arguments when those arguments are potentially context-typed (such as
Vec::newandDefault::default).DCA looks great: we almost get the
Nodes With Type At Length Limitcount down to 0, sacrificing only a relatively small amount of resolvable calls.