fix Cannot infer set type from usage #2693#2785
fix Cannot infer set type from usage #2693#2785asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Fixes a delayed/first-use inference edge case where an initial narrowing-style read (e.g. if key not in seen:) incorrectly disabled later inference from mutating uses (e.g. seen.add(key)), preventing set() from being inferred as set[str] in common dedup patterns.
Changes:
- Adjusted deferred bound-name finalization so narrowing reads no longer consume/disable first-use inference.
- Added a regression test covering
set()inference inside a loop with aninmembership check before.add(...).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pyrefly/lib/binding/bindings.rs |
Updates first-use/deferred inference side-effects for narrowing reads during deferred bound-name finalization. |
pyrefly/lib/test/delayed_inference.rs |
Adds a regression testcase ensuring seen = set() infers set[str] in the reported loop pattern. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| } else { | ||
| // Narrowing or other reads: mark as DoesNotPin if still undetermined. | ||
| if matches!(first_use, FirstUse::Undetermined) { | ||
| self.mark_does_not_pin_if_first_use(def_idx); | ||
| } | ||
| // Narrowing reads should not pin partial types, but they also should not | ||
| // consume first-use inference. A later ordinary read may still determine the | ||
| // type, such as `if key not in seen: seen.add(key)`. | ||
| } |
There was a problem hiding this comment.
This else branch is intentionally a no-op but is an empty block (comment-only). If cargo clippy is run with clippy::all, this can trigger clippy::empty_else warnings. Consider removing the else block and moving the comment to a standalone comment after the if/else if chain (or otherwise ensuring the branch isn’t syntactically empty).
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: rotki (https://github.com/rotki/rotki)
+ ERROR rotkehlchen/data_import/utils.py:75:21-53: Object of class `bool` has no attribute `append` [missing-attribute]
ibis (https://github.com/ibis-project/ibis)
+ ERROR ibis/expr/types/relations.py:514:12-18: Returned type `dict[object, ibis.expr.operations.core.Value[Unknown]]` is not assignable to declared return type `Mapping[str, ibis.expr.types.generic.Value]` [bad-return]
pwndbg (https://github.com/pwndbg/pwndbg)
- ERROR pwndbg/commands/vmmap.py:326:77-87: Object of class `NoneType` has no attribute `vaddr` [missing-attribute]
- ERROR pwndbg/commands/vmmap.py:328:29-39: Object of class `NoneType` has no attribute `vaddr` [missing-attribute]
apprise (https://github.com/caronc/apprise)
+ ERROR apprise/cli.py:1040:17-44: Object of class `int` has no attribute `append`
+ Object of class `str` has no attribute `append` [missing-attribute]
+ ERROR apprise/cli.py:1089:30-45: Type `int` is not iterable [not-iterable]
+ ERROR apprise/cli.py:1090:27-36: Object of class `str` has no attribute `url` [missing-attribute]
+ ERROR apprise/cli.py:1102:24-34: Object of class `str` has no attribute `tags` [missing-attribute]
+ ERROR apprise/cli.py:1104:66-78: No matching overload found for function `str.join` called with arguments: (object | set[Unknown] | Unknown) [no-matching-overload]
+ ERROR apprise/config/base.py:788:43-53: Argument `dict[object, set[Unknown]]` is not assignable to parameter `group_tags` with type `dict[str, set[str]]` in function `ConfigBase.__normalize_tag_groups` [bad-argument-type]
+ ERROR apprise/config/base.py:1300:43-53: Argument `dict[object | Unknown, set[object] | set[Unknown]]` is not assignable to parameter `group_tags` with type `dict[str, set[str]]` in function `ConfigBase.__normalize_tag_groups` [bad-argument-type]
cloud-init (https://github.com/canonical/cloud-init)
+ ERROR cloudinit/net/eni.py:442:9-40: Object of class `str` has no attribute `append` [missing-attribute]
+ ERROR cloudinit/netinfo.py:355:16-20: Returned type `dict[Unknown, dict[str, Unknown]] | dict[Unknown, dict[str, bool | list[Unknown] | str]] | dict[Unknown, Unknown] | Unknown` is not assignable to declared return type `dict[str, dict[str, Interface]]` [bad-return]
+ ERROR cloudinit/netinfo.py:376:12-16: Returned type `dict[Unknown, dict[str, Unknown]] | dict[Unknown, dict[str, bool | list[Unknown] | str]] | dict[Unknown, Unknown] | Unknown` is not assignable to declared return type `dict[str, dict[str, Interface]]` [bad-return]
hydpy (https://github.com/hydpy-dev/hydpy)
+ ERROR hydpy/models/kinw/kinw_model.py:3505:38-40: Argument `list[Parameter | float | Unknown]` is not assignable to parameter `hvector` with type `Iterable[float]` in function `BaseModelProfile.calculate_qgvector` [bad-argument-type]
prefect (https://github.com/PrefectHQ/prefect)
+ ERROR src/prefect/server/models/block_schemas.py:575:21-85: Object of class `dict` has no attribute `append` [missing-attribute]
+ ERROR src/prefect/server/models/block_schemas.py:584:81-587:22: Cannot set item in `dict[str | None, dict[str, str]]` [unsupported-operation]
|
Primer Diff Classification❌ 6 regression(s) | ✅ 1 improvement(s) | 7 project(s) total | +14, -2 errors 6 regression(s) across rotki, pwndbg, apprise, cloud-init, hydpy, prefect. error kinds:
Detailed analysis❌ Regression (6)rotki (+1)
Pyrefly is incorrectly inferring the type of This is a type inference limitation/regression. The code is correct at runtime, and neither mypy nor pyright flag it.
pwndbg (-2)
apprise (+6)
Per-category reasoning:
cloud-init (+3)
eni.py:442 — netinfo.py:355,376 — The
hydpy (+1)
The analysis correctly identifies this as a false positive caused by the PR's inference changes.
prefect (+2)
For error 1 (line 575): The For error 2 (line 584): Pyrefly rejects assigning a Both are false positives because the outer dict
✅ Improvement (1)ibis (+1)
Suggested fixesSummary: The PR's removal of **1. In } else {
// Narrowing reads: only skip DoesNotPin for containment checks
// (e.g., `key not in seen`) to allow later .add() to pin the type.
// For other narrowing patterns, preserve DoesNotPin to avoid
// over-constraining dict value types from heterogeneous literals.
if matches!(first_use, FirstUse::Undetermined) && !is_containment_narrowing_on_self(current_idx) {
self.mark_does_not_pin_if_first_use(def_idx);
}
}Alternatively, a simpler fix: instead of distinguishing narrowing kinds, change the approach so that narrowing reads neither pin NOR consume first-use inference AND also don't mark DoesNotPin — but ensure that when the empty container's type is resolved, heterogeneous dict literal values are handled by inferring the value type from ALL assignments to the dict (not just the first one that pins). The core issue is that without DoesNotPin, the first ordinary read after
2. As an alternative/complementary fix in the type inference layer (likely in the dict literal type resolution or the first-use type solver): when resolving the type of an empty dict
Was this helpful? React with 👍 or 👎 Classification by primer-classifier (7 LLM) |
Summary
Fixes #2693
Adjusted first-use binding so an initial narrowing read no longer permanently disables deferred inference. In practice, if key not in seen: no longer forces seen off the first-use path, so the later seen.add(key) can infer set[str] as intended.
Test Plan
add test