◐ Shell
clean mode source ↗

gh-105922: Add PyImport_AddModuleRef() function by vstinner · Pull Request #105923 · python/cpython

vstinner

Conversation

erlend-aasland

Choose a reason for hiding this comment

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

Nice; I left some comments.

Too bad we cannot run the ref leak bots now; there's already a ref leak in test_import and test_peg_generator that has not been resolved :(

@vstinner

I updated my PR:

  • Rebased on the merged pythonrunc.c refactoring: commit a5c2ad0
  • Don't deprecate PyImport_AddModule() and PyImport_AddModuleObject() in this PR anymore: I prefer to handle this in a separated PR.
  • Fix pythonrun.c
  • Use PyImport_AddModuleRef() for _frozen_importlib

erlend-aasland

Choose a reason for hiding this comment

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

Small typo nit. Otherwise looks good to me.

 
* Add tests on PyImport_AddModuleRef(), PyImport_AddModule() and
  PyImport_AddModuleObject().
* pythonrun.c: Replace Py_XNewRef(PyImport_AddModule(name)) with
  PyImport_AddModuleRef(name).

@vstinner

I fixed the typo: similar than => similar to.

@vstinner vstinner deleted the pyimport_addmoduleref branch

June 20, 2023 06:48

Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes

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

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

@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