➜
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2016-09-28 08:11 by xiang.zhang, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| PyUnicode_AsUCS4.patch | xiang.zhang, 2016-09-28 15:13 | review | ||
| PyUnicode_AsUCS4_v2.patch | xiang.zhang, 2016-09-30 05:15 | review | ||
| PyUnicode_AsUCS4_v3.patch | xiang.zhang, 2016-10-02 10:56 | review | ||
| Messages (11) | |||
|---|---|---|---|
| msg277595 - (view) | Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-09-28 08:11 | |
PyUnicode_AsUCS4 declares to raise ValueError if buffer length is smaller than string length. But now the implementation raises SystemError. |
|||
| msg277637 - (view) | Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-09-28 15:13 | |
Here is the patch. |
|||
| msg277715 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-09-29 19:25 | |
In general the patch LGTM. I have small doubt about the name of one variable, and may be worth to add yet few tests. |
|||
| msg277738 - (view) | Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-09-30 05:15 | |
v2 now adds more tests and change the problematic variable name. |
|||
| msg277886 - (view) | Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-10-02 10:56 | |
v3 resolves comments on v2. |
|||
| msg277905 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-10-02 17:39 | |
The remaining question is what should be the type of the exception. ValueError is documented exception, but SystemError is actually raised exception (and it always was raised). PyUnicode_AsUCS4() is used 6 times in 3 files in CPython code, and it should never raise this exception. PyUnicode_AsUCS4() is in public API an can be used in third party code. Seems raising this exception can be caused only by programming error in C extension. SystemError is right exception in this case. It looks to me that the code is correct and the documentation should be fixed to match the code. |
|||
| msg277906 - (view) | Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-10-02 17:54 | |
Yes. Changing the implementation or the doc is still in question so in the title I just use conflicts. Scanning unicodeobject.c there seems no general rules about which to use. But actually I'm in favour of ValueError. From the description in https://docs.python.org/3/library/exceptions.html, SystemError is raised when the interpreter finds an internal error. I actually don't think this error is an interpreter internal error. But no matter what the resolution is, we can add a test for ucs4. :-) |
|||
| msg277908 - (view) | Author: Roundup Robot (python-dev)  |
Date: 2016-10-02 18:31 | |
New changeset ac838bf5499d by Serhiy Storchaka in branch '3.5': Issue #28295: Fixed the documentation and added tests for PyUnicode_AsUCS4(). https://hg.python.org/cpython/rev/ac838bf5499d New changeset 3efeafc6517a by Serhiy Storchaka in branch '3.6': Issue #28295: Fixed the documentation and added tests for PyUnicode_AsUCS4(). https://hg.python.org/cpython/rev/3efeafc6517a New changeset 94fb4c4f58dd by Serhiy Storchaka in branch 'default': Issue #28295: Fixed the documentation and added tests for PyUnicode_AsUCS4(). https://hg.python.org/cpython/rev/94fb4c4f58dd |
|||
| msg277909 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-10-02 18:35 | |
Additional tests are always nice. Thank you for your report and your patch Xiang. |
|||
| msg277910 - (view) | Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-10-02 18:50 | |
Serhiy, in 05788a9a0b88, test_invalid_sequences seems don't have to stay in CAPITest. |
|||
| msg277911 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-10-02 19:00 | |
Good catch! Thanks Xiang. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:37 | admin | set | github: 72482 |
| 2016-10-02 19:00:59 | serhiy.storchaka | set | messages: + msg277911 |
| 2016-10-02 18:50:39 | xiang.zhang | set | messages: + msg277910 |
| 2016-10-02 18:35:41 | serhiy.storchaka | set | status: open -> closed assignee: serhiy.storchaka resolution: fixed stage: patch review -> resolved |
| 2016-10-02 18:35:19 | serhiy.storchaka | set | messages: + msg277909 |
| 2016-10-02 18:31:24 | python-dev | set | nosy:
+ python-dev messages: + msg277908 |
| 2016-10-02 17:54:06 | xiang.zhang | set | messages: + msg277906 |
| 2016-10-02 17:39:02 | serhiy.storchaka | set | messages: + msg277905 |
| 2016-10-02 10:56:17 | xiang.zhang | set | files:
+ PyUnicode_AsUCS4_v3.patch messages: + msg277886 |
| 2016-09-30 05:16:00 | xiang.zhang | set | files:
+ PyUnicode_AsUCS4_v2.patch messages: + msg277738 |
| 2016-09-29 19:25:05 | serhiy.storchaka | set | messages: + msg277715 |
| 2016-09-28 15:13:41 | xiang.zhang | set | files:
+ PyUnicode_AsUCS4.patch keywords: + patch messages: + msg277637 stage: patch review |
| 2016-09-28 08:11:06 | xiang.zhang | create | |