-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 None
promotable to object
, not a subclass, when resolving overloads
#3763
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2650,9 +2650,10 @@ def overload_arg_similarity(actual: Type, formal: Type) -> int: | |
# NoneTyp matches anything if we're not doing strict Optional checking | ||
return 2 | ||
else: | ||
# NoneType is a subtype of object | ||
# NoneType can be promoted to object, but isn't actually a subtype (otherwise you | ||
# wouldn't be able to define overloads between object and None). | ||
if isinstance(formal, Instance) and formal.type.fullname() == "builtins.object": | ||
return 2 | ||
return 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update the docstring above to mention that we treat There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Would that be preferable? I'd be happy to implement (optionally including refactoring this function to return an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
if isinstance(actual, UnionType): | ||
return max(overload_arg_similarity(item, formal) | ||
for item in actual.relevant_items()) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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."