[ty] prevent "tainted" unions in cycle normalization#23563
Merged
carljm merged 4 commits intoastral-sh:mainfrom Mar 10, 2026
Merged
[ty] prevent "tainted" unions in cycle normalization#23563carljm merged 4 commits intoastral-sh:mainfrom
carljm merged 4 commits intoastral-sh:mainfrom
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 85.05%. The percentage of expected errors that received a diagnostic held steady at 78.05%. The number of fully passing files held steady at 63/132. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownflake8
trio
sphinx
prefect
|
|
Merging this PR will improve performance by 8.29%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | hydra-zen |
1.3 s | 1.2 s | +8.29% |
Comparing mtshiba:improve-cycle-normalized (f5f3564) with main (ab37a05)
Footnotes
-
30 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
1 | 0 | 39 |
invalid-await |
0 | 40 | 0 |
unsupported-operator |
6 | 0 | 13 |
unresolved-attribute |
2 | 3 | 13 |
invalid-return-type |
0 | 1 | 9 |
type-assertion-failure |
0 | 0 | 10 |
invalid-assignment |
0 | 0 | 3 |
not-iterable |
0 | 0 | 1 |
| Total | 9 | 44 | 88 |
mtshiba
pushed a commit
to mtshiba/ruff
that referenced
this pull request
Mar 6, 2026
…m PR astral-sh#23563) Apply the cycle_normalized simplification from astral-sh#23563: - Replace complex match logic with iteration-count check: if iteration <= 1, return self directly; otherwise union previous and self - Remove as_divergent helper (no longer used) - Remove divergent poisoning logic in narrow.rs - Update test expectations for improved type deduplication and ordering This simplification fixes a duplicate list[Divergent] issue and improves union element ordering, but does not fully resolve the sympy non-determinism. https://claude.ai/code/session_01ASx43ig9Tdi2JYbGkiAurY
b1d9ada to
cc269b5
Compare
Type::cycle_normalized
carljm
approved these changes
Mar 10, 2026
Contributor
carljm
left a comment
There was a problem hiding this comment.
Very nice! Makes a lot of sense. I do wonder if there could be more complex cycles where we want to skip unioning for more than two iterations -- but it's easy to bump the iteration count limit here if we need to. And as long as we start unioning at some point, it should still serve the purpose of avoiding indefinite oscillations.
Member
One thing we always need to keep in mind is that the iteration count is global for the entire cycle and not the cycle participant. Meaning, cyclic queries only conditionally participating in a larger outer cycle (e.g. only in later iterations) may never hit the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
There are some issues with our current cycle handling, particularly with our implementation of
Type::cycle_normalized.To prevent values from oscillating endlessly within a fixed-point iteration,
cycle_normalizedunions the type of the previous iteration with the current type. This ensures monotonicity of the type inference calculation.However, we've observed that always applying this union can sometimes produce undesirable results. Therefore, we make some exceptions where this union isn't applied (see #21909, #21910).
However, these prescriptions are likely not exhaustive, and #22794 also confirmed the occurrence of "divergent pollution" when narrowing. The core issue in #22794 is that
Divergentcan inhibit narrowing. The result type itself does not containDivergent, butDivergentindirectly affects it.This suggests that determining whether a type contains
Divergentis not a perfect indicator of "taintedness".After carefully examining these cases, I found that the problematic "low-precision types" only appear in the first few iterations:
Divergentitself and the type of the next iteration constructed by referencing it. Therefore, we shouldn't union these few iterations. I found that this simple change can remove all of the exceptional handling forDivergentintroduced in #21909, #21910, and #22794.I also found that this change removes the too-many-cycle panic blocking #22633.
Test Plan
mdtest updated