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
@vstinner vstinner deleted the object_dictptr branch December 3, 2025 15:37
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