Better handling of compile-time relational operators #3158
Conversation
| b = PY3 and 's' | ||
| c = PY2 or 's' | ||
| d = PY3 or 's' | ||
| reveal_type(a) # E: Revealed type is 'builtins.bool' |
There was a problem hiding this comment.
Shouldn't this be Union[bool, str]? Same for all of these, really.
There was a problem hiding this comment.
Oh wait this is testing the special-casing for PY2 and PY3 as well. I guess it's okay. I may not get to the full review until next week.
|
@ilevkivskyi Could you review this? |
|
OK, will review this tomorrow. |
|
Sorry for the delays; I kinda got hung up on finals... |
|
NP, hope finals went/are going/will go well! |
| PY2 = f() | ||
| PY3 = f() | ||
|
|
||
| a = PY2 and 's' |
There was a problem hiding this comment.
I would add few more tests exchanging the order of 's' and PY2/PY3. So that e.g. 's' and False == False.
There was a problem hiding this comment.
@ilevkivskyi That doesn't work, though, since infer_condition_value doesn't take constants into account...
| y = 'x' / 1 # E: Unsupported left operand type for / ("str") | ||
| y | ||
|
|
||
|
|
There was a problem hiding this comment.
This empty line is unnecessary.
| x = 'foo' | ||
| else: | ||
| x = 3 | ||
| reveal_type(x) # E: Revealed type is 'builtins.str' |
There was a problem hiding this comment.
I would also add few more tests here and for and, so that more combinations of True and False are tested.
|
@kirbyfan64 Are you going to continue working on this PR (here are just few tests missing and a merge conflict needs to be fixed)? |
|
My motherboard died. Somehow, I feel like this has been one of the craziest college years I've ever had... |
refi64
force-pushed
the
relop
branch
2 times, most recently
from
19e8556 to
3c78e82
Compare
|
@ilevkivskyi Done! |
3 participants
Inspired by (but doesn't fix [yet!]) #3156. (A "fix" would to be to add an early
returntocheck_boolean_opife.right_unreachableisTrue, but I'm not sure if that change would have other repercussions that I'm not aware of.)Now stuff like this works better: