Description

gdementen

opened on Feb 14, 2023

Issue body actions

Feature or enhancement

I just noticed the _bisectmodule.c is the only internal module using the Py_EnterRecursiveCall and Py_LeaveRecursiveCall public API functions instead of the private ones with underscores. For both consistency and a tiny performance enhancement, I think it would make sense to use the private API instead.

Pitch

I have a PR ready for this, but since I have no idea of what I am doing this might be completely bogus 😉.

Linked PRs

• gh-101903: use _Py_EnterRecursiveCall internal API in _bisectmodule.c #101931
• gh-101903: Remove unnecessary undefs for non-existent/old macros Py_EnterRecursiveCall and Py_LeaveRecursiveCall #101923

Activity

1. gdementen

   added

   type-featureA feature request or enhancementA feature request or enhancement

   on Feb 14, 2023
2. OTheDev commented on Feb 14, 2023

   

   OTheDev

   on Feb 14, 2023

   Contributor

   More actions

   Looks like the Public API functions

   cpython/Python/ceval.c

   Lines 3066 to 3081 in 23751ed

   	/* Implement Py_EnterRecursiveCall() and Py_LeaveRecursiveCall() as functions
   	for the limited API. */
   	#undef Py_EnterRecursiveCall
   	int Py_EnterRecursiveCall(const char *where)
   	{
   	return _Py_EnterRecursiveCall(where);
   	}
   	#undef Py_LeaveRecursiveCall
   	void Py_LeaveRecursiveCall(void)
   	{
   	_Py_LeaveRecursiveCall();
   	}

   are just wrappers for the private static inline functions

   cpython/Include/internal/pycore_ceval.h

   Lines 135 to 147 in 7644935

   	static inline int _Py_EnterRecursiveCall(const char *where) {
   	PyThreadState *tstate = _PyThreadState_GET();
   	return _Py_EnterRecursiveCallTstate(tstate, where);
   	}
   	static inline void _Py_LeaveRecursiveCallTstate(PyThreadState *tstate) {
   	tstate->c_recursion_remaining++;
   	}
   	static inline void _Py_LeaveRecursiveCall(void) {
   	PyThreadState *tstate = _PyThreadState_GET();
   	_Py_LeaveRecursiveCallTstate(tstate);
   	}

   @gdementen - I don't see the harm in submitting a PR!
3. arhadthedev

   added

   extension-modulesC modules in the Modules dirC modules in the Modules dir

   on Feb 15, 2023
4. iritkatriel

   added a commit that references this issue on Feb 21, 2023

   gh-101903: Remove obsolete undefs for previously removed macros Py_En…

   7346a38
5. erlend-aasland commented on Feb 21, 2023

   

   erlend-aasland

   on Feb 21, 2023

   Contributor

   More actions

   As noted in #101931 (comment), this requires defining _bisectmodule.c as a core module, which IMO requires a broader discussion.

   We deliberately make some stdlib extension modules use the public C API. As an (unrwritten) general rule, the modules in Modules/Setup.bootstrap.in are built using the internal C API, whereas the modules in Modules/Setup.stdlib.in should preferably use the public C API.

   Suggesting to close this issue as not-planned.

   Reacted by Gaëtan de Menten
6. erlend-aasland

   added

   pendingThe issue will be closed if no feedback is providedThe issue will be closed if no feedback is provided

   on Feb 21, 2023
7. erlend-aasland commented on Feb 21, 2023

   

   erlend-aasland

   on Feb 21, 2023

   Contributor

   More actions

   See also Eric's comments from similar API issues: #31376 (comment) and #31376 (comment)
8. erlend-aasland

   closed this as not plannedon Feb 21, 2023
9. erlend-aasland

   removed

   pendingThe issue will be closed if no feedback is providedThe issue will be closed if no feedback is provided

   on Feb 21, 2023
10. OTheDev commented on Feb 21, 2023

    

    OTheDev

    on Feb 21, 2023

    Contributor

    More actions

    Thanks @erlend-aasland

    Reacted by Gaëtan de Menten
11. carljm

    added a commit that references this issue on Feb 23, 2023

    Merge branch 'main' into lockfreecp

    b257a16
12. python-sidebar

    added 2 commits that reference this issue on Sep 1, 2024

    pythongh-101903: Remove obsolete undefs for previously removed macros…

    b0f2b1b

    pythongh-101903: Remove obsolete undefs for previously removed macros…

    6f23007
13. JelleZijlstra

    added a commit that references this issue on Sep 10, 2024

    pythongh-101903: Remove obsolete undefs for previously removed macros…

    f3640ce

Issue actions