Skip to content

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 9, 2025

@vstinner vstinner added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API labels Oct 9, 2025
@vstinner
Copy link
Member Author

vstinner commented Oct 9, 2025

Check labels / Unresolved review (pull_request): Failing after 7s

The job fails with:

Error: Label error. Requires exactly 1 of: awaiting merge. Found: type-feature, awaiting core review, interpreter-core, topic-C-API

I don't understand this error. Let me try to close/reopen the PR to try to repair the CI.

@vstinner
Copy link
Member Author

vstinner commented Oct 9, 2025

I added the awaiting merge label to work around the CI issue.

@encukou
Copy link
Member

encukou commented Oct 10, 2025

Can those users switch to PyObject_GenericGetDict, or be better served by a variant of PyObject_GenericGetDict that doesn't raise for the “no __dict__” case?
Do they need a PyObject**?

@vstinner
Copy link
Member Author

Can those users switch to PyObject_GenericGetDict, or be better served by a variant of PyObject_GenericGetDict that doesn't raise for the “no __dict__” case? Do they need a PyObject**?

It seems like PyObject** is needed.

SWIG generates code which writes into the PyObject **dict_ptr:

#if !defined(SWIG_PYTHON_SLOW_GETSET_THIS)
      PyObject **dictptr = _PyObject_GetDictPtr(inst);
      if (dictptr != NULL) {
        PyObject *dict = *dictptr;
        if (dict == NULL) {
          dict = PyDict_New();
          *dictptr = dict;
        }
        if (dict) {
          PyDict_SetItem(dict, SWIG_This(), swig_this);
        } else{
          Py_DECREF(inst);
          inst = 0;
        }
      }
#else
      if (PyObject_SetAttr(inst, SWIG_This(), swig_this) == -1) {
        Py_DECREF(inst);
        inst = 0;
      }
#endif

pybind11 also writes into the PyObject **dict_ptr:

    PyObject **dict_ptr = _PyObject_GetDictPtr(self);
    if (dict_ptr) {
        Py_CLEAR(*dict_ptr);
    }

Obviously, there are other usages which only read the dictionary. Example from pybind11:

    PyObject *&dict = *_PyObject_GetDictPtr(self);
    Py_VISIT(dict);

@ZeroIntensity
Copy link
Member

I added the awaiting merge label to work around the CI issue.

That comes up because of the type-feature label, because of the plan to make core review required for new features, but I don't know what happened to that.

* so it should be treated as part of the public API.
*/
PyObject **
_PyObject_GetDictPtr(PyObject *obj)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be deprecated?

@colesbury
Copy link
Contributor

SWIG generates code which writes into the PyObject **dict_ptr:

I don't think Swig uses that code path for Python 3:

https://github.com/swig/swig/blob/df3eec20fa56d43bfc0614bc795b39d51964b894/Lib/python/pyrun.swg#L1407-L1412

@vstinner
Copy link
Member Author

Making this API a public function seems to be a bad idea: #139852 (comment). I prefer to close this PR for now.

I proposed #139968 to add PyObject_GetDict() function instead.

@vstinner vstinner closed this Oct 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review awaiting merge interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants