Skip to content

Reconcile diamond-inherited generic bases with variance - #5100

Open
nitishagar wants to merge 1 commit into
facebook:mainfrom
nitishagar:fix/diamond-inheritance-variance-4842
Open

nitishagar wants to merge 1 commit into
facebook:mainfrom
nitishagar:fix/diamond-inheritance-variance-4842

Conversation

@nitishagar

Copy link
Copy Markdown
Contributor

Fixes #4842

Problem

When two base paths reach the same generic ancestor, pyrefly compared the two specializations with plain type-argument equality:

class Base(Generic[ValueT_contra, ValueT_co]): ...
class RangeBase(Base[ValueT_contra, ValueT_co], Generic[ValueT_contra, ValueT_co]): ...
class WideParam(Base[SupportsInt | SupportsIndex, int]): ...

class NarrowRange(RangeBase[int, int], WideParam): ...
# ERROR: Class `NarrowRange` has inconsistent type arguments for base class `Base`:
# `Base[int, int]` and `Base[SupportsIndex | SupportsInt, int]`

Base is contravariant in its first parameter, so Base[SupportsIndex | SupportsInt, int] is a subtype of Base[int, int] and the diamond is consistent — pyright accepts it (and this pattern occurs in the wild, e.g. click's ParamType hierarchy).

Change

The conflict predicate inside the C3-linearization check now delegates to a variance-aware comparison callback (constructed where solver access exists):

  • positions are compared under the ancestor class's variance (declared keywords or inferred, via the same VarianceMap the subset check uses);
  • a covariant or contravariant position is compatible when either specialization is assignable to the other (the first-wins walk order must not decide the outcome);
  • invariant and bivariant positions still require two-way consistency, so distinct type variables and unrelated invariant arguments error exactly as before, with the same message;
  • Any remains gradual; MRO construction (C3) and the first-wins ancestor selection are unchanged.

One pre-existing expectation is updated deliberately: the Iterable[int] vs Iterable[int | str] diamond in tuple.rs is a covariant-legal pair under the adopted semantics (pyright-compatible), so it no longer errors.

Test plan

  • New tests: the issue's contravariant diamond (clean), a covariant diamond (clean), int vs str in a contravariant position (still errors, same message), an Any diamond (no-regression pin)
  • All previously pinned inconsistent-diamond tests (generic_basic.rs, tuple.rs minus the covariant-legal case above) pass unedited
  • cargo test -p pyrefly --lib -- --skip test::lsp: 7744 passed, 0 failed (the two missing-source LSP interaction failures also fail on unmodified main)
  • Formatting and linting clean

Two base paths reaching the same ancestor class were compared with plain
type-argument equality, so a diamond whose specializations differ only in
a non-invariant position was rejected: `NarrowRange(RangeBase[int, int],
WideParam)` reporting `Base[int, int]` against `Base[SupportsIndex |
SupportsInt, int]` even though `Base` is contravariant in that position
and the pair is therefore consistent (issue facebook#4842; pyright accepts).

The conflict predicate in the C3 linearization now delegates to a
comparison callback that checks each differing position under the
ancestor class's variance: covariant or contravariant positions are
compatible when either specialization is assignable to the other (the
first-wins walk order is arbitrary), invariant and bivariant positions
require two-way consistency as before, and `Any` stays gradual. Distinct
type variables and unrelated invariant arguments keep erroring with the
same message, and the `Iterable[int]` / `Iterable[int | str]` diamond
that the old equality check flagged is now correctly accepted.
@meta-cla meta-cla Bot added the cla signed label Oct 5, 2026
@github-actions github-actions Bot added the size/m label Oct 5, 2026
@meta-codesync

meta-codesync Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D123340876. (Because this pull request was imported automatically, there will not be any future comments.)

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

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

============================================================
SUMMARY
============================================================
Total: +0 new errors, -9 fixed errors
By preset: +0/-9 (default), +0/-9 (strict)

Projects with changes (2):
  steam.py: +0 -8
  meson: +0 -1
============================================================

FULL DIFF DETAILS
------------------------------------------------------------

steam.py (https://github.com/Gobot1234/steam.py)
- ERROR steam/ext/commands/converters.py:273:7-20: Class `UserConverter` has inconsistent type arguments for base class `ConverterBase`: `ConverterBase[PartialUser]` and `ConverterBase[User]` [invalid-inheritance]
- ERROR steam/ext/commands/converters.py:273:7-20: Class `UserConverter` has inconsistent type arguments for base class `Converter`: `Converter[PartialUser]` and `Converter[User]` [invalid-inheritance]
- ERROR steam/ext/commands/converters.py:294:7-29: Class `PartialMemberConverter` has inconsistent type arguments for base class `ConverterBase`: `ConverterBase[PartialUser]` and `ConverterBase[PartialMember]` [invalid-inheritance]
- ERROR steam/ext/commands/converters.py:294:7-29: Class `PartialMemberConverter` has inconsistent type arguments for base class `Converter`: `Converter[PartialUser]` and `Converter[PartialMember]` [invalid-inheritance]
- ERROR steam/ext/commands/converters.py:302:7-22: Class `MemberConverter` has inconsistent type arguments for base class `ConverterBase`: `ConverterBase[PartialUser]` and `ConverterBase[Member]` [invalid-inheritance]
- ERROR steam/ext/commands/converters.py:302:7-22: Class `MemberConverter` has inconsistent type arguments for base class `Converter`: `Converter[PartialUser]` and `Converter[Member]` [invalid-inheritance]
- ERROR steam/ext/commands/converters.py:389:7-26: Class `PartialAppConverter` has inconsistent type arguments for base class `ConverterBase`: `ConverterBase[App]` and `ConverterBase[PartialApp[str]]` [invalid-inheritance]
- ERROR steam/ext/commands/converters.py:389:7-26: Class `PartialAppConverter` has inconsistent type arguments for base class `Converter`: `Converter[App]` and `Converter[PartialApp[str]]` [invalid-inheritance]

meson (https://github.com/mesonbuild/meson)
- ERROR mesonbuild/utils/universal.py:716:7-33: Class `PerThreeMachineDefaultable` has inconsistent type arguments for base class `PerMachine`: `PerMachine[_T | None | None]` and `PerMachine[_T | None]` [invalid-inheritance]

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

invalid-inheritance doesn't account for variance when reconciling diamond-inherited generic base classes

2 participants