◐ Shell
reader mode source ↗
Skip to content

gh-103583: Add codecs and maps to _codecs_* module state#103540

Merged
erlend-aasland merged 5 commits into
python:mainfrom
erlend-aasland:isolate-multibytecodec
Apr 17, 2023
Merged

gh-103583: Add codecs and maps to _codecs_* module state#103540
erlend-aasland merged 5 commits into
python:mainfrom
erlend-aasland:isolate-multibytecodec

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Apr 14, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@erlend-aasland
Rename module state struct …
ed80fe3 
The current naming is too confusing, since we've also got MultibyteCodec_State
@erlend-aasland
Add codec and mapping lists to codec module states
f5b906f
@bedevere-bot bedevere-bot mentioned this pull request Apr 14, 2023
Closed
25 tasks
@erlend-aasland

erlend-aasland commented Apr 14, 2023

Copy link
Copy Markdown
Contributor Author

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.

@erlend-aasland

erlend-aasland commented Apr 14, 2023

Copy link
Copy Markdown
Contributor Author

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.

@erlend-aasland

erlend-aasland commented Apr 14, 2023

Copy link
Copy Markdown
Contributor Author

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.

@erlend-aasland

erlend-aasland commented Apr 14, 2023

Copy link
Copy Markdown
Contributor Author

@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.

@corona10

corona10 commented Apr 15, 2023

Copy link
Copy Markdown
Member

@erlend-aasland Will take a look by this weekend :) Thanks

@corona10 corona10 added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 16, 2023
@bedevere-bot

bedevere-bot commented Apr 16, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 9973a7d 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 16, 2023
@corona10 corona10 self-assigned this Apr 16, 2023
@erlend-aasland erlend-aasland changed the title gh-103092: Isolate _multibytecodec Apr 16, 2023
@bedevere-bot bedevere-bot mentioned this pull request Apr 16, 2023
Closed
@erlend-aasland erlend-aasland marked this pull request as ready for review April 16, 2023 21:51
@erlend-aasland

erlend-aasland commented Apr 16, 2023

Copy link
Copy Markdown
Contributor Author

Are you happy with these changes as they stand, @corona10?

corona10
corona10 approved these changes Apr 16, 2023
View reviewed changes

@corona10 corona10 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.

Hide comment

lgtm :)

@erlend-aasland

erlend-aasland commented Apr 16, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the review, Dong-hee! :)

@erlend-aasland erlend-aasland changed the title gh-103583: Isolate _multibytecodec Apr 16, 2023
@erlend-aasland erlend-aasland added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 16, 2023
@bedevere-bot

bedevere-bot commented Apr 16, 2023

Copy link
Copy Markdown

🤖 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 bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 16, 2023
@miss-islington

miss-islington commented Apr 17, 2023

Copy link
Copy Markdown
Contributor

Status check is done, and it's a success ✅.

@erlend-aasland erlend-aasland merged commit 217911e into python:main Apr 17, 2023
carljm added a commit to carljm/cpython that referenced this pull request Apr 17, 2023
@carljm
Merge branch 'main' into superopt …
5136459 
* 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)
carljm added a commit to carljm/cpython that referenced this pull request Apr 17, 2023
@carljm
Merge branch 'superopt' into superopt_spec …
079c65a 
* 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)
@erlend-aasland erlend-aasland deleted the isolate-multibytecodec branch July 4, 2023 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corona10 corona10 corona10 approved these changes

Assignees

@corona10 corona10

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@erlend-aasland @corona10 @bedevere-bot @miss-islington