gh-103583: Add codecs and maps to _codecs_* module state #103540
Conversation
erlend-aasland
commented
Apr 14, 2023
•
erlend-aasland
commented
- Isolate Stdlib Extension Modules #103092
- Isolate _multibytecodec #103583
The current naming is too confusing, since we've also got MultibyteCodec_State
|
I've done a couple of experiments with isolating the codec submodules, but it gets hairy pretty quickly. If any of you see an elegant solution, please shout out. |
|
Also: this is very much WIP. With this PR, I've tried to keep the diff small, but I really do not like all these macro templates. It all feels very fragile to me. |
|
Most of the remaining globals are just static data, as far as I can see. Perhaps the solution is as easy as making sure they all live in the const data segment and forget about module state. |
|
@corona10, as the code owner for these modules: what do you think about the approach taken in this PR? IMO, I'm not too fond of using macros for so much templating, but I went for it nonetheless since that's the approach already taken by this part of the code base. |
|
@erlend-aasland Will take a look by this weekend :) Thanks |
| static const MultibyteCodec *codec_list = \ | ||
| (const MultibyteCodec *)_codec_list; | ||
|
|
||
| #define CODEC_STATEFUL(enc) \ |
There was a problem hiding this comment.
Wow, readability will be frustrating (it's not your fault), I will think about a better way.. :(
There was a problem hiding this comment.
I know, readability is already frustrating with these modules. If we are to keep the diff down, we just have to go with the macro template style already used. An alternative is to refactor the modules first, but that bears a risk.
corona10
added
the
🔨 test-with-refleak-buildbots
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
erlend-aasland
changed the title
|
Are you happy with these changes as they stand, @corona10? |
|
Thanks for the review, Dong-hee! :) |
erlend-aasland
changed the title
erlend-aasland
added
the
🔨 test-with-refleak-buildbots
|
🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 65af110 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
|
Status check is done, and it's a success ✅. |
* main: Remove `expert-*` from `project-updater` GH workflow (python#103579) pythongh-103583: Add codecs and maps to _codecs_* module state (python#103540) pythongh-48330: address review comments to PR-12271 (python#103209) pythongh-103527: Add multibytecodec.h as make dep for _codecs_* (python#103567) pythongh-103553: Improve `test_inspect`: add more assertions, remove unused (python#103554) pythonGH-103517: Improve tests for `pathlib.Path.walk()` (pythonGH-103518) pythongh-102114: Make dis print more concise tracebacks for syntax errors in str inputs (python#102115) pythonGH-78079: Fix UNC device path root normalization in pathlib (pythonGH-102003) pythongh-101517: Add regression test for a lineno bug in try/except* impacting pdb (python#103547) pythongh-103527: Add make deps for _codecs_* and _multibytecodec (python#103528) pythongh-103532: Fix reST syntax in NEWS entry (pythonGH-103544) pythongh-103532: Add NEWS entry (python#103542)
* superopt: update generated cases with new comment review comments Remove `expert-*` from `project-updater` GH workflow (python#103579) pythongh-103583: Add codecs and maps to _codecs_* module state (python#103540) pythongh-48330: address review comments to PR-12271 (python#103209) pythongh-103527: Add multibytecodec.h as make dep for _codecs_* (python#103567) pythongh-103553: Improve `test_inspect`: add more assertions, remove unused (python#103554) pythonGH-103517: Improve tests for `pathlib.Path.walk()` (pythonGH-103518) pythongh-102114: Make dis print more concise tracebacks for syntax errors in str inputs (python#102115) pythonGH-78079: Fix UNC device path root normalization in pathlib (pythonGH-102003) pythongh-101517: Add regression test for a lineno bug in try/except* impacting pdb (python#103547) pythongh-103527: Add make deps for _codecs_* and _multibytecodec (python#103528) pythongh-103532: Fix reST syntax in NEWS entry (pythonGH-103544) pythongh-103532: Add NEWS entry (python#103542)
