Skip to content

Conversation

@mtshiba
Copy link
Contributor

@mtshiba mtshiba commented Nov 7, 2025

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
Copy link
Contributor

github-actions bot commented Nov 7, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

mypy_primer results

Changes were detected when running on open source projects
pandera (https://github.com/pandera-dev/pandera)
- pandera/api/dataframe/container.py:1273:31: warning[possibly-missing-attribute] Attribute `dtype` may be missing on object of type `Any | None`
- pandera/api/dataframe/container.py:1274:33: warning[possibly-missing-attribute] Attribute `parsers` may be missing on object of type `Any | None`
- pandera/api/dataframe/container.py:1275:32: warning[possibly-missing-attribute] Attribute `checks` may be missing on object of type `Any | None`
- pandera/api/dataframe/container.py:1276:34: warning[possibly-missing-attribute] Attribute `nullable` may be missing on object of type `Any | None`
- pandera/api/dataframe/container.py:1277:32: warning[possibly-missing-attribute] Attribute `unique` may be missing on object of type `Any | None`
- pandera/api/dataframe/container.py:1278:32: warning[possibly-missing-attribute] Attribute `coerce` may be missing on object of type `Any | None`
- pandera/api/dataframe/container.py:1279:30: warning[possibly-missing-attribute] Attribute `name` may be missing on object of type `Any | None`
- Found 1636 diagnostics
+ Found 1629 diagnostics

scikit-build-core (https://github.com/scikit-build/scikit-build-core)
- src/scikit_build_core/build/wheel.py:98:20: error[no-matching-overload] No overload of bound method `__init__` matches arguments
- Found 45 diagnostics
+ Found 44 diagnostics

No memory usage changes detected ✅

@mtshiba mtshiba force-pushed the fix-comprehension-symbol-lookup branch 2 times, most recently from fb1bc92 to 9f904d1 Compare November 7, 2025 16:06
@mtshiba
Copy link
Contributor Author

mtshiba commented Nov 7, 2025

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

@mtshiba mtshiba marked this pull request as ready for review November 7, 2025 16:21
@AlexWaygood AlexWaygood added bug Something isn't working ty Multi-file analysis & type inference labels Nov 7, 2025
@mtshiba
Copy link
Contributor Author

mtshiba commented Nov 9, 2025

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
Copy link
Contributor Author

mtshiba commented Nov 9, 2025

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.

@carljm
Copy link
Contributor

carljm commented Nov 11, 2025

The new diagnostic on the conformance suite looks incorrect. We are now throwing an error on the reference to Private at https://github.com/python/typing/blob/main/conformance/tests/generics_syntax_scoping.py#L74 but we should not.

@carljm
Copy link
Contributor

carljm commented Nov 12, 2025

This is looking great, thanks! Just as a heads-up: I am making some (minor: code organization and naming) changes to this PR and then I will merge it.

@carljm carljm force-pushed the fix-comprehension-symbol-lookup branch from e9d70dd to ab62b0b Compare November 12, 2025 16:14
@carljm carljm merged commit 9dd666d into astral-sh:main Nov 12, 2025
41 checks passed
@mtshiba mtshiba deleted the fix-comprehension-symbol-lookup branch November 13, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants