[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Changes by Serhiy Storchaka : -- stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: Thank you Josh and Xiang for your contribution. -- dependencies: -Replace PyUnicode_CompareWithASCIIString with _PyUnicode_EqualToASCIIString resolution: -> fixed ___ Python tracker

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Roundup Robot added the comment: New changeset b995a6950139 by Serhiy Storchaka in branch '3.6': Issue #21449: Removed private function _PyUnicode_CompareWithId. https://hg.python.org/cpython/rev/b995a6950139 New changeset 9b053d3c74dc by Serhiy Storchaka in branch 'default': Issue #21449:

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: New changeset faf04a995031 by Serhiy Storchaka in branch '3.5': Issue #28701: Replace _PyUnicode_CompareWithId with _PyUnicode_EqualToASCIIId. https://hg.python.org/cpython/rev/faf04a995031 New changeset ff3dacc98b3a by Serhiy Storchaka in branch '3.6': Issue

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: LGTM. Except that _PyUnicode_EqualToASCIIString() could be used for simplicity. > _PyUnicode_CompareWithId is a private function. We can remove it if it has > issues. I would left it in maintained releases and removed it in 3.7 (or 3.6?). --

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Xiang Zhang added the comment: > _PyUnicode_CompareWithId is a private function. We can remove it if it has > issues. It doesn't. But once there is _PyUnicode_EqualToASCIIId, it's can be rarely used. The new patch implements a version of _PyUnicode_EqualToASCIIId. -- Added file:

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

STINNER Victor added the comment: _PyUnicode_CompareWithId is a private function. We can remove it if it has issues. -- ___ Python tracker ___

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: _PyUnicode_EqualToASCIIString() added in issue28701 would help in the patch for this issue. -- dependencies: +Replace PyUnicode_CompareWithASCIIString with _PyUnicode_EqualToASCIIString ___ Python tracker

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: > Callers should be fixed to handle errors. This would make the code of callers more complex for small benefit. And perhaps we will add more callers (if replace PyUnicode_CompareWithASCII with new function). > Since currently _PyUnicode_CompareWithId is

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Xiang Zhang added the comment: Since currently _PyUnicode_CompareWithId is used to compare a unicode with ascii identifiers for all cases, how about introduce a more specific function like _PyUnicode_EqualToASCIIId for this case? We can preserve _PyUnicode_CompareWithId for more general

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

STINNER Victor added the comment: > This issue is not just about readability or performance. It is about > correctness, since none of callers check for failure of > _PyUnicode_CompareWithId. Callers should be fixed to handle errors. -- ___ Python

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: > Please don't modify _PyUnicode_FromId(), I prefer to keep it as it is, decode > from UTF-8. Then we should add handling of three special cases: PyUnicode_READY() fails, _PyUnicode_FromId() fails and both fail due to memory error. This means that should

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

STINNER Victor added the comment: Please don't modify _PyUnicode_FromId(), I prefer to keep it as it is, decode from UTF-8. -- ___ Python tracker ___

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Changes by Antoine Pitrou : -- nosy: -pitrou ___ Python tracker ___ ___ Python-bugs-list

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: _PyUnicode_FromId would not fail due to bad encoded data if use the Latin1 encoding. Seems encoded data always is ASCII. We could use PyErr_WriteUnraisable() for output a warning if it is not. -- ___ Python

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Xiang Zhang added the comment: _PyUnicode_FromId could fail due to memoryerror or bad encoded data. They should be treated differently like PyUnicode_READY. Could we treat it like PyDict_GetItem? If memoryerror happens it's highly possible other parts will fail too. --

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: There are yet few subtle details. 1. _PyUnicode_FromId() uses UTF-8 for decoding from C string, but PyUnicode_CompareWithASCIIString() uses Latin1. Two ways of comparison can return different results. Currently all identifiers are ASCII, thus perhaps we

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Changes by Xiang Zhang : Added file: http://bugs.python.org/file45480/_PyUnicode_EqualToId_v2.patch ___ Python tracker ___

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Xiang Zhang added the comment: > The name _PyUnicode_CompareWithIdEqual looks too long to me. What about > _PyUnicode_EqualToId? +1. I think this name is more clear. Serhiy's idea on the implementation sounds good. As for _PyUnicode_FROM_ID, I think it's better for another issue. --

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Serhiy Storchaka added the comment: This issue is not just about readability or performance. It is about correctness, since none of callers check for failure of _PyUnicode_CompareWithId. I just came to the same problem from other side (replacing PyUnicode_CompareWithASCII with

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Josh Rosenberg added the comment: Is there someone else who should be looking at this? Having a fast path for identifier comparisons makes sense (and the concept of ordering between essentially unique identifiers makes no sense). It's not part of the public API (limited or not) so I don't

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Antoine Pitrou added the comment: Note that PyUnicode_CompareWithASCII should be quite fast in most cases (it uses memcmp() on UCS1 strings). -- nosy: +pitrou stage: - patch review versions: -Python 3.6 ___ Python tracker rep...@bugs.python.org

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

Antoine Pitrou added the comment: That said, I think it's quite a good idea. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21449 ___ ___

[issue21449] Replace _PyUnicode_CompareWithId with _PyUnicode_CompareWithIdEqual

New submission from Josh Rosenberg: _PyUnicode_CompareWithId is used exclusively for equality comparisons (after all, identifiers aren't really sortable in a meaningful way; they're isolated values, not a continuum). But because _PyUnicode_CompareWithId maintains the general comparison