Skip to content

bpo-46449: add more assertions to verify that deepfreeze modules are finalized properly#30985

Closed
kumaraditya303 wants to merge 3 commits intopython:mainfrom
kumaraditya303:ref
Closed

bpo-46449: add more assertions to verify that deepfreeze modules are finalized properly#30985
kumaraditya303 wants to merge 3 commits intopython:mainfrom
kumaraditya303:ref

Conversation

@kumaraditya303
Copy link
Copy Markdown
Contributor

@kumaraditya303 kumaraditya303 commented Jan 28, 2022 •
edited by bedevere-bot
Loading

@kumaraditya303 kumaraditya303 changed the title bpo-46449: add more assertions to verify that deepfreeze was finalized properly bpo-46449: add more assertions to verify that deepfreeze is finalized properly Jan 28, 2022
@kumaraditya303 kumaraditya303 force-pushed the ref branch 3 times, most recently from 0d44b58 to d04fe64 Compare January 28, 2022 11:48
@kumaraditya303 kumaraditya303 marked this pull request as draft January 28, 2022 11:58
@kumaraditya303 kumaraditya303 marked this pull request as ready for review January 28, 2022 12:43
@kumaraditya303
Copy link
Copy Markdown
Contributor Author

kumaraditya303 commented Jan 28, 2022

cc @vstinner

@kumaraditya303 kumaraditya303 changed the title bpo-46449: add more assertions to verify that deepfreeze is finalized properly bpo-46449: add more assertions to verify that deepfreeze modules are finalized properly Jan 28, 2022
vstinner
vstinner requested changes Jan 28, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

assert(Py_REFCNT(code) == 1000000000); is very specific to deepfreeze currently implementation, whereas this code is a generic implementation of frozen modules which exists in Python 3.10 and older. I prefer to not leak too many deepfreeze implementation details here.

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Jan 28, 2022

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
Copy link
Copy Markdown
Contributor Author

kumaraditya303 commented Jan 28, 2022

assert(Py_REFCNT(code) == 1000000000); is very specific to deepfreeze currently implementation, whereas this code is a generic implementation of frozen modules which exists in Python 3.10 and older. I prefer to not leak too many deepfreeze implementation details here.

Sure this code is generic implementation of frozen modules but if info->get_code is not null then the block is about deepfreeze implementation as get_code is present only on deepfrozen modules not on just frozen modules.

@vstinner vstinner closed this Jan 28, 2022
@vstinner
Copy link
Copy Markdown
Member

vstinner commented Jan 28, 2022

Sorry, I don't think that these assertions are correct.

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Jan 28, 2022

Tomorrow, a 3rd party code can decide to implement get_code() but use regular code objects, not use immortal code object. This code should remain generic.

@kumaraditya303 kumaraditya303 deleted the ref branch January 28, 2022 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner requested changes

@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner

@encukou encukou Awaiting requested review from encukou

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner

@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

Assignees

No one assigned

Labels

awaiting changes skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kumaraditya303 @bedevere-bot @vstinner @the-knights-who-say-ni