-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
WIP,BUG: ensure that casting to void is properly checked. #13646
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
Conversation
if (i != NPY_DATETIME) { | ||
/* | ||
* Bool -> <Anything> except datetime (since | ||
* it conceptually has no zero) | ||
*/ | ||
_npy_can_cast_safely_table[NPY_BOOL][i] = 1; | ||
} | ||
/* <Anything> -> Object */ | ||
/* <Anything> -> Object is possible */ |
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.
Out of scope, but is this true for object->object?
} | ||
|
||
_npy_can_cast_safely_table[NPY_STRING][NPY_UNICODE] = 1; | ||
_npy_can_cast_safely_table[NPY_BOOL][NPY_TIMEDELTA] = 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.
This is already included in the loop above.
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.
Do you want to make a follow-up patch that removes this then, since you closed this one?
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.
I felt it was too small, but you're right, might as well: #13652
Closing in favour of #13648, as I've convinced myself this was going in the wrong direction, changing things for the general |
As noted in #11114, currently the following clearly wrong things happen:
The present PR fixes it, and doesn't cause errors locally, but that is mostly because no tests are doing with casting to/from void. I'd happily add those, but what should and should not work?
EDIT: Should add that I'm not at all sure this is the right solution. Perhaps more likely the check should be different whenever from or to has fields.
fixes #11114