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] Unpacker: Make invariant explicit and directly return a Type #16018

Merged
merged 2 commits into from
Feb 7, 2025
Merged

[red-knot] Unpacker: Make invariant explicit and directly return a Type #16018

merged 2 commits into from
Feb 7, 2025
+12 −7

Conversation

sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Feb 7, 2025 •
edited
Loading

Summary

  • Do not return Option<Type<…>> from Unpacker::get, but just Type. Panic otherwise.
  • Rename Unpacker::get to Unpacker::expression_type

@sharkdp sharkdp added the red-knot Multi-file analysis & type inference label Feb 7, 2025
@sharkdp sharkdp mentioned this pull request Feb 7, 2025
Merged
@AlexWaygood AlexWaygood requested review from dhruvmanila and removed request for AlexWaygood February 7, 2025 11:20
MichaReiser
MichaReiser approved these changes Feb 7, 2025
View reviewed changes
crates/red_knot_python_semantic/src/types/unpacker.rs Outdated
/// Panics if a scoped expression ID is passed in that does not correspond to a sub-
/// expression of the target.
#[track_caller]
pub(crate) fn type_for(&self, expr_id: ScopedExpressionId) -> Type<'db> {
Copy link
Member

Choose a reason for hiding this comment

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

We call the same method expression_type in TypeInference. I'm somewhat undecided if I'd value the consistency here or if diverging is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's go with expression_type 👍

@sharkdp sharkdp enabled auto-merge (squash) February 7, 2025 11:57
@sharkdp sharkdp merged commit 1f7a29d into main Feb 7, 2025
20 checks passed
@sharkdp sharkdp deleted the david/unpacker-explicit-invariant branch February 7, 2025 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

@carljm carljm Awaiting requested review from carljm carljm is a code owner

@dhruvmanila dhruvmanila Awaiting requested review from dhruvmanila

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.
2 participants
@sharkdp @MichaReiser