Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-106078: Isolate decimal module #107287

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

CharlieZhao95
Copy link
Contributor

@CharlieZhao95 CharlieZhao95 commented Jul 26, 2023

Convert decimal global state to true module state.

@CharlieZhao95 CharlieZhao95 marked this pull request as ready for review July 27, 2023 06:14
@CharlieZhao95
Copy link
Contributor Author

cc: @kumaraditya303 @erlend-aasland

@kumaraditya303 kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 27, 2023
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 27, 2023
@kumaraditya303 kumaraditya303 added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section extension-modules C modules in the Modules dir labels Jul 27, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit 50d5243 🤖

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 27, 2023
@python python deleted a comment from bedevere-bot Jul 27, 2023
@rhettinger rhettinger removed their request for review July 27, 2023 10:15
@kumaraditya303 kumaraditya303 merged commit a43cc3f into python:main Jul 28, 2023
24 of 25 checks passed
@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 28, 2023

Following this PR, I now get a warning printed to my terminal whenever I run python -m test test_decimal locally on my Windows machine inside my CPython clone:

>python -m test test_decimal
Running Release|x64 interpreter...
0:00:00 Run tests sequentially
0:00:00 [1/1] test_decimal
C:\Users\alexw\coding\cpython\Modules\_decimal\libmpdec\context.c:57: warning: mpd_setminalloc: ignoring request to set MPD_MINALLOC a second time

(I bisected the warning to this PR)

@@ -5857,7 +5876,7 @@ PyInit__decimal(void)
mpd_free = PyMem_Free;
mpd_setminalloc(_Py_DEC_MINALLOC);
Copy link
Contributor Author

@CharlieZhao95 CharlieZhao95 Jul 29, 2023

Choose a reason for hiding this comment

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

Yes. The warning occures here, which is caused by the multi-phase initialization of decimal module. @AlexWaygood

This warning does not affect the module, because a static flag is used in mpd_setminalloc to ensure that MPD_MINALLOC can only be initialized once.

Maybe we can also add a static flag to the _decimal_exec to suppress this warning. But I'm not sure if it makes sense to do so (we add a static variable just to tell the compiler that we know a warning will be raised here). :)

Copy link
Member

Choose a reason for hiding this comment

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

I think it is reasonable. Warnings are disrupting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants