GeezerVison is showing github.com — GH-96458: Statically initialize utf8 representation of static strings by kumaraditya303 · Pull Request #96481 · python/cpython
Conversation
kumaraditya303
commented
Sep 1, 2022
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_PyUnicode_LATIN1_INIT is wrong name. You can statically initialize utf8 representation only in ASCII strings.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Are you generating utf8 strings separately from the ascii string, or do they share the same data?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Are you generating utf8 strings separately from the ascii string, or do they share the same data?
The utf8 cache strings are only ever created and used for non-ASCII strings (i.e. at least one character is 128 or larger). The unicode string object is smart enough to know that for pure ASCII strings the utf8 is identical so it doesn't bother creating or using the utf8 cache value.
This PR only generates utf8 data for exactly 6 strings in deepfreeze.c. I'm sure those are the ones with at least non-ASCII character.
In order to see a leak one would have to request the utf8 encoding of at least one of those 6 strings.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
_PyUnicode_LATIN1_INIT is wrong name. You can statically initialize utf8 representation only in ASCII strings.
What do you mean? Keep in mind that the macro is used exclusively to statically initialize latin-1 strings.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
_PyUnicode_LATIN1_INIT is wrong name. You can statically initialize utf8 representation only in ASCII strings.
What do you mean? Keep in mind that the macro is used exclusively to statically initialize latin-1 strings.
Indeed -- in fact the macro is only used in pycore_runtime_init_generated.h (a generated file :-) to initialize the _latin1 field of unicode objects. So the name seems eminently reasonable.
|
serhiy-storchaka reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Sorry, now I see that _PyUnicode_LATIN1_INIT takes a new argument.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|