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.

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] fix control flow for assignment expressions in elif tests #15274

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

carljm
Copy link
Contributor

@carljm carljm commented Jan 5, 2025

Summary

The test expression in an elif clause is evaluated whether or not we take the branch. Our control flow model for if/elif chains failed to reflect this, causing wrong inference in cases where an assignment expression occurs inside an elif test expression. Our "no branch taken yet" snapshot (which is the starting state for every new elif branch) can't simply be the pre-if state, it must be updated after visiting each test expression.

Once we do this, it also means we no longer need to track a vector of narrowing constraints to reapply for each new branch, since our "branch not taken" state (which is the initial state for each branch) is continuously updated to include the negative narrowing constraints of all previous branches.

Fixes #15033.

Test Plan

Added mdtests.

@carljm carljm marked this pull request as draft January 5, 2025 17:59
@AlexWaygood AlexWaygood added the red-knot Multi-file analysis & type inference label Jan 5, 2025
Copy link
Contributor

github-actions bot commented Jan 5, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@carljm carljm marked this pull request as ready for review January 5, 2025 18:16
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.

Nice!

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@carljm carljm enabled auto-merge (squash) January 5, 2025 18:33
@carljm carljm merged commit 2ea6362 into main Jan 5, 2025
20 checks passed
@carljm carljm deleted the cjm/ifelif branch January 5, 2025 18:35
dcreager added a commit that referenced this pull request Jan 6, 2025
* main: (60 commits)
  [`ruff`] Dataclass enums (`RUF049`) (#15299)
  Better error message when `--config` is given a table key and a non-inline-table value (#15266)
  Update pre-commit dependencies (#15289)
  Don't fix in ecosystem check (#15267)
  Update Rust crate itertools to 0.14.0 (#15287)
  Remove accidental empty block at the bottom of `split-static-string (SIM905)` doc (#15290)
  Update Rust crate clearscreen to v4 (#15288)
  Update Rust crate insta to v1.42.0 (#15286)
  Update NPM Development dependencies (#15285)
  Update dependency uuid to v11.0.4 (#15284)
  Update dependency ruff to v0.8.6 (#15283)
  Update Rust crate syn to v2.0.95 (#15282)
  Update Rust crate matchit to v0.8.6 (#15281)
  Update Rust crate bstr to v1.11.3 (#15280)
  [red-knot] Future-proof `Type::is_disjoint_from()` (#15262)
  [red-knot] Improve `Type::is_disjoint_from()` for `KnownInstanceType`s (#15261)
  [red-knot] Minor simplifications and improvements to constraint narrowing logic (#15270)
  Allow assigning ellipsis literal as parameter default value (#14982)
  [red-knot] fix control flow for assignment expressions in elif tests (#15274)
  [`refurb`] Mark fix as unsafe when the right-hand side is a string (`FURB171`) (#15273)
  ...
dcreager added a commit that referenced this pull request Jan 6, 2025
* main: (29 commits)
  [`ruff`] Dataclass enums (`RUF049`) (#15299)
  Better error message when `--config` is given a table key and a non-inline-table value (#15266)
  Update pre-commit dependencies (#15289)
  Don't fix in ecosystem check (#15267)
  Update Rust crate itertools to 0.14.0 (#15287)
  Remove accidental empty block at the bottom of `split-static-string (SIM905)` doc (#15290)
  Update Rust crate clearscreen to v4 (#15288)
  Update Rust crate insta to v1.42.0 (#15286)
  Update NPM Development dependencies (#15285)
  Update dependency uuid to v11.0.4 (#15284)
  Update dependency ruff to v0.8.6 (#15283)
  Update Rust crate syn to v2.0.95 (#15282)
  Update Rust crate matchit to v0.8.6 (#15281)
  Update Rust crate bstr to v1.11.3 (#15280)
  [red-knot] Future-proof `Type::is_disjoint_from()` (#15262)
  [red-knot] Improve `Type::is_disjoint_from()` for `KnownInstanceType`s (#15261)
  [red-knot] Minor simplifications and improvements to constraint narrowing logic (#15270)
  Allow assigning ellipsis literal as parameter default value (#14982)
  [red-knot] fix control flow for assignment expressions in elif tests (#15274)
  [`refurb`] Mark fix as unsafe when the right-hand side is a string (`FURB171`) (#15273)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[red-knot] Incorrect control flow modelling in if-elif chains
2 participants