Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make reachability code understand chained comparisons (v2) (#8148) #3

Merged
merged 1 commit into from
Dec 25, 2019

Conversation

sthagen
Copy link
Owner

@sthagen sthagen commented Dec 25, 2019

This pull request is v2 (well, more like v10...) of my attempts to
make our reachability code better understand chained comparisons.

Unlike python#7169, this diff focuses
exclusively on adding support for chained operation comparisons and
deliberately does not attempt to change any of the semantics of
how identity and equality operations are performed.

Specifically, mypy currently only examines the first two operands
within a comparison expression when refining types. That means
the following expressions all do not behave as expected:

x: MyEnum
y: MyEnum
if x is y is MyEnum.A:
    # x and y are not narrowed at all

if x is MyEnum.A is y:
    # Only x is narrowed to Literal[MyEnum.A]

This pull request fixes this so we correctly infer the literal type
for x and y in both conditionals.

Some additional notes:

  1. While analyzing our codebase, I found that while comparison
    expressions involving two or more is or == operators
    were somewhat common, there were almost no comparisons involving
    chains of != or is not operators, and no comparisons
    involving "disjoint chains" -- e.g. expressions like
    a == b < c == b where there are multiple "disjoint"
    chains of equality comparisons.

    So, this diff is primarily designed to handle the case where
    a comparison expression has just one chain of is or ==.
    For all other cases, I fall back to the more naive strategy
    of evaluating each comparison individually and and-ing the
    inferred types together without attempting to propagate
    any info.

  2. I tested this code against one of our internal codebases. This
    ended up making mypy produce 3 or 4 new errors, but they all
    seemed legitimate, as far as I can tell.

  3. I plan on submitting a follow-up diff that takes advantage of
    the work done in this diff to complete support for tagged unions
    using any Literal key, as previously promised.

    (I tried adding support for tagged unions in this diff, but
    attempting to simultaneously add support for chained comparisons
    while overhauling the semantics of == proved to be a little
    too overwhelming for me. So, baby steps.)

This pull request is v2 (well, more like v10...) of my attempts to
make our reachability code better understand chained comparisons.

Unlike #7169, this diff focuses
exclusively on adding support for chained operation comparisons and
deliberately does not attempt to change any of the semantics of
how identity and equality operations are performed.

Specifically, mypy currently only examines the first two operands
within a comparison expression when refining types. That means
the following expressions all do not behave as expected:

```python
x: MyEnum
y: MyEnum
if x is y is MyEnum.A:
    # x and y are not narrowed at all

if x is MyEnum.A is y:
    # Only x is narrowed to Literal[MyEnum.A]
```

This pull request fixes this so we correctly infer the literal type
for x and y in both conditionals.

Some additional notes:

1. While analyzing our codebase, I found that while comparison
   expressions involving two or more `is` or `==` operators
   were somewhat common, there were almost no comparisons involving
   chains of `!=` or `is not` operators, and no comparisons
   involving "disjoint chains" -- e.g. expressions like
   `a == b < c == b` where there are multiple "disjoint"
   chains of equality comparisons.

   So, this diff is primarily designed to handle the case where
   a comparison expression has just one chain of `is` or `==`.
   For all other cases, I fall back to the more naive strategy
   of evaluating each comparison individually and and-ing the
   inferred types together without attempting to propagate
   any info.

2. I tested this code against one of our internal codebases. This
   ended up making mypy produce 3 or 4 new errors, but they all
   seemed legitimate, as far as I can tell.

3. I plan on submitting a follow-up diff that takes advantage of
   the work done in this diff to complete support for tagged unions
   using any Literal key, as previously promised.

   (I tried adding support for tagged unions in this diff, but
   attempting to simultaneously add support for chained comparisons
   while overhauling the semantics of `==` proved to be a little
   too overwhelming for me. So, baby steps.)
@sthagen sthagen merged commit 67fb9fc into sthagen:master Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants