-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[red-knot] implement basic call expression inference #13164
[red-knot] implement basic call expression inference #13164
Conversation
This only works for the *result* of call expressions; it performs no inference on parameters.
|
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.
Looks good! Some things we should adjust in the implementation.
#[test] | ||
fn basic_call_expression() -> anyhow::Result<()> { | ||
let mut db = setup_db(); | ||
|
||
db.write_dedented( | ||
"src/a.py", | ||
" | ||
def get_int() -> int: | ||
return 42 | ||
|
||
x = get_int() | ||
", | ||
)?; | ||
|
||
assert_public_ty(&db, "src/a.py", "x", "int"); | ||
|
||
Ok(()) | ||
} |
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.
Fantastic, this is exciting to see happen! Unlocks so much new capability.
- Move inference on the call expression type to `Type::call`, matching what we do for `Type::member`. - Add a diagnostic for the case that the type is in fact not callable. Test it, noting that there is a separate issue around creating the same diagnostic more than once because it is emitted as a side effect of inference. - Leave correct breadcrumbs for future implementation in `Type::call`.
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.
Awesome, thank you!!
Looks like we need to adjust the expected diagnostics in the benchmark. |
CodSpeed Performance ReportMerging #13164 will degrade performances by 14.73%Comparing Summary
Benchmarks breakdown
|
Looks like this CodSpeed regression is real, but it's expected; we are now looking up the return type of every function called (and apparently doing it twice.) Fixing the double inference part should reduce the size of the regression somewhat. |
Now we do not call (or emit diagnostics!) twice for call expressions on the RHS of an assignment. The issue here was not the call expression but the assignment, and had existed for some time, but this makes for a much cleaner landing! Co-authored-by: Carl Meyer <carl@astral.sh>
On closer look, I think a lot of the regression (and why it shows up only in incremental benchmark) is that annotations are deferred in stubs, and now we look up a lot of return types from stubs, which causes us to trigger the |
Summary
Adds basic support for inferring the type resulting from a call expression. This only works for the result of call expressions; it performs no inference on parameters. It also intentionally does nothing with class instantiation,
__call__
implementors, or lambdas.Test Plan
Adds a test that it infers the right thing!