-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
Make PyUnicode_CompareWithASCIIString() never failing #72994
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
Comments
PyUnicode_CompareWithASCIIString() never set an exception in 3.2 and earlier versions. Since 3.3 it sets an exception and returns -1 if the first argument is not ready Unicode object, but this was not documented until bpo-28701. Due to undocumenting this behavior many (if not all) callers don't check whether it returned an error. Proposed patch restores old behavior of PyUnicode_CompareWithASCIIString(). |
LGTM. Although at first I am not in favour of this change but searching Github it seems all usages of this API doesn't check the possible error. |
Thanks Xiang. Ned, I ask your permission for committing this patch in 3.6. This is not critical neither new bug, it was introduced in 3.3 and is rarely reproducible. But 3.6b4 is the first version in which the changed behavior was documented, and this patch reverts the documentation change together with fixing the behavior. If release 3.6.0 without this patch, this will encourage users to change their code for handling possible error in PyUnicode_CompareWithASCIIString(), but these changes will be not needed when commit the patch. |
I reviewed PyUnicode_CompareWithASCIIString-no-errors.patch. |
Ah wait, you want to push this change into Python 3.5? I would prefer to leave Python 3.5 unchanged. Even if you modify Python 3.5, you still have to mention the "versionchanged", since it's a behaviour change. |
Usually we don't add "versionchanged" for every fixed bug. |
Updated patch addresses some Victor's comments. But I disagree that this bugfix needs a versionchanged directive. |
This is currently blocking 360rc1. Victor, Serhiy, we either need to get this in now or wait until 3.6.1. I am leaning towards pushing the latest patch for 3.6 without having a decision about 3.5 yet. |
PyUnicode_CompareWithASCIIString-no-errors-2.patch LGTM. Go ahead Serhiy. |
New changeset b431d39da67f by Serhiy Storchaka in branch '3.5': New changeset 5bdc8e1a50c8 by Serhiy Storchaka in branch '3.6': New changeset db56e39ea067 by Serhiy Storchaka in branch 'default': |
Thanks Ned and Victor. |
Misc/NEWS
so that it is managed by towncrier #552Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: