Conversation

tyralla

Fixes #14987

I was puzzled as to why my previous attempts to avoid false unreachable warnings for loops failed for issue #14987. After some debugging, I realised that the underlying problem is that type narrowing does not work with promotions if both the declared type and the constraining type are unions:

x: float | None
y: int | None
x = y
reveal_type(x)  #  None !!!

The fix seems straightforward (but let's see what the Mypy primer says) and is checked by the test cases testNarrowPromotionsInsideUnions1 and testNarrowPromotionsInsideUnions2.

@github-actionsGitHub Actions

This comment has been minimized.

@tyralla

The test suite currently fails due to a previous merge conflict that will be resolved by #19118.

The Mypy primer differences are only due to different union item orders.

@ilevkivskyi: Would you like to review this simple change? It is a small extension of one of your code sections.

@tyrallatyralla requested a review from ilevkivskyi June 6, 2025 22:52
@github-actionsGitHub Actions

This comment has been minimized.

mypy/meet.py Outdated
narrow_declared_type(x, narrowed)
for x in declared.relevant_items()
narrow_declared_type(d, n)
for d, n in product(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like the correct way of doing that, despite higher comp complexity. What's more mypyc-friendly - itertools.product or a plain listcomp with two for clauses?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like plain listcomp would be faster (we're consuming all the items anyways), but I would be surprised if this is a hot path because there's an is_subtype check -- surely that's somewhat slow!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the feedback!

I have no experience with Mypyc, so I can only guess, and would agree with @A5rocks that the possible overhead of itertools.product (if it exists) should be negligible compared to the cost of the additional calls to narrow_declared_type, is_overlapping_types, and is_subtype.

Copy link
Collaborator

Choose a reason for hiding this comment

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

itertools.product may not be a significant bottleneck here, but I'd like to avoid it so that new code is less likely to follow the example and add usages in locations where it will be a bottleneck. Also it might slow things down measurably in very simple cases at least, e.g. union of 2 items. I'd prefer to write this using list comprehensions for these reasons.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I removed itertools.product. The second-best readable solution that came to mind. No list comprehensions; I hope that's fine.

mypy/meet.py Outdated
narrow_declared_type(x, narrowed)
for x in declared.relevant_items()
narrow_declared_type(d, n)
for d, n in product(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like plain listcomp would be faster (we're consuming all the items anyways), but I would be surprised if this is a hot path because there's an is_subtype check -- surely that's somewhat slow!

@github-actionsGitHub Actions

Diff from mypy_primer, showing the effect of this PR on open source code:

ibis (https://.com/ibis-project/ibis)
- ibis/backends/mssql/__init__.py:691: error: Item "Mapping[str, str | DataType]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/mssql/__init__.py:691: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/mssql/__init__.py:738: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]"; expected "Schema"  [arg-type]
+ ibis/backends/mssql/__init__.py:691: error: Item "Mapping[str, str | DataType]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/mssql/__init__.py:691: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/mssql/__init__.py:738: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any"; expected "Schema"  [arg-type]
- ibis/backends/databricks/__init__.py:215: error: Item "Mapping[str, str | DataType]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | None" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/databricks/__init__.py:215: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | None" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/databricks/__init__.py:215: error: Item "Mapping[str, str | DataType]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any | None" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/databricks/__init__.py:215: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any | None" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/databricks/__init__.py:215: error: Item "None" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | None" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/databricks/__init__.py:215: error: Item "None" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any | None" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/risingwave/__init__.py:564: error: Item "Mapping[str, str | DataType]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/risingwave/__init__.py:564: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/risingwave/__init__.py:602: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]"; expected "Schema"  [arg-type]
+ ibis/backends/risingwave/__init__.py:564: error: Item "Mapping[str, str | DataType]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/risingwave/__init__.py:564: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/risingwave/__init__.py:602: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any"; expected "Schema"  [arg-type]
- ibis/backends/postgres/__init__.py:646: error: Item "Mapping[str, str | DataType]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/postgres/__init__.py:646: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/postgres/__init__.py:678: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]"; expected "Schema"  [arg-type]
+ ibis/backends/postgres/__init__.py:646: error: Item "Mapping[str, str | DataType]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/postgres/__init__.py:646: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/postgres/__init__.py:678: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any"; expected "Schema"  [arg-type]
- ibis/backends/mysql/__init__.py:411: error: Item "Mapping[str, str | DataType]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/mysql/__init__.py:411: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/mysql/__init__.py:438: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]"; expected "Schema"  [arg-type]
+ ibis/backends/mysql/__init__.py:411: error: Item "Mapping[str, str | DataType]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/mysql/__init__.py:411: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/mysql/__init__.py:438: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any"; expected "Schema"  [arg-type]
- ibis/backends/exasol/__init__.py:385: error: Item "Mapping[str, str | DataType]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/exasol/__init__.py:385: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/exasol/__init__.py:410: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]"; expected "Schema"  [arg-type]
+ ibis/backends/exasol/__init__.py:385: error: Item "Mapping[str, str | DataType]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/exasol/__init__.py:385: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/exasol/__init__.py:410: error: Argument "schema" to "DatabaseTable" has incompatible type "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any"; expected "Schema"  [arg-type]
- ibis/backends/duckdb/__init__.py:190: error: Item "Mapping[str, str | DataType]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "null_fields"  [union-attr]
- ibis/backends/duckdb/__init__.py:190: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "null_fields"  [union-attr]
- ibis/backends/duckdb/__init__.py:203: error: Item "Mapping[str, str | DataType]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
- ibis/backends/duckdb/__init__.py:203: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Any | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]]" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/duckdb/__init__.py:190: error: Item "Mapping[str, str | DataType]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "null_fields"  [union-attr]
+ ibis/backends/duckdb/__init__.py:190: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "null_fields"  [union-attr]
+ ibis/backends/duckdb/__init__.py:203: error: Item "Mapping[str, str | DataType]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]
+ ibis/backends/duckdb/__init__.py:203: error: Item "Iterable[tuple[str, str | DataType]]" of "Schema | Mapping[str, str | DataType] | Iterable[tuple[str, str | DataType]] | Any" has no attribute "to_sqlglot"  [union-attr]

pandas (https://.com/pandas-dev/pandas)
- pandas/core/arrays/_mixins.py:150: error: Argument "dtype" to "view" of "ndarray" has incompatible type "ExtensionDtype | dtype[Any]"; expected "dtype[Any] | _HasDType[dtype[Any]]"  [arg-type]
+ pandas/core/arrays/_mixins.py:150: error: Argument "dtype" to "view" of "ndarray" has incompatible type "dtype[Any] | ExtensionDtype"; expected "dtype[Any] | _HasDType[dtype[Any]]"  [arg-type]

@tyrallatyralla requested a review from JukkaL June 13, 2025 17:25
Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

False positive unreachable when variable is set and reset inside loop