Skip to content

bpo-40596: Fix str.isidentifier() for non-canonicalized strings containing non-BMP characters on Windows. #20035

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.

Sign up for GitHub

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

Closed
serhiy-storchaka wants to merge 2 commits into from
Closed

bpo-40596: Fix str.isidentifier() for non-canonicalized strings containing non-BMP characters on Windows. #20035

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee4dfd3
bpo-40596: Fix str.isidentifier() for non-canonicalized strings conta…
serhiy-storchaka May 11, 2020
750d91f
Apply suggestions from code review
serhiy-storchaka May 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
bpo-40596: Fix str.isidentifier() for non-canonicalized strings conta… 
…ining non-BMP characters on Windows.
  • Loading branch information
@serhiy-storchaka
serhiy-storchaka committed May 11, 2020
commit ee4dfd3331eb2b62f77a0965451a02579f9960ca
7 changes: 7 additions & 0 deletions Lib/test/test_unicode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,13 @@ def test_isidentifier(self):
self.assertFalse("©".isidentifier())
self.assertFalse("0".isidentifier())

@support.cpython_only
def test_isidentifier_legacy(self):
import _testcapi
u = '𝖀𝖓𝖎𝖈𝖔𝖉𝖊'
self.assertTrue(u.isidentifier())
self.assertTrue(_testcapi.unicode_legacy_string(u).isidentifier())

def test_isprintable(self):
self.assertTrue("".isprintable())
self.assertTrue(" ".isprintable())
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2020-05-11-20-53-52.bpo-40596.dwOH_X.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed :meth:`str.isidentifier` for non-canonicalized strings containing
non-BMP characters on Windows.
6 changes: 6 additions & 0 deletions Objects/typeobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2319,6 +2319,12 @@ valid_identifier(PyObject *s)
Py_TYPE(s)->tp_name);
return 0;
}
/* Since there is no way to return an error from PyUnicode_IsIdentifier()
we have to call PyUnicode_READY() to ensure that the string object is
in the "canonical" representation. */
Comment thread
serhiy-storchaka marked this conversation as resolved.
Outdated
if (PyUnicode_READY(s) < 0) {
return 0;
}
if (!PyUnicode_IsIdentifier(s)) {
PyErr_SetString(PyExc_TypeError,
"__slots__ must be identifiers");
Expand Down
6 changes: 6 additions & 0 deletions Objects/unicodeobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12373,6 +12373,12 @@ static PyObject *
unicode_isidentifier_impl(PyObject *self)
/*[clinic end generated code: output=fe585a9666572905 input=2d807a104f21c0c5]*/
{
/* Since there is no way to return an error from PyUnicode_IsIdentifier()
we have to call PyUnicode_READY() to ensure that the string object is
in the "canonical" representation. */
Copy link
Copy Markdown
Member

@vstinner vstinner May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that this comment is worth it. It's something usual to report an error when a function fails. All functions in this file does that. I suggest to remove the comment.

Comment thread
serhiy-storchaka marked this conversation as resolved.
Outdated
if (PyUnicode_READY(self) < 0) {
return NULL;
}
return PyBool_FromLong(PyUnicode_IsIdentifier(self));
}

Expand Down