FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-139852: Add PyObject_GetDictPtr() function by vstinner · Pull Request #139854 · python/cpython · GitHub

Repository navigation

gh-139852: Add PyObject_GetDictPtr() function - #139854

Closed
vstinner wants to merge 4 commits into
python:mainfrom
vstinner:object_dictptr
Closed

vstinner wants to merge 4 commits into
python:mainfrom
vstinner:object_dictptr

Conversation

vstinner commented Oct 9, 2025 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

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 commented Oct 9, 2025

Copy link
Copy Markdown
Member Author

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 commented Oct 9, 2025

Copy link
Copy Markdown
Member Author

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

encukou commented Oct 10, 2025

Copy link
Copy Markdown
Member

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**?

Comment thread Doc/c-api/object.rst

Copy link
Copy Markdown
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);

Copy link
Copy Markdown
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.

Comment thread Objects/object.c
@@ -1550,18 +1571,12 @@ _PyObject_ComputedDictPointer(PyObject *obj)
PyObject **
_PyObject_GetDictPtr(PyObject *obj)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should this be deprecated?

Copy link
Copy Markdown
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

Copy link
Copy Markdown
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 closed this Oct 11, 2025
vstinner deleted the object_dictptr branch December 3, 2025 15:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL