-
-
Notifications
You must be signed in to change notification settings - Fork 32k
gh-67565: remove redundant C-contiguity checks #105521
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
CC: @vstinner |
It looks like the original old Stefan's patch. But currently most functions use Argument Clinic which copied a code from getargs.c, so now there are a lot of |
@ericsnowcurrently Thank you very much for your valuable feedbacks. I've made some changes. Can you review it again? |
@furkanonder, can you resolve the conflict, please? |
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.
LGTM. I have minor suggestion for tests.
Since this PR adds new tests, I think that it is better to split it on two parts:
- Tests only. This change will be backported to 3.12 and 3.11 to avoid regressions.
- Other changes. So we can be sure that they do not affect the user code.
cc. @ericsnowcurrently, who requested changes. |
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.
LGTM.
Thank you for your contribution, Furkan. It took a long time because I wasn't sure it was completely safe. But I hope the new tests will catch the bug if we are wrong. |
@serhiy-storchaka You're welcome. |
Co-authored-by: Stefan Krah <skrah@bytereef.org>
Co-authored-by: Stefan Krah <skrah@bytereef.org>
Uh oh!
There was an error while loading. Please reload this page.