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

Empty list unification #1403

Merged
merged 5 commits into from
Apr 20, 2016
Merged

Empty list unification #1403

merged 5 commits into from
Apr 20, 2016

Conversation

gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Apr 18, 2016

There are three TODOs left:

  • Suppress errors of first attempt if we analyze the left branch a second time (needs a reproducing example)
  • Pick a better name for has_unfinished_types()
  • Find an example where the overall context is required for ([x] if ... else []) (and that works with the limited test fixtures)

@gvanrossum
Copy link
Member Author

(Note, this branches off #1402 but it's really independent from that.)

@gvanrossum
Copy link
Member Author

gvanrossum commented Apr 19, 2016

FWIW I racked my brain but I have not come up with an example where both the context and the order of evaluation (left-first or right-first) matter. In all cases I can think of, if a context is provided at all, either if_expr fits the context and then the type of if_expr is the same as the context; or if_expr doesn't fit the context and then it's an error no matter what.

@gvanrossum
Copy link
Member Author

gvanrossum commented Apr 19, 2016

Here's something suggesting that maybe I'm not going far enough?

a = [0] if False else [0.0]
a() # So mypy reveals the type of a

The errors are:

x.py:1: error: List item 0 has incompatible type "float"
x.py:2: error: List[int] not callable

I would expect [0] and [0.0] to unify to List[float], but it doesn't seem to work that way. It's not because of invariance -- if I use [0.0] and [0] it happily infers List[float].

EDIT That's not new behavior -- it's just because the else-clause is analyzed using the type of the if-clause as context, and that was always there. However in this case my code to reverse the roles doesn't trigger because [0] doesn't have any type variables degenerated to None.

@gvanrossum
Copy link
Member Author

My intuition just wants to try both versions always: (a) analyze the if-clause first and use it as the context to analyze the else-clause; (b) the reverse; and then (c) use the best of the two, reporting only the errors from the winner. But I don't have a good intuition for how to judge the best result -- it could be the one with the least errors, or the one that isn't object or Any or some other degenerate result. Suggestions?

@@ -1502,6 +1521,19 @@ def not_ready_callback(self, name: str, context: Context) -> None:
self.chk.handle_cannot_determine_type(name, context)


# TODO: What's a good name for this function?
def has_unfinished_types(t: Type) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a similar function already in checker.py: is_valid_inferred_type. You can probably use it instead of this.

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 19, 2016

If we do it always both ways, we could pick the result where the result of the join is the most specific (s is more specific than t if s is a subtype of t but not vice versa).

Here are three more ideas for the [0] vs [0.0] problem:

  1. If the type inferred for the right operand is neither a subtype nor a supertype of the left operand type, type check the left operand again, using the right operand type as the type context.

  2. Check if the erased right operand type is a subtype of the left operand type but the non-erased type is not a subtype, and in this case type check the left operand again similar to (1). This is more specific than (1) so I'd probably prefer this over (1).

  3. Calculate the second pass context as a "covariant" join of the left and right operand types. In a covariant join invariant types are treated as covariant, so that covariant_join(List[int], List[float]) == List[float]. First type check both operands using the context for the whole expression, and then if neither operand is a subtype of the other operand, use the "covariant" join as context for both and try again. This would also work for the [] if x else [1] case as covariant_join(List[None], List[int]) == List[int].

This problem is actually more general. For example, consider this code (I'm not suggesting to fix this now, but giving this as additional context):

if x:
    a = [0]
else:
    a = [0.0]

@gvanrossum gvanrossum force-pushed the empty-list-unification branch from 1d2a30d to e20d469 Compare April 19, 2016 18:44
@gvanrossum
Copy link
Member Author

I've addressed your low-level review nits and refactored the code a bit, but I'm not making any progress on the [0]...[0.0] issue, and I propose to give up on that for now. I'll file it as a new issue we can address for a later release.

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 20, 2016

Makes sense -- the [0] .. [0.0] issue is likely rare and not worth spending a ton of time on now.

@JukkaL JukkaL merged commit 110842c into master Apr 20, 2016
@gvanrossum gvanrossum deleted the empty-list-unification branch April 20, 2016 15:22
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