Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[red-knot] add call checking #15200

Merged
merged 24 commits into from
Jan 7, 2025
Merged

[red-knot] add call checking #15200

merged 24 commits into from
Jan 7, 2025

Conversation

carljm
Copy link
Contributor

@carljm carljm commented Dec 30, 2024 •
edited
Loading

Summary

This implements checking of calls.

I ended up following Micha's original suggestion from back when the signature representation was first introduced, and flattening it to a single array of parameters. This turned out to be easier to manage, because we can represent parameters using indices into that array, and represent the bound argument types as an array of the same length.

Starred and double-starred arguments are still TODO; these won't be very useful until we have generics.

The handling of diagnostics is just hacked into return_ty_result, which was already inconsistent about whether it emitted diagnostics or not; now it's even more inconsistent. This needs to be addressed, but could be a follow-up.

The new benchmark errors here surface the need for intersection support in is_assignable_to.

Fixes #14161.

Test Plan

Added mdtests.

@carljm carljm added the red-knot Multi-file analysis & type inference label Dec 30, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 30, 2024 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood mentioned this pull request Jan 3, 2025
Merged
7 tasks
@carljm carljm marked this pull request as ready for review January 6, 2025 17:11
AlexWaygood
AlexWaygood reviewed Jan 6, 2025
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haven't finished reviewing (or reading the diff), but here's what I have so far

crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/arguments.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/bind.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/arguments.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/bind.rs Outdated Show resolved Hide resolved
@carljm
Copy link
Contributor Author

carljm commented Jan 7, 2025

Thanks for the reviews! I think I've addressed all comments so far.

MichaReiser
MichaReiser approved these changes Jan 7, 2025
View reviewed changes
crates/red_knot_python_semantic/src/types/call/arguments.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/signatures.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/signatures.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/signatures.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/signatures.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/infer.rs Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/bind.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/bind.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/bind.rs Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/bind.rs Outdated Show resolved Hide resolved
AlexWaygood
AlexWaygood reviewed Jan 7, 2025
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Still not a full review)

crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/resources/mdtest/call/function.md Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/call/bind.rs Outdated Show resolved Hide resolved
@carljm
Copy link
Contributor Author

carljm commented Jan 7, 2025

Thanks for all the great review comments! I think I've addressed them all, planning to land if CI is green.

@carljm carljm enabled auto-merge (squash) January 7, 2025 20:35
@carljm carljm merged commit f2a86fc into main Jan 7, 2025
20 checks passed
@carljm carljm deleted the cjm/checksigs branch January 7, 2025 20:39
dcreager added a commit that referenced this pull request Jan 8, 2025
@dcreager
Merge branch 'main' into dcreager/thread-safe-panics
6bc5d8b 
* main:
  [`pylint`] Fix `unreachable` infinite loop (`PLW0101`) (#15278)
  fix invalid syntax in workflow file (#15357)
  [`pycodestyle`] Avoid false positives related to type aliases (`E252`) (#15356)
  [`flake8-builtins`] Disapply `A005` to stub files (#15350)
  Improve logging system using `logLevel`, avoid trace value (#15232)
  [`flake8-builtins`] Rename `A005` and improve its error message (#15348)
  Spruce up docs for pydoclint rules (#15325)
  [`flake8-type-checking`] Apply `TC008` more eagerly in `TYPE_CHECKING` blocks and disapply it in stubs (#15180)
  [red-knot] `knot_extensions` Python API (#15103)
  Display Union of Literals as a Literal (#14993)
  [red-knot] all types are assignable to object (#15332)
  [`ruff`] Parenthesize arguments to `int` when removing `int` would change semantics in `unnecessary-cast-to-int` (`RUF046`) (#15277)
  [`eradicate`] Correctly handle metadata blocks directly followed by normal blocks (`ERA001`) (#15330)
  Narrowing for class patterns in match statements (#15223)
  [red-knot] add call checking (#15200)
  Spruce up docs for `slice-to-remove-prefix-or-suffix` (`FURB188`) (#15328)
  [`internal`] Return statements in finally block point to end block for `unreachable` (`PLW0101`) (#15276)
  [`ruff`] Treat `)` as a regex metacharacter (`RUF043`, `RUF055`) (#15318)
  Use uv consistently throughout the documentation (#15302)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments

@sharkdp sharkdp sharkdp left review comments

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees
No one assigned
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[red-knot] check call arguments
4 participants
@carljm @MichaReiser @sharkdp @AlexWaygood