-
-
Notifications
You must be signed in to change notification settings - Fork 3k
feat: add CastExpr support to constant_fold_expr
[1/1]
#19983
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
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
constant_fold_expr
[1/1]constant_fold_expr
[1/1]
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.
cast
is supposed to raise a TypeError if the target type is not compatible, and it's also expected to change the type of the value. I think this needs to be more a bit more complex to fully preserve semantics. Do you have a real-world use case where this would help? If not, due to the subtleties I mentioned this may not be worth it.
Yes, I use a dependency that uses a bunch of NewType types that have chain inheritance Example of shallow chain inheritance: CoolString = NewType("CoolString", str) so if you want to go from a my_sad_normal_string = ":("
special = SpecialString(CoolString(my_sad_normal_string)) my_sad_normal_string = ":("
special = cast(SpecialString, my_sad_normal_string) I prefer the latter since its less verbose Sometimes these casts take place when assigning values to Final vars, so imo they're a good candidate for folding. Even if nobody else cares, it definitely helps me personally! :D What if we only constant fold in cases where we can ensure no type collisions? easy logic would be to unwrap new types and ensure the base type matches, bail out in all other cases. This seems safe to do in all cases I can think of. more advanced logic could probably be implemented to cover other cases. I think we could match on rprimitive type without risking safety, (with the exception of object_rprimitive, which we should bail on), but that might be unnecessary? I want to think about it a little more. |
Let's put this on hold until #19982 is merged, so I can use the @folding_candidate decorator |
Maybe its the case that this particular folding makes sense for mypyc but not for mypy itself |
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
This PR adds CastExpr support to
constant_fold_expr
since its just a noop