Make None promotable to object, not a subclass, when resolving overloads

[benkuhn]

Fixes #3757. However, I'm not at all sure that this is the right approach--please let me know if this seems reasonable, or if it's an abuse of the meaning of "overload arg similarity" and I should do something else!

[ilinum]

@JukkaL what do you think?

[JukkaL]

This is slightly unsafe -- if we have a variable x with static type object but the value None as an argument, we may infer the wrong result type. I don't think that this is a real problem though. We already have the same problem with int -> float promotions.

[JukkaL]

This looks like a reasonable approach. A few comments below, plus can you also fix this when not using strict optional checking -- it still infers Any for a None argument.

[JukkaL]

Update the docstring above to mention that we treat None -> object as a promotion though it's actually treated as a real subtype elsewhere.

[benkuhn]

Oh, I forgot that this was true. That makes this seem more hacky.

A general solution (not specific to None or strict-optional) would be to add a fourth level of overload arg similarity for subclasses, e.g.

0: No match
1: Actual can be promoted to formal
2: Actual is a subclass of formal
3: Actual is the same as formal

Would that be preferable? I'd be happy to implement (optionally including refactoring this function to return an Enum) if so. It seems less hacky but much more likely to have unforseen consequences/introduce new errors.

[ilevkivskyi]

TBH, I was also thinking that three levels of similarity might be not enough, see e.g. #3656. But introducing a new level of similarity is a big change, so that I think we should wait until @JukkaL is back.

[JukkaL]

Treating promotions as different from other forms of compatibility is already unsafe, and adding more levels of similarity would add more room for unsafety. I'd prefer to use the hack since its effect is limited. Basically we'd only surgically enable a specific use case that we know to be important, but otherwise overload resolution would remain the same.

[JukkaL]

The comment doesn't sound quite right. Maybe something like this: "As a special case, we don't consider NoneType as a subtype of object here, as otherwise you wouldn't be able to define overloads between object and None."

[JukkaL]

@benkuhn Are you interested in finishing this up?

[benkuhn]

Doh, completely forgot about this, sorry! Resurrecting now.

[benkuhn]

Hmm. The previous fix appears no longer to work due to #4118. The change to make object overlap with everything, means that the overload now fails to type-check at definition time.

Thinking out loud about a fix:

Currently, is_unsafe_overlapping_signatures checks is_overlapping_types(t1, t2). If it instead checked overload_argument_similarity(t1, t2) == 2 (for consistency with the resolution logic in overload_call_target), then this change would work again.

This change would break testOverloadTupleInstance and testOverloadTupleEllipsisNumargs, because overload_argument_similarity performs generic erasure whereas is_overlapping_types does not. Note however that the generics are erased when the function is actually invoked, leading to the ability to define overloads that can never be resolved:

def f(x: Tuple[int]) -> int: ...
def f(x: Tuple[str]) -> str: ...
reveal_type(f((1,)))  # E: revealed type is 'Any'

So the behavior of warning that the signatures overlap with incompatible return types is arguably reasonable here--though much less ergonomic, of course.

Ways to get around this that I can think of:

• Add a special case to overload_argument_similarity not to do erasure on Tuple (and maybe other generics?) and use that instead of is_overlapping_types in is_unsafe_overlapping_signatures
• Add a special case for None vs object to is_unsafe_overlapping_signatures (https://github.com/python/mypy/blob/master/mypy/checker.py#L3294). Probably better due to more limited scope, but makes me worried because now this special case lives in 2 places.
• Something better that I'm missing due to lack of context
• give up

Thoughts? @JukkaL

[ilevkivskyi]

@benkuhn
Note that actual meaning of overloads needs some clarifications, see e.g. python/typing#253 The way overloads currently work is influenced by the fact that long time ago mypy supported runtime dispatch on overloads. Therefore something that is currently considered unsafe can be allowed.

[benkuhn]

Oh! I didn't realize that runtime dispatch was completely abandoned. Does that mean there's no longer any reason to do type erasure in overload_arg_similarity (since the comment Overloading resolution is based on runtime behavior which erases type parameters is out of date)? There seems to be some demand for this already (see #4159).

[JukkaL]

Add a special case for None vs object to is_unsafe_overlapping_signatures (https://github.com/python/mypy/blob/master/mypy/checker.py#L3294). Probably better due to more limited scope, but makes me worried because now this special case lives in 2 places.

It's okay to have the special casing in two places, as long as each of them has a comment that cross-references the other place, and we have good test coverage. Would this be sufficient to move forward with this PR?

Also, I'd like to get rid of type erasure when determining whether overload items match. However, there may be some interesting edge cases, and just turning off type erasure may not be sufficient by itself.

Here's my current thinking on how things could work in the future:

• By default, overload checking would be pretty lenient (more lenient than what is today). The goal would make common cases work as expected, at the cost of some additional unsafety. This would allow overloading based on None/object.
• We could add a strictness flag such as --strict-overloads that enforces that overloads are absolutely safe (or as safe as we can make them), at least outside stubs. This mode would flag overloads based on None/object.

Here are some steps if you want to take the route of getting rid of type erasure:

• Don't do type erasure on TupleTypes. These don't need to be erased anyway, as the item types are runtime introspectable. This would be a separate PR that we'd merge before this.
• Don't do type erasure on generics. This would be somewhat unsafe, but we can live with it, and in the long term we'd have a strictness flag to control for this. Again, this would be a separate PR.
• Don't do type erasure on callable types. Again, this is unsafe, but not being able to do this is pretty restrictive. This would be third separate PR.

One final option is to wait until python/typing#253 has been resolved before moving forward with this PR.

[ilevkivskyi]

@Michael0x2a what do you think about this special case? IIUC the idea here is that it is impossible to express "anything but None". Also this PR may be outdated in view of your recent work. I think we might just close this if you agree.

[Michael0x2a]

@ilevkivskyi -- it looks like the issue this PR was trying to address was ultimately related to typing descriptors, which we decided to address by just letting the __get__ method have unsafe return types. I'm hoping that's enough and that APIs that need to distinguish between None and literally anything else end up being rare.

If we do want to support this use case more broadly, I can implement that -- I just wish there were a way of doing it in a more principled way instead of special-casing overloads. (E.g. maybe have all non-null concrete types be a subclass of or promote to some phantom type, maybe special-case some Protocol... But I'm guessing this pretty unlikely to happen: it's unclear to me how we'd support such a type for non-strict optional users, we'd probably need to amend PEP 484 and get buy-in from other type checkers...)

But yeah, I think we can just close this. I'm actually planning on simplifying overload_arg_similarity at some point so it just returns a bool: since we just pick the first match, I don't think we need to do anything with promotions anymore.

[ilevkivskyi]

OK, lets just close this then. @benkuhn Thanks for your efforts!