gh-111545: Add Py_HashPointer() function

[vstinner]

• Keep _Py_HashPointer() function as an alias to PyHash_Pointer().
• Add tests on PyHash_Pointer() to test_capi.test_hash.
• Remove _Py_HashPointerRaw() function: inline code in _Py_hashtable_hash_ptr().

• Issue: Make _Py_HashDouble public again as "unstable" API #111545

---

📚 Documentation preview 📚: https://cpython-previews--112096.org.readthedocs.build/

[vstinner]

I checked how _Py_rotateright_uintptr() is compiled on x86-64:

• clang -O3 with __builtin_rotateright64(): ror rax, cl
• gcc -O3 with (x >> bits) | (x << (8 * SIZEOF_UINTPTR_T - bits)): ror rbx,cl

As expected, the ROR instruction is used in both cases.

---

I used gdb: in Python, I run import _testinternalcapi, and then in gdb I type disassemble rotateright_uintptr.

For that, I made the following change. The check_rotateright_uintptr() cannot be used to check the machine code: since check_rotateright_uintptr() arguments are constant, GCC and clang just pre-compute the function result at build. The funciton is no longer called at runtime.

Change:

diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
index 2d888e0ae2c..a08b3dfb168 100644
--- a/Modules/_testinternalcapi.c
+++ b/Modules/_testinternalcapi.c
@@ -290,6 +290,23 @@ test_rotateright_uintptr(PyObject *self, PyObject *Py_UNUSED(args))
 }
 
 
+static PyObject*
+rotateright_uintptr(PyObject *self, PyObject *args)
+{
+    PyObject *obj;
+    int bits;
+    if (!PyArg_ParseTuple(args, "O!i", &PyLong_Type, &obj, &bits)) {
+        return NULL;
+    }
+    unsigned long long x = PyLong_AsUnsignedLongLong(obj);
+    if (x == (unsigned long long)-1 && PyErr_Occurred()) {
+        return NULL;
+    }
+    uintptr_t y = _Py_rotateright_uintptr((uintptr_t)x, bits);
+    return PyLong_FromUnsignedLongLong(y);
+}
+
+
 #define TO_PTR(ch) ((void*)(uintptr_t)ch)
 #define FROM_PTR(ptr) ((uintptr_t)ptr)
 #define VALUE(key) (1 + ((int)(key) - 'a'))
@@ -1689,6 +1706,7 @@ static PyMethodDef module_functions[] = {
     {"test_popcount", test_popcount, METH_NOARGS},
     {"test_bit_length", test_bit_length, METH_NOARGS},
     {"test_rotateright_uintptr", test_rotateright_uintptr, METH_NOARGS},
+    {"rotateright_uintptr", rotateright_uintptr, METH_VARARGS},
     {"test_hashtable", test_hashtable, METH_NOARGS},
     {"get_config", test_get_config, METH_NOARGS},
     {"set_config", test_set_config, METH_O},

[vstinner]

I checked how _Py_rotateright_uintptr() is compiled on x86-64

Adding _Py_rotateright_uintptr() static inline function added too much code, and it's not needed: current code is already compiled to the expected most efficient x86-64 ROR instruction. I removed the _Py_rotateright_uintptr() function.

[vstinner]

Changes:

• PR rebased on main.
• Rename the function to Py_HashPointer() following gh-111545: Add PyHash_Double() function #112095 (comment) discussion.
• Keep _Py_HashPointerRaw() as a static inline function to avoid code duplication.
• No longer use SIZEOF_UINTPTR_T, but sizeof().

[vstinner]

@encukou: Please review the updated PR.

[encukou]

This looks great to me, thank you for adding it!

I hope Serhiy is OK with this iteration. (And that the rest of the C-API WG won't have major objections, but Idon't think that's likely.)

[vstinner]

@serhiy-storchaka: Do you want to review this change?

[vstinner]

I rebased the PR on main to get a NoGIL fix.

[vstinner]

@encukou:

(And that the rest of the C-API WG won't have major objections, but Idon't think that's likely.)

PEP 731 says:

The working group may operate primarily through reviews of GitHub issues and PRs.

Are you suggesting that the C API cannot be modified anymore unless whole C API Working Group approve a change? Or are you suggesting that the C API Working Group must review all C API additions? I don't think that was the plan. Well, the plan is not well defined.

[encukou]

Are you suggesting that the C API cannot be modified anymore unless whole C API Working Group approve a change?

No, I'm saying that the others might disagree -- and that I don't think it's very likely that they will disagree.
I'd optimistically merge this (edit: after checking with Serhiy), but be prepared to fix it up or revert.

[zooba]

No concerns from me, other than the lack of docs.

Adding new C API should include the WG up until we've developed and agreed upon the guidelines for adding new C API. Without those, yeah, we should all get pinged for any new APIs. (And if that takes up all our time and we don't get a chance to develop guidelines, so be it...)

[zooba]

Can we add a bit more text to clarify that it ensures the pointer itself is usable as a hash value, and does not even attempt to access the memory the pointer refers to (let alone trying to call __hash__ or similar)?

[vstinner]

I completed the doc. Is it more explicit? I clarified that only the pointer value is hashed.

[vstinner]

Adding new C API should include the WG up until we've developed and agreed upon the guidelines for adding new C API.

@encukou created capi-workgroup/decisions#1

[iritkatriel]

I'm missing some mention of the use case for this (not necessarily here). The issue talks about _Py_HashDouble, there is no mention of Py_HashPointer.

[vstinner]

@iritkatriel:

I'm missing some mention of the use case for this (not necessarily here). The issue talks about _Py_HashDouble, there is no mention of Py_HashPointer.

Proposed Py_HashDouble() API requires the caller to call Py_HashPointer(obj) for NaN: see #112449

Moreover, I removed the private function _Py_HashPointer() in Python 3.13 alpha1 and it broke a few projects. A code search on PyPI top 5,000 projects found 4 projects:

• cffi (1.16.0)
• gmpy2 (2.1.5)
• numba (0.58.1)
• tree_sitter (0.20.4)

Finally, igraph-0.11.2 contains a copy of the private _Py_HashPointer() function in src/_igraph/pyhelpers.c.

[vstinner]

I merged my PR. The API was approved by the C API Working Group: capi-workgroup/decisions#1