STINNER Victor added the comment: > It looks like the struct of a dict is already arranged in a way that it could > switch to PyObject_VAR_HEAD
Can someone check if it has an impact of the size of the structure (because of the complex rules of alignment)? If the structure has the same size, I'm in favor of using PyObject_VAR_HEAD. I reviewed _PyDict_GET_SIZE-2.patch: I like the overall change, but I added many comments. When changes are not direct replacement ma_used => _PyDict_GET_SIZE(), please push the changes in a separated commit when you will push the whole change (I'm thinking to a micro-optimization in ceval.c). In typeobject.c, you added a check to test if *dictptr is a dict or not. This change seems strange to me. I'm not sure about the right behaviour here. You might open a separated issue for this change. I don't know if an exception should be raised or not. And my comments on the macro itself: Please add a tiny inline documentation: /* Get the number of items of a dictionary. */ I don't think that it's worth it to make the macro private, just name it PyDict_GET_SIZE(). The macro is not defined in the limited API, and we already expose like "everything" in the default API... No need to make the API harder to use. I was always surprised to have to check if the result if PyDict_Size() is negative and that no macro existed. Your change is a large and so hard to review. I would be more confident if you add an assertion here, as done in crazy macros of unicodeobject.h: #define _PyDict_GET_SIZE(mp) (assert(PyDict_Check(mp),Py_SIZE(mp)) We can add such assertion since it's a new macro. I'm not sure that we can do it in existing macros like PyTuple_GET_SIZE(), since these macros may be abused to modify the size, not only get it. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue28959> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
