[ty] fix global symbol lookup from eager scopes #21317

mtshiba · 2025-11-07T15:37:52Z

Summary

cf. #20962

In the following code, foo in the comprehension was not reported as unresolved:

# error: [unresolved-reference] "Name `foo` used when not defined"
foo
foo = [
    # no error!
    # revealed: Divergent
    reveal_type(x) for _ in () for x in [foo]
]

baz = [
    # error: [unresolved-reference] "Name `baz` used when not defined"
    # revealed: Unknown
    reveal_type(x) for _ in () for x in [baz]
]

In fact, this is a more serious bug than it looks: for foo, explicit_global_symbol is called, causing a symbol that should actually be Undefined to be reported as being of type Divergent.

This PR fixes this bug. As a result, the code in mdtest/regression/pr_20962_comprehension_panics.md no longer panics.

Test Plan

corpus\cyclic_symbol_in_comprehension.py is added.
New tests are added in mdtest/comprehensions/basic.md.

github-actions · 2025-11-07T15:41:00Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

github-actions · 2025-11-07T15:41:21Z

`mypy_primer` results

No ecosystem changes detected ✅

Memory usage changes were detected when running on open source projects

trio (https://github.com/python-trio/trio)
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`

sphinx (https://github.com/sphinx-doc/sphinx)
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
+ WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`

prefect (https://github.com/PrefectHQ/prefect)
+ WARN expected `heap_size` to be provided by Salsa query `ClassLiteral < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`
- WARN expected `heap_size` to be provided by Salsa query `GenericAlias < 'db >::variance_of_`

mtshiba · 2025-11-07T16:19:58Z

ty_server::e2e pull_diagnostics::document_diagnostic_caching_changed seems to hang non-deterministically.

https://github.com/astral-sh/ruff/actions/runs/19173434027/job/54811898939?pr=21317

carljm · 2025-11-07T21:16:58Z

crates/ty_python_semantic/src/types/infer/builder.rs

                                ));
+                                // Reaching here means that no bindings are found in any scope.
+                                // Since `explicit_global_symbol` may return a cycle initial value, we return `Place::Undefined` here.
+                                return Place::Undefined.into();


If returning here is correct, then pushing the constraint into constraint_keys immediately above this cannot have any effect, so should we remove that as well? (Removing it doesn't seem to fail any test.)

Why do we hit the FoundConstraint case in this situation in the first place? It's not clear to me where the "constraint" comes from.

Since place includes member as well as symbol, narrowing constraints must still be tracked even if there are no bindings, for example:

from typing import Literal class Foo: bar: Literal[1, 2] foo = Foo() if foo.bar == 1: # `foo.bar` has no explicit binding, but the constraint is `foo.bar == 1` [reveal_type(foo.bar) for _ in ()] # Literal[1]

But as you pointed out, there was no need to add the constraint to constraint_keys here, because we already added it when iterating over ancestor_scopes above. Fixed.

A more fundamental issue is that the global scope should be excluded from the scopes of enclosing scope lookup.
See the comments below.

mtshiba · 2025-11-09T08:01:51Z

I found another example of a too-many-cycle-iteration panic.

if C:
    class C[_T](C): ...

The panic occurred because C in the base class list was being resolved circularly.
It should evaluate to Undefined rather than throwing a cyclic-class-definition error.

mtshiba · 2025-11-09T08:08:28Z

I don't have time right now (I originally found this bug while working on #20566), but I think the logic in infer_place_load is clearly too complicated and could be a breeding ground for bugs like this one.
I think it should be refactored at some point.

MichaReiser · 2025-11-10T08:36:55Z

crates/ty_python_semantic/resources/corpus/cyclic_symbol_in_comprehension.py

@@ -1,13 +1,7 @@
-# Documentation of two fuzzer panics involving comprehensions
+# Regression test for https://github.com/astral-sh/ruff/pull/20962


Can you say more about why you converted this file to a corpus test? It's not clear to me why that would be preferred or required.

The previous test asserted that we panicked; the new test asserts that the snippet does not cause us to panic

I guess it could have stayed as an mdtest, but corpus tests are what we usually use when all we care about is checking that we didn't panic; it makes sense to me

MichaReiser · 2025-11-10T08:38:05Z

crates/ty_python_semantic/src/types/infer/builder.rs

            for (enclosing_scope_file_id, _) in self.index.ancestor_scopes(file_scope_id).skip(1) {
+                // If the current enclosing scope is global, no place lookup is performed here,
+                // instead falling back to the module's explicit global lookup below.
+                if enclosing_scope_file_id == FileScopeId::global() {


Nit

Suggested change

if enclosing_scope_file_id == FileScopeId::global() {

if enclosing_scope_file_id.is_global() {

mtshiba force-pushed the fix-comprehension-symbol-lookup branch from 7cc7ce1 to fb1bc92 Compare November 7, 2025 15:44

[ty] fix a bug with global symbol lookup from eager scopes

9f904d1

mtshiba force-pushed the fix-comprehension-symbol-lookup branch from fb1bc92 to 9f904d1 Compare November 7, 2025 16:06

mtshiba marked this pull request as ready for review November 7, 2025 16:21

mtshiba requested review from AlexWaygood, carljm, dcreager and sharkdp as code owners November 7, 2025 16:21

AlexWaygood added bug Something isn't working ty Multi-file analysis & type inference labels Nov 7, 2025

carljm reviewed Nov 7, 2025

View reviewed changes

mtshiba added 2 commits November 8, 2025 19:58

Update builder.rs

877b833

Don't lookup global places in enclosing scope lookup

0030db2

mtshiba added 2 commits November 9, 2025 17:20

Update builder.rs

113a3b4

fix panics in nested scope cases

39fb865

MichaReiser reviewed Nov 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ty] fix global symbol lookup from eager scopes #21317

[ty] fix global symbol lookup from eager scopes #21317

mtshiba commented Nov 7, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 7, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 7, 2025 •

edited

Loading

Uh oh!

mtshiba commented Nov 7, 2025

Uh oh!

carljm Nov 7, 2025

Uh oh!

mtshiba Nov 8, 2025

Uh oh!

mtshiba Nov 9, 2025

Uh oh!

mtshiba commented Nov 9, 2025

Uh oh!

mtshiba commented Nov 9, 2025

Uh oh!

MichaReiser Nov 10, 2025 •

edited

Loading

Uh oh!

AlexWaygood Nov 10, 2025

Uh oh!

AlexWaygood Nov 10, 2025

Uh oh!

MichaReiser Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -1,13 +1,7 @@
		# Documentation of two fuzzer panics involving comprehensions
		# Regression test for https://github.com/astral-sh/ruff/pull/20962

	if enclosing_scope_file_id == FileScopeId::global() {
	if enclosing_scope_file_id.is_global() {

[ty] fix global symbol lookup from eager scopes #21317

Are you sure you want to change the base?

[ty] fix global symbol lookup from eager scopes #21317

Conversation

mtshiba commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

github-actions bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

mtshiba commented Nov 7, 2025

Uh oh!

carljm Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

mtshiba Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

mtshiba Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

mtshiba commented Nov 9, 2025

Uh oh!

mtshiba commented Nov 9, 2025

Uh oh!

MichaReiser Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mtshiba commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading

`mypy_primer` results

MichaReiser Nov 10, 2025 •

edited

Loading