FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-157710: Move docs for mutating PyUnicode to new section by encukou · Pull Request #159028 · python/cpython · GitHub

Repository navigation

gh-157710: Move docs for mutating PyUnicode to new section - #159028

Open
encukou wants to merge 3 commits into
python:mainfrom
encukou:move-deprecated-unicode-api
Open

encukou wants to merge 3 commits into
python:mainfrom
encukou:move-deprecated-unicode-api

Conversation

encukou commented Oct 8, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

This moves documentation of the naughty functions to a new own section under the existing "Deprecated API", to de-emphasize them, and provide a common introduction (with links from each function).

encukou requested a review from vstinner October 8, 2026 14:42
encukou requested a review from ZeroIntensity as a code owner October 8, 2026 14:42
bedevere-app Bot added the type-feature A feature request or enhancement label Oct 8, 2026
bedevere-app Bot added awaiting core review docs Documentation in the Doc dir skip news labels Oct 8, 2026

read-the-docs-community Bot commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #35022584 | 📁 Comparing 944886c against main (05d80cc)

  🔍 Preview build  

1 file changed
± c-api/unicode.html

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's a great idea to moving these functions to a dedicated function and write more details on conditions when using these functions is safe!

Comment thread Doc/c-api/unicode.rst

- must not be hashed,
- must not be :c:func:`converted to UTF-8 <PyUnicode_AsUTF8AndSize>`,
or another non-"canonical" representation,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Currently, _PyUnicode_IsModifiable() returns 1 even if _PyUnicode_UTF8() is not NULL (for non-ASCII strings). Maybe it would be worth it return 0 in this case.

Note: For compact ASCII strings, _PyUnicode_UTF8() is always non-NULL.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yeah. If you modify the string after the UTF-8 representation is cached, it'll get out of sync.

For compact ASCII strings, _PyUnicode_UTF8() has undefined behaviour. Maybe it wants an assert.

I filed #159033

Comment thread Doc/c-api/unicode.rst

encukou left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The details on conditions are just moved from the PyUnicode_New docs :)

Comment thread Doc/c-api/unicode.rst

- must not be hashed,
- must not be :c:func:`converted to UTF-8 <PyUnicode_AsUTF8AndSize>`,
or another non-"canonical" representation,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yeah. If you modify the string after the UTF-8 representation is cached, it'll get out of sync.

For compact ASCII strings, _PyUnicode_UTF8() has undefined behaviour. Maybe it wants an assert.

I filed #159033

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

ZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm a little late to the party, but this seems worthwhile!

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge docs Documentation in the Doc dir skip news type-feature A feature request or enhancement

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL