Skip to content

gh-131238: Move _Py_VISIT_STACKREF() to pycore_stackref.h #131560

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 21, 2025
Merged

gh-131238: Move _Py_VISIT_STACKREF() to pycore_stackref.h #131560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions Include/internal/pycore_gc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,16 +352,6 @@ union _PyStackRef;
extern int _PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg);
extern int _PyGC_VisitStackRef(union _PyStackRef *ref, visitproc visit, void *arg);

// Like Py_VISIT but for _PyStackRef fields
#define _Py_VISIT_STACKREF(ref) \
do { \
if (!PyStackRef_IsNull(ref)) { \
int vret = _PyGC_VisitStackRef(&(ref), visit, arg); \
if (vret) \
return vret; \
} \
} while (0)

#ifdef Py_GIL_DISABLED
extern void _PyGC_VisitObjectsWorldStopped(PyInterpreterState *interp,
gcvisitobjects_t callback, void *arg);
Expand Down
3 changes: 2 additions & 1 deletion Include/internal/pycore_genobject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_interpframe.h" // _PyInterpreterFrame
#include "pycore_interpframe_structs.h" // _PyGenObject

#include <stddef.h> // offsetof()


static inline
PyGenObject *_PyGen_GetGeneratorFromFrame(_PyInterpreterFrame *frame)
Expand Down
10 changes: 10 additions & 0 deletions Include/internal/pycore_stackref.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,16 @@ _Py_TryIncrefCompareStackRef(PyObject **src, PyObject *op, _PyStackRef *out)

#endif

// Like Py_VISIT but for _PyStackRef fields
#define _Py_VISIT_STACKREF(ref) \
do { \
if (!PyStackRef_IsNull(ref)) { \
int vret = _PyGC_VisitStackRef(&(ref), visit, arg); \
Copy link
Contributor

Choose a reason for hiding this comment

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

This depends on _PyGC_VisitStackRef, which is in pycore_gc.h.

What's the motivation for moving the macro here?

Copy link
Member Author

Choose a reason for hiding this comment

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

pycore_stackref.h depends on pycore_object.h which depends on pycore_gc.h.

_Py_VISIT_STACKREF() (currently defined in pycore_gc.h) uses PyStackRef_IsNull() which is defined in pycore_stackref.h.

Problem: If dependencies are made explicit (that's part of my work), pycore_gc.h should depend on pycore_stackref.h which creates a dependency cycle!

Moving _Py_VISIT_STACKREF() to pycore_stackref.h breaks this cycle.

Copy link
Member Author

Choose a reason for hiding this comment

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

Previously, using _Py_VISIT_STACKREF() without including pycore_stackref.h worked thanks to luck and the fact that including one header pulled dozens of other headers. With my work on reducing dependencies, this luck is gone, and dependencies must be written explicitly.

if (vret) \
return vret; \
} \
} while (0)

#ifdef __cplusplus
}
#endif
Expand Down
1 change: 1 addition & 0 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,7 @@ PYTHON_HEADERS= \
$(srcdir)/Include/internal/pycore_interp.h \
$(srcdir)/Include/internal/pycore_interp_structs.h \
$(srcdir)/Include/internal/pycore_interpframe.h \
$(srcdir)/Include/internal/pycore_interpframe_structs.h \
$(srcdir)/Include/internal/pycore_intrinsics.h \
$(srcdir)/Include/internal/pycore_jit.h \
$(srcdir)/Include/internal/pycore_list.h \
Expand Down
14 changes: 7 additions & 7 deletions Objects/frameobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
/* Frame object implementation */

#include "Python.h"
#include "pycore_cell.h" // PyCell_GetRef()
#include "pycore_ceval.h" // _PyEval_SetOpcodeTrace()
#include "pycore_code.h" // CO_FAST_LOCAL, etc.
#include "pycore_code.h" // CO_FAST_LOCAL
#include "pycore_dict.h" // _PyDict_LoadBuiltinsFromGlobals()
#include "pycore_frame.h" // PyFrameObject
#include "pycore_function.h" // _PyFunction_FromConstructor()
#include "pycore_genobject.h" // _PyGen_GetGeneratorFromFrame()
#include "pycore_moduleobject.h" // _PyModule_GetDict()
#include "pycore_cell.h" // PyCell_GetRef() PyCell_SetTakeRef()
#include "pycore_interpframe.h" // _PyFrame_GetLocalsArray()
#include "pycore_modsupport.h" // _PyArg_CheckPositional()
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
#include "pycore_opcode_metadata.h" // _PyOpcode_Deopt, _PyOpcode_Caches
#include "pycore_opcode_metadata.h" // _PyOpcode_Caches
#include "pycore_optimizer.h" // _Py_Executors_InvalidateDependency()
#include "pycore_unicodeobject.h" // _PyUnicode_Equal()


#include "frameobject.h" // PyFrameObject
#include "pycore_frame.h"
#include "frameobject.h" // PyFrameLocalsProxyObject
#include "opcode.h" // EXTENDED_ARG

#include "clinic/frameobject.c.h"


#define PyFrameObject_CAST(op) \
(assert(PyObject_TypeCheck((op), &PyFrame_Type)), (PyFrameObject *)(op))

Expand Down
8 changes: 5 additions & 3 deletions Objects/genobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,19 @@
#include "pycore_call.h" // _PyObject_CallNoArgs()
#include "pycore_ceval.h" // _PyEval_EvalFrame()
#include "pycore_frame.h" // _PyInterpreterFrame
#include "pycore_freelist.h" // _Py_FREELIST_FREE(), _Py_FREELIST_POP()
#include "pycore_freelist.h" // _Py_FREELIST_FREE()
#include "pycore_gc.h" // _PyGC_CLEAR_FINALIZED()
#include "pycore_genobject.h"
#include "pycore_genobject.h" // _PyGen_SetStopIterationValue()
#include "pycore_interpframe.h" // _PyFrame_GetCode()
#include "pycore_modsupport.h" // _PyArg_CheckPositional()
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
#include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_*
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_UINT8_RELAXED()
#include "pycore_pyerrors.h" // _PyErr_ClearExcState()
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_warnings.h" // _PyErr_WarnUnawaitedCoroutine()


// Forward declarations
static PyObject* gen_close(PyObject *, PyObject *);
static PyObject* async_gen_asend_new(PyAsyncGenObject *, PyObject *);
Expand Down
1 change: 1 addition & 0 deletions PCbuild/pythoncore.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@
<ClInclude Include="..\Include\internal\pycore_interp.h" />
<ClInclude Include="..\Include\internal\pycore_interp_structs.h" />
<ClInclude Include="..\Include\internal\pycore_interpframe.h" />
<ClInclude Include="..\Include\internal\pycore_interpframe_structs.h" />
<ClInclude Include="..\Include\internal\pycore_intrinsics.h" />
<ClInclude Include="..\Include\internal\pycore_jit.h" />
<ClInclude Include="..\Include\internal\pycore_list.h" />
Expand Down
3 changes: 3 additions & 0 deletions PCbuild/pythoncore.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,9 @@
<ClInclude Include="..\Include\internal\pycore_interpframe.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_interpframe_structs.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_intrinsics.h">
<Filter>Include\cpython</Filter>
</ClInclude>
Expand Down
4 changes: 2 additions & 2 deletions Python/_warnings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#include "Python.h"
#include "pycore_frame.h" // PyFrameObject members
#include "pycore_frame.h" // PyFrameObject
#include "pycore_genobject.h" // PyAsyncGenObject
#include "pycore_import.h" // _PyImport_GetModules()
#include "pycore_interp.h" // PyInterpreterState.warnings
#include "pycore_interpframe.h" // _PyFrame_GetCode()
#include "pycore_long.h" // _PyLong_GetZero()
#include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing()
#include "pycore_pystate.h" // _PyThreadState_GET()
Expand Down
12 changes: 5 additions & 7 deletions Python/frame.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@

#define _PY_INTERPRETER

#include "Python.h"
#include "frameobject.h"
#include "pycore_code.h" // stats
#include "pycore_frame.h"
#include "pycore_genobject.h"
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
#include "opcode.h"
#include "pycore_frame.h" // _PyFrame_New_NoTrack()
#include "pycore_interpframe.h" // _PyFrame_GetCode()
#include "pycore_genobject.h" // _PyGen_GetGeneratorFromFrame()
#include "pycore_stackref.h" // _Py_VISIT_STACKREF()


int
_PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg)
Expand Down
9 changes: 4 additions & 5 deletions Python/intrinsics.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@
#define _PY_INTERPRETER

#include "Python.h"
#include "pycore_frame.h"
#include "pycore_function.h"
#include "pycore_global_objects.h"
#include "pycore_compile.h" // _PyCompile_GetUnaryIntrinsicName
#include "pycore_function.h" // _Py_set_function_type_params()
#include "pycore_genobject.h" // _PyAsyncGenValueWrapperNew
#include "pycore_compile.h" // _PyCompile_GetUnaryIntrinsicName, etc
#include "pycore_interpframe.h" // _PyFrame_GetLocals()
#include "pycore_intrinsics.h" // INTRINSIC_PRINT
#include "pycore_pyerrors.h" // _PyErr_SetString()
#include "pycore_runtime.h" // _Py_ID()
#include "pycore_sysmodule.h" // _PySys_GetRequiredAttr()
#include "pycore_tuple.h" // _PyTuple_FromArray()
#include "pycore_typevarobject.h" // _Py_make_typevar()
#include "pycore_unicodeobject.h" // _PyUnicode_FromASCII
#include "pycore_unicodeobject.h" // _PyUnicode_FromASCII()


/******** Unary functions ********/
Expand Down
Loading