Skip to content

[ty] Preserve short-circuit reachability for direct conditions - #28082

Merged
carljm merged 11 commits into
mainfrom
cjm/ty-direct-condition-reachability
Aug 28, 2026
Merged

[ty] Preserve short-circuit reachability for direct conditions#28082
carljm merged 11 commits into
mainfrom
cjm/ty-direct-condition-reachability

Conversation

@carljm

@carljm carljm commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

ty can incorrectly treat a branch guarded by an impossible compound condition as reachable when an operand has potentially mutable truthiness. For example, if value and False: always skips its body in CPython; the first falsy operand selects the control-flow path immediately, it doesn't fully evaluate value and False and then re-evaluate the truthiness of the result. Saving value and False to a variable and then testing the saved object can instead call value.__bool__ twice and get different answers.

Distinguish expressions consumed directly as conditions from expressions that produce values. Evaluate direct boolean and conditional expressions structurally, and retain the combined truthiness of chained comparisons even when a comparison returns an arbitrary object. Operands with types equivalent to Never cannot produce a result; preserve that distinction from ambiguous truthiness while composing conditions. Value contexts continue to allow repeated truthiness checks to disagree.

To limit retained memory, comparison truthiness uses sparse overrides in optional inference metadata, stored as boxed slices. Ordinary comparison chains use the truthiness of their inferred value type, and root chained-comparison conditions reuse expression inference without a separate condition-analysis memo. Cycle widening preserves overrides when needed to keep the effective truthiness monotonic. Tracking operands that cannot produce a result adds only transient evaluation state.

Prerequisite for #28045, which addresses astral-sh/ty#4380.

Ecosystem results

Five expected diagnostic removals across pwndbg, pytest, and Spark; no additions. Each comes from a branch that is unreachable under the checked platform or declared types. See the Codex ecosystem analysis for details and minimized reproducers.

Test plan

  • Mdtests cover compound conditions, nested boolean operations and not, conditional expressions, and chained comparisons across branches, loops, assertions, comprehension filters, and match guards.
  • Reachability assertions use revealed variable types, including reachable saved results, assignment expressions, nested value expressions, and call arguments whose truthiness can be tested again.
  • Cases with operands narrowed to Never, aliases and unions of aliases of Never, and type variables bounded by Never assert branch reachability using unrelated variables and check precise conditional-expression result types.
  • Additional cases cover unreachable loop assignments and conditional expressions in stringified Annotated metadata.
  • A Rust invariant test covers sparse comparison-truthiness overrides appearing or disappearing across inference-cycle results, including preservation of previous-only overrides.

carljm added 2 commits August 25, 2026 14:00
Evaluate direct conditions separately from saved expression values so mutable
truthiness does not make unreachable branches appear reachable.

Store comparison-chain truthiness only when it differs from the inferred value
type, and read chained-comparison predicates directly from expression inference.
Preserve sparse overrides across result merging and cycle recovery.

Cover short-circuit consumers, saved values, comparison chains, conditional
expression types, quoted annotation metadata, and cyclic inference.

Prerequisite for #28045.
@carljm carljm added the ty Multi-file analysis & type inference label Aug 26, 2026
@astral-sh-bot

astral-sh-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

Typing conformance results

No changes detected ✅

Current numbers
The percentage of diagnostics emitted that were expected errors held steady at 97.79%. The percentage of expected errors that received a diagnostic held steady at 94.33%. The number of fully passing files held steady at 112/136.

@astral-sh-bot

astral-sh-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

Memory usage report

Summary

Project Old New Diff Outcome
prefect 477.78MB 478.16MB +0.08% (386.59kB)
sphinx 172.67MB 172.94MB +0.16% (285.31kB)
trio 96.95MB 96.99MB +0.04% (42.77kB)
flake8 40.22MB 40.23MB +0.03% (10.98kB)

Significant changes

Click to expand detailed breakdown

prefect

Name Old New Diff Outcome
analyze_condition 0.00B 618.12kB +618.12kB (new)
infer_expression_types_impl 24.58MB 24.42MB -0.66% (165.11kB) ⬇️
infer_definition_types 37.10MB 37.04MB -0.15% (57.38kB) ⬇️
infer_statement_types_impl 676.27kB 674.45kB -0.27% (1.81kB) ⬇️
infer_scope_types_impl 19.17MB 19.16MB -0.01% (1.73kB) ⬇️
all_narrowing_constraints_for_expression 4.68MB 4.68MB -0.03% (1.57kB) ⬇️
loop_header_reachability 270.50kB 269.34kB -0.43% (1.16kB) ⬇️
place_by_id 3.75MB 3.75MB -0.03% (992.00B) ⬇️
analyze_non_terminal_call 1.03MB 1.03MB -0.04% (480.00B) ⬇️
member_lookup_with_policy_and_receiver_inner 2.08MB 2.08MB -0.01% (312.00B) ⬇️
member_lookup_with_policy_inner 9.35MB 9.34MB -0.00% (312.00B) ⬇️
infer_deferred_types 4.56MB 4.56MB -0.01% (264.00B) ⬇️
infer_expression_type_impl 266.83kB 266.66kB -0.06% (176.00B) ⬇️
Specialization 5.01MB 5.01MB -0.00% (144.00B) ⬇️
GenericAlias 1.88MB 1.88MB -0.00% (72.00B) ⬇️
... 4 more

sphinx

Name Old New Diff Outcome
analyze_condition 0.00B 426.42kB +426.42kB (new)
infer_expression_types_impl 12.02MB 11.94MB -0.62% (76.88kB) ⬇️
infer_definition_types 11.61MB 11.58MB -0.32% (38.50kB) ⬇️
infer_expression_type_impl 289.49kB 282.27kB -2.49% (7.22kB) ⬇️
member_lookup_with_policy_inner 3.69MB 3.68MB -0.14% (5.34kB) ⬇️
member_lookup_with_policy_and_receiver_inner 1.65MB 1.64MB -0.32% (5.32kB) ⬇️
loop_header_reachability 252.26kB 250.14kB -0.84% (2.12kB) ⬇️
infer_statement_types_impl 529.82kB 528.23kB -0.30% (1.59kB) ⬇️
infer_scope_types_impl 6.13MB 6.13MB -0.02% (1.38kB) ⬇️
UnionType 627.97kB 627.09kB -0.14% (896.00B) ⬇️
all_narrowing_constraints_for_expression 2.39MB 2.39MB -0.03% (784.00B) ⬇️
analyze_non_terminal_call 483.48kB 482.86kB -0.13% (632.00B) ⬇️
StaticClassLiteral<'db>::implicit_attribute_inner_ 766.66kB 766.29kB -0.05% (384.00B) ⬇️
infer_unpack_types 272.11kB 272.06kB -0.02% (48.00B) ⬇️
place_by_id 1.16MB 1.16MB -0.00% (40.00B) ⬇️
... 2 more

trio

Name Old New Diff Outcome
analyze_condition 0.00B 71.60kB +71.60kB (new)
infer_expression_types_impl 6.43MB 6.42MB -0.28% (18.52kB) ⬇️
infer_definition_types 4.73MB 4.72MB -0.11% (5.46kB) ⬇️
place_by_id 624.57kB 622.57kB -0.32% (2.00kB) ⬇️
infer_scope_types_impl 2.83MB 2.83MB -0.02% (472.00B) ⬇️
function_known_decorators 368.97kB 368.52kB -0.12% (464.00B) ⬇️
loop_header_reachability 98.75kB 98.35kB -0.41% (416.00B) ⬇️
all_narrowing_constraints_for_expression 694.47kB 694.19kB -0.04% (288.00B) ⬇️
analyze_non_terminal_call 448.02kB 447.75kB -0.06% (280.00B) ⬇️
member_lookup_with_policy_inner 1.25MB 1.25MB -0.02% (200.00B) ⬇️
infer_expression_type_impl 44.85kB 44.71kB -0.31% (144.00B) ⬇️
infer_deferred_types 1.19MB 1.19MB -0.01% (136.00B) ⬇️
StaticClassLiteral<'db>::implicit_attribute_inner_ 143.66kB 143.56kB -0.07% (104.00B) ⬇️
member_lookup_with_policy_and_receiver_inner 244.08kB 243.98kB -0.04% (96.00B) ⬇️
ClassType<'db>::abstract_methods_ 23.76kB 23.68kB -0.33% (80.00B) ⬇️
... 12 more

flake8

Name Old New Diff Outcome
analyze_condition 0.00B 13.66kB +13.66kB (new)
infer_expression_types_impl 806.56kB 804.79kB -0.22% (1.77kB) ⬇️
infer_definition_types 1.12MB 1.12MB -0.06% (648.00B) ⬇️
infer_statement_types_impl 32.18kB 32.14kB -0.15% (48.00B) ⬇️
infer_scope_types_impl 533.95kB 533.91kB -0.01% (32.00B) ⬇️
analyze_non_terminal_call 34.82kB 34.79kB -0.09% (32.00B) ⬇️
all_narrowing_constraints_for_expression 108.58kB 108.55kB -0.03% (32.00B) ⬇️
member_lookup_with_policy_and_receiver_inner 55.22kB 55.19kB -0.06% (32.00B) ⬇️
member_lookup_with_policy_inner 292.60kB 292.57kB -0.01% (32.00B) ⬇️
infer_expression_type_impl 21.30kB 21.27kB -0.15% (32.00B) ⬇️
loop_header_reachability 9.83kB 9.81kB -0.16% (16.00B) ⬇️
StaticClassLiteral<'db>::implicit_attribute_inner_ 55.53kB 55.52kB -0.03% (16.00B) ⬇️
infer_unpack_types 27.57kB 27.56kB -0.03% (8.00B) ⬇️

@astral-sh-bot

astral-sh-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

ecosystem-analyzer results

Lint rule Added Removed Changed
invalid-assignment 0 2 0
invalid-argument-type 0 1 0
unresolved-import 0 1 0
unsound-assignment 0 1 0
Total 0 5 0

Raw diff:

pwndbg (https://github.com/pwndbg/pwndbg)
- pwndbg/aglib/disasm/assistant.py:390:24 error[invalid-assignment] Object of type `None` is not assignable to `Emulator`
- pwndbg/aglib/disasm/assistant.py:391:19 error[invalid-assignment] Object of type `None` is not assignable to `Emulator`

pytest (https://github.com/pytest-dev/pytest)
- src/_pytest/_io/terminalwriter.py:80:24 error[unresolved-import] Cannot resolve imported module `colorama`
- src/_pytest/_io/terminalwriter.py:84:24 warning[unsound-assignment] Unsound assignment: `Unknown` is not a subtype of `TextIO | None`

spark (https://github.com/apache/spark)
- python/pyspark/streaming/context.py:389:42 error[invalid-argument-type] Argument to bound method `SparkContext.parallelize` is incorrect: Expected `Iterable[Unknown]`, found `RDD[T@queueStream]`

Full report with detailed diff (timing results)

@carljm
carljm requested a review from AlexWaygood August 26, 2026 21:32
@codspeed-hq

codspeed-hq Bot commented Aug 26, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 126 untouched benchmarks
⏩ 84 skipped benchmarks1


Comparing cjm/ty-direct-condition-reachability (f387002) with main (ce3849b)

Open in CodSpeed

Footnotes

  1. 84 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.

@carljm
carljm marked this pull request as ready for review August 27, 2026 04:16
@carljm
carljm requested review from a team as code owners August 27, 2026 04:16
@carljm

carljm commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author
Codex ecosystem analysis

PR #28082 ecosystem summary

The five removed diagnostics come from branches made unreachable by platform checks or declared types. Each removal is consistent with evaluating a short-circuit expression directly as a condition.

Affected projects

1. pwndbg's declared return type makes an emulation-failure branch unreachable

Report entries:

The merge base reports two error[invalid-assignment] diagnostics: “Object of type None is not assignable to Emulator”. Both assignments are guarded by jump_emu and None in jump_emu.single_step(...). Since single_step is annotated to return tuple[int, int], its membership test is always false under the declared types. The PR recognizes that the condition cannot enter the body and removes both diagnostics.

This follows the annotations, which do not fully describe runtime failure handling: single_step can construct InstructionExecutedResult(None, None) even though both fields are annotated int. The existing constructor diagnostics remain unchanged.

Both assignments reduce to the same behavior:

def f(value: int):
    if value and False:
        # Merge base: error[invalid-assignment] Object of type `None` is not assignable to `int`
        # PR: no diagnostic
        value = None

2. pytest's Windows-only terminal setup is unreachable on Linux

Report entries:

pytest guards its optional colorama setup with hasattr(file, "isatty") and file.isatty() and sys.platform == "win32". The sys.stdout annotation includes Any, so the preceding isatty() call can be dynamically typed, but the body is still unreachable on Linux. The merge base reports error[unresolved-import] (“Cannot resolve imported module colorama”) and warning[unsound-assignment] (“Unsound assignment: Unknown is not a subtype of TextIO | None”). The PR correctly removes both diagnostics.

An operand with unknown truthiness cannot make if value and False enter its body. The two diagnostics reduce to this example, where missing is an absent module:

def write(stdout):
    if stdout and False:
        # Merge base: error[unresolved-import] Cannot resolve imported module `missing`
        # PR: no diagnostic
        import missing
        # Merge base: warning[unsound-assignment] Unsound assignment: `Unknown` is not a subtype of `int`
        # PR: no diagnostic
        file: int = missing

3. Spark's declared element type makes a conversion branch unreachable

Report entries:

The merge base reports error[invalid-argument-type]: “Argument to bound method SparkContext.parallelize is incorrect: Expected Iterable[Unknown], found RDD[T@queueStream]”. The PR correctly removes this diagnostic. queueStream declares rdds as list[RDD[T]], so its conversion branch, guarded by rdds and not isinstance(rdds[0], RDD), is unreachable under the declared types: an empty list short-circuits, and a nonempty list contains an RDD at index zero.

A final false operand prevents an and condition from taking the truthy branch. The merge base still checks the guarded call below; the PR recognizes it as unreachable.

def takes_str(x: str):
    ...

def f(x: int):
    if x and False:
        # Merge base: error[invalid-argument-type] Argument to function `takes_str` is incorrect: Expected `str`, found `int & ~AlwaysFalsy`
        # PR: no diagnostic
        takes_str(x)

Reproduction

  • Detailed report: ecosystem-analyzer report
  • Actions run: run 33024431513, attempt 1
  • Ruff comparison: b52fe1ba25 to 591d3719e3
  • ecosystem-analyzer: 74af2b661a
  • mypy-primer: 3058720299
  • Dependency cutoff: 2026-08-26T23:43:08Z
  • Project Python: pwndbg, pytest, and Spark use Python 3.11.
  • Project analysis mode: pwndbg and pytest are non-strict; Spark is strict.
  • Comparison method: both exact-revision profiling binaries check each pinned project with its Python environment, --python-platform linux, and the PR's .github/ty-ecosystem.toml as user-level configuration. Spark additionally uses --config analysis.strict-equality-semantics=true --config analysis.strict-generic-narrowing=true. The standalone examples use the same configuration and target Python 3.11.

Comment thread crates/ty_python_core/src/builder.rs Outdated
Comment thread crates/ty_python_core/src/builder.rs Outdated
Comment thread crates/ty_python_semantic/src/types/infer/builder.rs Outdated
Comment thread crates/ty_python_semantic/src/types/infer/builder.rs
Comment thread crates/ty_python_semantic/src/types/infer/builder.rs Outdated
Comment thread crates/ty_python_semantic/src/types/infer/builder.rs Outdated
Comment thread crates/ty_python_semantic/src/types/infer/builder.rs Outdated
Comment thread crates/ty_python_semantic/src/types/infer/builder.rs Outdated
Comment thread crates/ty_python_semantic/src/types/infer/builder.rs Outdated
Comment thread crates/ty_python_semantic/src/types/infer/builder.rs
Comment thread crates/ty_python_semantic/src/types/infer.rs Outdated
Comment thread crates/ty_python_semantic/src/reachability.rs Outdated
Comment thread crates/ty_python_semantic/src/reachability.rs
@AlexWaygood

Copy link
Copy Markdown
Member

(I speak through Carl today, we reviewed this synchronously together)

@carljm
carljm merged commit 1776127 into main Aug 28, 2026
67 checks passed
@carljm
carljm deleted the cjm/ty-direct-condition-reachability branch August 28, 2026 00:03

/// Like [`Truthiness::and`], but evaluates `other` only when `self` may be true.
#[must_use]
pub fn and_else(self, other: impl FnOnce() -> Self) -> Self {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the normal convention would be to call this .and_then(), similar to how Option has both .or_else() which only calls the closure if self is None and .and_then() which only does work when self is Some

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants