Optimize bind_self() and deprecation checks

[ilevkivskyi]

This contains two related optimizations:

• Simplify deprecation check by removing several bind_self() calls and instead restoring callable type definitions in fixup.py, and using them during overload item matching.
• Consequently, best effort filtering in bind_self() should be not needed anymore, since all non-trivial calls to bind_self() are now after check_self_arg() calls.

There are also few micro-optimizations I noticed when looking at relevant code. In total this gives around 1% on self-check.

Note: this may be not a no-op in some corner cases. If so, I will adjust overload filtering in check_self_arg().

[ilevkivskyi]

OK, the change in trio needs fixing, this is an important edge case. I will take a look later (hopefully tomorrow).

[ilevkivskyi]

OK, I inlined important part of the old logic, and added a test. Although it is a quite niche edge case, I think it is important, as it is another class of examples where the logic actual <: erased(template) (that we use in many places) doesn't work correctly.

I added couple more micro-optimizations, so it looks like now it is even a bit more than 1%.

[ilevkivskyi]

Oh well, it looks like it is a bit too strict now (even though new errors may be correct). I will try to reproduce old logic even closer (hopefully it is possible without an extra loop).

[github-actions]

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

spack (https://github.com/spack/spack)
+ lib/spack/spack/database.py:1765: error: Unused "type: ignore" comment  [unused-ignore]
+ lib/spack/spack/database.py:1765: error: Missing named argument "key" for "sort" of "list"  [call-arg]
+ lib/spack/spack/database.py:1765: note: Error code "call-arg" not covered by "type: ignore" comment
+ lib/spack/spack/solver/asp.py:3134: error: Unused "type: ignore" comment  [unused-ignore]
+ lib/spack/spack/solver/asp.py:3134: error: Missing named argument "key" for "sort" of "list"  [call-arg]
+ lib/spack/spack/solver/asp.py:3134: note: Error code "call-arg" not covered by "type: ignore" comment

pydantic (https://github.com/pydantic/pydantic)
- pydantic/v1/validators.py:615: note:     def __init__(self, str, Iterable[tuple[str, Any]], /) -> NamedTupleT
+ pydantic/v1/validators.py:615: note:     def NamedTuple(self, str, Iterable[tuple[str, Any]], /) -> NamedTupleT
- pydantic/v1/validators.py:615: note:     def __init__(self, str, None = ..., /, **kwargs: Any) -> NamedTupleT
+ pydantic/v1/validators.py:615: note:     def NamedTuple(self, str, None = ..., /, **kwargs: Any) -> NamedTupleT

[ilevkivskyi]

OK, I think the primer output looks good. The change of error code is because previously we returned a single item overload from bind_self(), but check_self_arg() always automatically converted single item overloads to callable types. IMO the new error makes more sense.

[sterliakov]

Great housekeeping effort, thank you!

[sterliakov]

Can this be called before SemanticAnalyzer.visit_decorator processes the node?

[ilevkivskyi]

Unfortunately, there is currently no protection against this (and other typeops-related things) being accessed during semantic analysis (except semanal_typeargs, where it can be, and likely is, accessed). That said, I hope docstring already makes it clear this property should be used for type-checking purposes.

[sterliakov]

Ah, well, this shouldn't be problematic anyway? Even if this check falsely rejects the fast path, the only consequence (not taking potential bugs in fast/slow bind_self into account) is just inefficient check that takes a bit longer, right?

[sterliakov]

But maybe it's better to check that explicitly? If there is exactly one decorator and it refers to property fullname, do not reject. (not sure if it's worth the cycles if you are confident enough that is_trivial_self isn't currently accessed prior to removing @property)

[ilevkivskyi]

not sure if it's worth the cycles if you are confident enough that is_trivial_self isn't currently accessed prior to removing @property

Oh yes, there is no way it is called that soon now. I was thinking more about the future, but you are right, worst case (if someone does something really weird like calling bind_self() during semantic analysis) we will mark something as non-trivial self, while it is actually trivial.