Short term fix for bytes narrowing

[hauntsaninja]

Fixes #20701 / see discussion there

Background:

• In Rework narrowing logic for equality and identity #20492 I added code to treat bytes, bytearray and memoryview as having custom __eq__, since they can compare equal to values that are of other types. I did this based on primer hits on draft versions of that PR
• In Better narrowing with custom equality #20643 I rewrote the handling of custom equality when narrowing. That fixed many issues, but regressed the equivalent of testNarrowingOptionalBytes because we're now more principled about custom __eq__. Perhaps interestingly the primer on that PR is small and not controversial, so I didn't feel the need to prioritise unsound things

However, we only need one of the operands to have custom equality to model these things as reachable, so I think we can probably just remove builtins.bytes from here.

This behaviour still isn't ideal. For instance, narrowing here shouldn't depend on whether we promote or not (we shouldn't narrow v_all to bytes with --strict-bytes)

The real fix is adding more dedicated special casing so that we have an in-between option between "narrow assuming we can only compare equal to values of the same type" and "assume __eq__ means we can't conclude anything about the positive case"

Other relevant PRs:

• [wip] Better handling of generics when narrowing #20673 attempts to remove the other custom eq logic for list, dict, set I added in Rework narrowing logic for equality and identity #20492 . It should be close to mergeable
• [wip] Improve reachability in narrowing logic, avoid losing narrowing constraints #20660 improves reachability and unfortunately to land it I have to do a rethink for how int, float, complex narrowing works. I actually think the current version has pretty good semantics and the primer is valid (but scary). But I want to think about it more

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[A5rocks]

The real fix is adding more dedicated special casing so that we have an in-between option between "narrow assuming we can only compare equal to values of the same type" and "assume __eq__ means we can't conclude anything about the positive case"

I'm not so sure about this? Can we just treat all literals as being fine?

x: bytes = b"blah"
y: bytes | None = b"blah"
if x == y:
  reveal_type(y)  # should reveal `bytes | None`
if b"blah" == y:
  reveal_type(y)  # should reveal `bytes`

but that's also, not necessarily wrong:

class X(bytes):
  def __eq__(self, *args: object) -> bool:
    return True

x: bytes = X(b"blah")
y: bytes | None = b"blah"
if x == y:
  reveal_type(y)  # yeah, it could be None

It's true that generally we're optimistic about whether something has a custom eq though, so I guess it doesn't matter. (e.g. you could substitute out a subclass for str...) I guess it might be worth experimenting with pretending all (non-final) classes have custom eq, only literals don't...

[A5rocks]

This definitely solves the problem. I guess I'd be interested in something that recognizes:

x: bytes | bytearray | None = ...
if x == b"hello":
  reveal_type(x)  # should be bytes | bytearray
else:
  reveal_type(x)  # should be bytes | bytearray | None

but that's probably more complicated. (and just a bit more special casing)

[hauntsaninja]

Yes, we do indeed now have the behaviour from your test case in this PR

We're optimistic about whether something has a custom eq though [...] guess it might be worth experimenting with pretending all (non-final) classes have custom eq, only literals don't

That's right. I'm not planning on trying this experiment, but given primer on my other custom equality changes maybe it is more viable than I would expect at first 😅

The real fix is adding more dedicated special casing

When I say this, I'm actually talking about bytearray and memoryview. As of this PR, we don't treat bytes as having custom __eq__, but we do still do this for bytearray and memoryview. Which means we will consider memoryview as potentially being equal to None.

What we want is to have a reasonable way to teach mypy that memoryview will compare equal to bytes and bytearray, but not to other types. Similarly, int can compare equal to float and complex, but not to other types (currently trying to work through this in a principled way in my reachability PR that exposes a lot of messiness around here).

Specifically, this reveal_type is wrong: https://github.com/python/mypy/pull/20704/files#diff-0413401ad6a91bc5c66579455009ba606ee0ddfb0a78278c9e53618c83b53ed5R3426

Also we don't have the ideal behaviour on this snippet (which is fine on primer — I mean even this bytes change has no effect on primer — but still nice to have):

x: memoryview | None
if x == b"asdf":
    reveal_type(x)  # memoryview | None
if x == memoryview(b"asdf"):
    reveal_type(x)  # memoryview | None

(edit most of this was written before your second comment loaded)