Repository navigation
Unexpected conditional expression type inference change #7986
Description
#7955 caused this example to start generating type errors:
from typing import List, Union
def g(a: List[str], b: bool) -> bool:
x = a and b
y: bool
return x or y # Incompatible return value type (got "Union[List[str], bool]", expected "bool")Previously, this generated no errors. It's unclear whether this fixed a false negative or whether this is a regression.
@Michael0x2a Do you have an idea of what might be going on?
Activity
ilevkivskyi commentedon Nov 20, 2019
I would say this is rather a bug (I am however not sure whether it previously worked on purpose or by coincidence).
I think the inferred type of
xshould be likeUnionType(Instance('list', can_be_true=False), Instance('bool')), and then theorin return statement should eliminate the false-only items from union.JukkaL commentedon Nov 20, 2019
Just to be clear: the new behavior is a bug, or the old behavior is a bug?
Michael0x2a commentedon Nov 20, 2019
The only thing I can think of is the changes to
LastKnownValueEraser.visit_instance: now that we try erasing types if there are any args, that visitor will end up callingInstance.copy_modifiedmore frequently -- and it doesn't seem that copy method actually preserves thecan_be_trueorcan_be_falsefields.I'm not 100% sure, but I feel the new behavior is probably a bug? I tried playing around with the example function, and I could never get it to return a list, so a inferred return type of
boolis probably want we want.But maybe the deeper bug is that there's a discrepancy between how we handle
last_known_valueandcan_be_trueandcan_be_false? It seems we want to erase the latter less aggressively -- so this would probably be yet another binder/"it would be nice if we had a more sophisticated abstract interpreter" issue?ilevkivskyi commentedon Nov 20, 2019
IMO the new behaviour is a false positive.
ilevkivskyi commentedon Nov 20, 2019
@Michael0x2a
I think there is a chance a pretty simple fix exists. Will you have time to look into preserving
can_be_trueandcan_be_falseinInstance.copy_modified()? If not, I can look into this tomorrow.Michael0x2a commentedon Nov 20, 2019
Sure, I can take a look.
Preserve can_be_true and can_be_false when copying types
Preserve can_be_true and can_be_false when copying types
Preserve can_be_true and can_be_false when copying types (#7991)