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

Constant fold more unary and binary expressions #14838

Closed
wants to merge 160 commits into from

Conversation

ichard26
Copy link
Collaborator

@ichard26 ichard26 commented Mar 5, 2023

Now mypy can constant fold these additional operations:

  • Float arithmetic
  • Mixed int and float arithmetic
  • String multiplication
  • Complex plus or minus a literal real (eg. 1+j2)

While this can be useful with literal types, the main goal is to improve constant folding in mypyc (mypyc/mypyc#772).

mypyc can also fold bytes addition and multiplication, but mypy cannot as byte values can't be easily stored anywhere.

try:
ret = left**right
except OverflowError:
return None
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Digging through the CPython source, only power ops can raise an OverflowError.

@@ -3350,7 +3350,7 @@ def analyze_simple_literal_type(self, rvalue: Expression, is_final: bool) -> Typ
return None

value = constant_fold_expr(rvalue, self.cur_mod_id)
if value is None:
if value is None or isinstance(value, complex):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no idea whether complex literals make any sense in mypy. PTAL.

value = node.final_value
if isinstance(value, (CONST_TYPES)):
return value
final_value = node.final_value
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The value local name should support ConstantValue | None, but Var.final_value does not support bytes. To avoid causing type errors later in the function from this assignment implicitly setting value's type, these variables were renamed.

Comment on lines -1039 to +1040
return 5j+1.0
real = 1
return 5j+real
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The intent of this change is to preserve the old test case behavior where constant folding wasn't performed.

div = 2.0 / 0.0
floor_div = 2.0 // 0.0
power_imag = (-2.0)**0.5
power_overflow = 2.0**10000.0
Copy link
Collaborator Author

@ichard26 ichard26 Mar 5, 2023

Choose a reason for hiding this comment

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

While floats usually don't raise an error on overflow, this is the one exception I know of. This is why there's a guard against OverflowError in mypy/constant_fold.py.

@ichard26 ichard26 marked this pull request as draft March 5, 2023 18:50
@github-actions

This comment has been minimized.

@ichard26 ichard26 force-pushed the improved-constant-folding branch from eb204a9 to 8d24649 Compare March 5, 2023 19:52
@github-actions

This comment has been minimized.

@ichard26 ichard26 marked this pull request as ready for review March 5, 2023 20:51
hauntsaninja and others added 5 commits March 5, 2023 14:03
In python#4070, `CallableType.__init__` was changed to accept `arg_names:
Sequence[str | None]` so we could pass e.g. `list[str]` to it. We're
making a similar change to `CallableType.copy_modified`.
Validate `attr.evolve` calls to specify correct arguments and types.

The implementation makes it so that at every point where `attr.evolve`
is called, the signature is modified to expect the attrs class'
initializer's arguments (but so that they're all kw-only and optional).

Notes:
- Added `class dict: pass` to some fixtures files since our attrs type
stubs now have **kwargs and that triggers a `builtin.dict` lookup in
dozens of attrs tests.
- Looking up the type of the 1st argument with
`ctx.api.expr_checker.accept(inst_arg)` which is a hack since it's not
part of the plugin API. This is a compromise for due to python#10216.

Fixes python#14525.
Fixes python#14841.

`types-backports` is obsolete and leads to incorrect suggestions; see
the issue.

`types-backports_abc` was removed from typeshed in python/typeshed#7533.
@AlexWaygood AlexWaygood added the topic-mypyc mypyc bugs label Mar 6, 2023
@JukkaL
Copy link
Collaborator

JukkaL commented Mar 6, 2023

Thanks for the PR! This may have some conflicts with an work-in-progress PR that I've been working on for some time that adds native float support. Since I've had to postpone the native float PR a few times already (in part due to merge conflicts), and it results in pretty big perf gains in benchmarks that use floats, I'd like to get it merged ASAP, which would mean postponing this PR. The conflicts shouldn't be too difficult to handle but might be a bit annoying. I'll get back to this PR afterwards. Sorry about the delay and friction. I'll try to get my PR up this weekend (but no promises).

@ichard26
Copy link
Collaborator Author

ichard26 commented Mar 6, 2023

No worries! Native float support is exciting!

I'll work on another issue in the meanwhile. I was planning on taking a stab at fixing the tuple unboxing code, table-driven imports, optimizing membership tests against Final tuples, or using generic PySetGets for Python level module getters and setters. I hope one of those doesn't conflict with your WIP mypyc PRs.

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 6, 2023

I don't expect any of the features you mention to have non-trivial conflicts. You should be good to go!

hauntsaninja and others added 6 commits March 6, 2023 17:13
…#14827)

Fixes python#14782

Currently, mypy issues a false positive if you try to unpack an enum
class:

```python
from enum import Enum

class E(Enum):
    A = 1
    B = 2

A, B = E  #  error: "Type[E]" object is not iterable  [misc]
```

This is because of a more general problem with class objects that have
`__iter__` defined on their metaclass. Mypy issues a false positive on
this code, where `Foo` is iterable by virtue of having `Meta` as its
metaclass:

```python
from typing import Iterator
class Meta(type):
    def __iter__(cls) -> Iterator[int]:
        yield from [1, 2, 3]

class Foo(metaclass=Meta): ...

a, b, c = Foo  # error: "Type[Foo]" object is not iterable  [misc]
reveal_type(a)  # error: Cannot determine type of "a"  [has-type]  # note: Revealed type is "Any"
```

This PR fixes the false positive with enums, and the more general false
positive with iterable class objects.
I reviewed documentation changes to be included in mypy 1.1.
The `stubtest.py` cast is no longer needed -- I think the mypyc issue
was fixed by python/typeshed#9146.

The `types.py` casts were never needed. They were added relatively
recently by somebody called "Alex Waygood".
…in (python#14854)

Fixes: python#14853

Adding support for `if` statements in the `dataclass` and `dataclass_transform` decorators, so
the attributes defined conditionally are treated as those that are
directly in class body.
…14864)

Previously most member expressions only produced the wildcard reference
such as `*.attr` when using the (undocumented) `--export-ref-info` flag.
Use the type of the object to generate better fullnames, such as
`module.Cls.attr`.
AlexWaygood and others added 26 commits May 1, 2023 15:22
This is allegedly causing large performance problems, see 13821

typeshed/8231 had zero hits on mypy_primer, so it's not the worst thing
to undo. Patching this in typeshed also feels weird, since there's a
more general soundness issue. If a typevar has a bound or constraint, we
might not want to solve it to a Literal.

If we can confirm the performance regression or fix the unsoundness
within mypy, I might pursue upstreaming this in typeshed.

(Reminder: add this to the sync_typeshed script once merged)
Since the plugin provides superior type checking: python#13987 (comment)
A manual cherry-pick of e437cdf.
…ython#15155)

Fixes python#15060 

The example in the issue contains another case where an erased type may
legitimately appear in a generic function. Namely, when we have a lambda
in a generic context, and its body contains a call to a generic
_method_. First, since we infer the type of lambda in erased context,
some of lambda parameters may get assigned a type containing erased
components. Then, when accessing a generic method on such type we may
get a callable that is both generic and has erased components, thus
causing the crash (actually there are two very similar crashes depending
on the details of the generic method).

Provided that we now have two legitimate cases for erased type appearing
in `expand_type()`, and special-casing (enabling) them would be tricky
(a lot of functions will need to have `allow_erased_callables`), I
propose to simply remove the check.

Co-authored-by: Shantanu <[email protected]>
…n#15157)

Fix python#15004 

FWIW I don't think dataclass protocols make much sense, but we
definitely should not crash. Also the root cause has nothing to do with
dataclasses, the problem is that a self attribute assignment in a
protocol created a new `Var` (after an original `Var` was created in
class body), which is obviously wrong.
…eal error code (python#15164)

Fixes python#8823 

The issue above makes `--warn-unused-ignores` problematic for
cross-platform/cross-version code (btw it is one of the most upvoted
bugs). Due to how the unused ignore errors are generated, it is hard to
not generate them in unreachable code without having precise span
information for blocks. So my fix will not work on Python 3.7, where end
lines are not available, but taking into account that 3.7 reaches end of
life in less than 2 month, it is not worth the effort.

Also this PR makes `unused-ignore` work much more like a regular error
code, so it can be used with
`--enable-error-code`/`--disable-error-code` (including per-file) etc.
More importantly this will also allow to use `--warn-unused-ignores` in
code that uses e.g. `try/except` imports (instead of `if` version
checks) with the help of `# type: ignore[import,unused-ignore]` (which
btw will also help on Python 3.7).

Note also currently line numbers for `Block`s are actually wrong (and
hence few TODOs in `fastparse.py`). So in this PR I set them all
correctly and update a lot of parse tests (I went through all test
updates and verified they are reasonable).

---------

Co-authored-by: Jukka Lehtosalo <[email protected]>
<!--pre-commit.ci start-->
updates:
- [github.com/pycqa/isort: 5.11.5 →
5.12.0](PyCQA/isort@5.11.5...5.12.0)
- [github.com/pycqa/flake8: 5.0.4 →
6.0.0](PyCQA/flake8@5.0.4...6.0.0)
<!--pre-commit.ci end-->

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Fixes python#2608.

Use the heuristic suggested in python#2608 and allow narrowed types of
variables (but not attributes) to be propagated to nested functions if
the variable is not assigned to after the definition of the nested
function in the outer function.

Since we don't have a full control flow graph, we simply look for
assignments that are textually after the nested function in the outer
function. This can result in false negatives (at least in loops) and
false positives (in if statements, and if the assigned type is narrow
enough), but I expect these to be rare and not a significant issue. Type
narrowing is already unsound, and the additional unsoundness seems
minor, while the usability benefit is big.

This doesn't do the right thing for nested classes yet. I'll create an
issue to track that.

---------

Co-authored-by: Alex Waygood <[email protected]>
This adds tests for mixing star args with typevar tuples from PEP646. In
order to do this we have to handle an additional case in applytype where
the result of expanding an unpack is a homogenous tuple rather than a
list of types.

This PR also includes (partially by accident) a fix to the empty case
for type analysis. After re-reading the PEP we discover that the empty
case is meant to be inferred as a Tuple[Any, ...] rather than an empty
tuple and must fix that behavior as well as some of the previous test
cases that assumed that it meant an empty tuple.

When we actually call `target(*args)` from the testcase in `call` we
expose a crash in mypy. This will be handled by a subsequent PR.
…4835)" (python#15179)

This reverts commit a9ee618.

The original fix doesn't work in all cases. Reverting it since fixing
remaining issues is non-trivial, and it's better not to include
regressions or partially implemented features in mypy 1.3. See python#9655 for
context.
Fixes python#12677

When assert_type fails, when the type of value examined and the
specified type have the same name, mypy returns an error with more
descriptive and distinct names.
Fixes python#12977

IMO current dict expression inference logic is quite arbitrary: we only
take the non-star items to infer resulting type, then enforce it on the
remaining (star) items. In this PR I simplify the logic to simply put
all expressions as arguments into the same call. This has following
benefits:
* Makes everything more consistent/predictable.
* Fixes one of top upvoted bugs
* Makes dict item indexes correct (previously we reshuffled them causing
wrong indexes for non-star items after star items)
* No more weird wordings like `List item <n>` or `Argument <n> to
"update" of "dict"`
* I also fix the end position of generated expressions to show correct
spans in errors

The only downside is that we will see `Cannot infer type argument` error
instead of `Incompatible type` more often. This is because
`SupportsKeysAndGetItem` (used for star items) is invariant in key type.
I think this is fine however, since:
* This only affects key types, that are mixed much less often than value
types (they are usually just strings), and for latter we use joins.
* I added a dedicated note for this case
…4680)

Fixes python#9901
Fixes python#13662

Fix inheriting from a call-based `collections.namedtuple` /
`typing.NamedTuple` definition that was omitted from the generated stub.

This automatically adds support for the call-based `NamedTuple` in
general not only in class bases (Closes python#13788).

<details>
<summary>An example before and after</summary>
Input:

```python
import collections
import typing
from collections import namedtuple
from typing import NamedTuple

CollectionsCall = namedtuple("CollectionsCall", ["x", "y"])

class CollectionsClass(namedtuple("CollectionsClass", ["x", "y"])):
    def f(self, a):
        pass

class CollectionsDotClass(collections.namedtuple("CollectionsClass", ["x", "y"])):
    def f(self, a):
        pass

TypingCall = NamedTuple("TypingCall", [("x", int | None), ("y", int)])

class TypingClass(NamedTuple):
    x: int | None
    y: str

    def f(self, a):
        pass

class TypingClassWeird(NamedTuple("TypingClassWeird", [("x", int | None), ("y", str)])):
    z: float | None

    def f(self, a):
        pass

class TypingDotClassWeird(typing.NamedTuple("TypingClassWeird", [("x", int | None), ("y", str)])):
    def f(self, a):
        pass
```

Output diff (before and after):
```diff
diff --git a/before.pyi b/after.pyi
index c88530e2c..95ef843b4 100644
--- a/before.pyi
+++ b/after.pyi
@@ -1,26 +1,29 @@
+import typing
 from _typeshed import Incomplete
 from typing_extensions import NamedTuple

 class CollectionsCall(NamedTuple):
     x: Incomplete
     y: Incomplete

-class CollectionsClass:
+class CollectionsClass(NamedTuple('CollectionsClass', [('x', Incomplete), ('y', Incomplete)])):
     def f(self, a) -> None: ...

-class CollectionsDotClass:
+class CollectionsDotClass(NamedTuple('CollectionsClass', [('x', Incomplete), ('y', Incomplete)])):
     def f(self, a) -> None: ...

-TypingCall: Incomplete
+class TypingCall(NamedTuple):
+    x: int | None
+    y: int

 class TypingClass(NamedTuple):
     x: int | None
     y: str
     def f(self, a) -> None: ...

-class TypingClassWeird:
+class TypingClassWeird(NamedTuple('TypingClassWeird', [('x', int | None), ('y', str)])):
     z: float | None
     def f(self, a) -> None: ...

-class TypingDotClassWeird:
+class TypingDotClassWeird(typing.NamedTuple('TypingClassWeird', [('x', int | None), ('y', str)])):
     def f(self, a) -> None: ...
```
</details>
@ichard26 ichard26 closed this May 6, 2023
@ichard26 ichard26 deleted the improved-constant-folding branch May 6, 2023 17:22
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-mypyc mypyc bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.