bpo-46881: Statically allocate and initialize the latin1 characters.

[kumaraditya303]

https://bugs.python.org/issue46881

Automerge-Triggered-By: GH:ericsnowcurrently

[ericsnowcurrently]

Other than one small thing, LGTM.

[kumaraditya303]

This will require some more changes because only strings in range(128) use PyASCIIObject whereas latin1 uses PyCompactUnicodeObject hence tests are failing.
One idea I have is to use have two array of struct one of PyASCIIObject and one PyCompactUnicodeObject to store the latin1 characters.

[ericsnowcurrently]

You might want to take a look at my attempt at this from December: ericsnowcurrently@9c93a13. Its a little outdated but, IIRC, it worked.

[ericsnowcurrently]

Also, we should probably add all 256 to _PyRuntimeState.global_objects.singletons.strings, identifiers for the ASCII alphabet characters and literals for the rest. Then we point to those in our lookup array.

[ericsnowcurrently]

This is looking good. I've left a couple comments for you to consider.

Also, I recommend updating _PyUnicode_InitGlobalObjects() to call _PyUnicode_CheckConsistency() on each of the latin1 objects, like we do with the empty string. This helps ensure that we are statically initializing the objects correctly. We should probably do that for at least one of the identifiers and one of the other "literals", for the same reason, but that can be addressed separately.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[ericsnowcurrently]

Please avoid rebasing once someone has reviewed a PR. It makes it hard for others to follow what you have changed (and GitHub isn't great at adjusting for the "force push"). Instead, merge main into your branch.

[kumaraditya303]

Please avoid rebasing once someone has reviewed a PR. It makes it hard for others to follow what you have changed (and GitHub isn't great at adjusting for the "force push"). Instead, merge main into your branch.

Yes, I try to avoid rebase but there was a conflict and I basically had to rewrite the PR so rebase and start from main branch was easier.

[ericsnowcurrently]

there was a conflict and I basically had to rewrite the PR so rebase and start from main branch was easier.

I've certainly been in that situation. 🙂 The problem here is that I could not tell what you changed since the last time I looked at the PR. Also, GitHub doesn't show the results of previous CI runs if you force-push. I wanted to see what was failing there before. Neither would have been a problem with a merge instead of a rebase.

It's not the end of the world but it does relate to the community nature of Python core development. In case you find it helpful, here's a little insight:

The question of rebase-vs-merge in a PR is one of those little things that can add up in an open-source project's collaborative workflow. My concern isn't so much for my own time (though I do appreciate consideration 🙂). Rather, I strongly encourage you to be respectful of reviewers' time in general. In a community project like CPython, we all need to treat the time of other contributors as sacred. I always try to ask myself if there's good reason to (effectively) demand that someone else spend an additional portion of their free time to accommodate me saving some time. The answer is almost always "no".

My intention isn't to chastise you or tell you that you are doing anything wrong. I just want to be sure you have an awareness of the human element of the project. I really do appreciate all the work you have been doing on behalf of the Python community! Python wouldn't be nearly as great as it is without contributors like you sacrificing their spare time. ❤️

[kumaraditya303]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@ericsnowcurrently: please review the changes made to this pull request.

[ericsnowcurrently]

LGTM )aside from a couple of minor comments)

I'll merge once the remaining review comments are addressed.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[kumaraditya303]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@ericsnowcurrently: please review the changes made to this pull request.

[kumaraditya303]

@ericsnowcurrently FYI I had changed the macros as you suggested, please review again thanks

[ericsnowcurrently]

LGTM

Thanks for working on this!