gh-111545: Add Py_HashPointer() function#112096
Conversation
eaf1414 to
3a0752a
Compare
5b19577 to
d132eb6
Compare
992c39e to
f63b726
Compare
Adding |
f63b726 to
fb08414
Compare
|
Changes:
|
fb08414 to
ffbdae1
Compare
|
@encukou: Please review the updated PR. |
ffbdae1 to
cd8849b
Compare
|
@serhiy-storchaka: Do you want to review this change? |
136b536 to
8031210
Compare
|
I rebased the PR on main to get a NoGIL fix. |
PEP 731 says:
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. |
No, I'm saying that the others might disagree -- and that I don't think it's very likely that they will disagree. |
zooba
left a comment
There was a problem hiding this comment.
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...)
|
|
||
| .. c:function:: Py_hash_t Py_HashPointer(const void *ptr) | ||
|
|
||
| Hash a pointer. |
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
I completed the doc. Is it more explicit? I clarified that only the pointer value is hashed.
* Implement _Py_HashPointerRaw() as a static inline function. * Add Py_HashPointer() tests to test_capi.test_hash. * Keep _Py_HashPointer() function as an alias to Py_HashPointer().
8031210 to
5415271
Compare
@encukou created capi-workgroup/decisions#1 |
| ``uintptr_t`` internally). The pointer is not dereferenced. | ||
|
|
||
| The function cannot fail: it cannot return ``-1``. | ||
|
|
There was a problem hiding this comment.
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 Moreover, I removed the private function
Finally, igraph-0.11.2 contains a copy of the private |
* Implement _Py_HashPointerRaw() as a static inline function. * Add Py_HashPointer() tests to test_capi.test_hash. * Keep _Py_HashPointer() function as an alias to Py_HashPointer().
* Implement _Py_HashPointerRaw() as a static inline function. * Add Py_HashPointer() tests to test_capi.test_hash. * Keep _Py_HashPointer() function as an alias to Py_HashPointer().

_Py_HashDoublepublic again as "unstable" API #111545📚 Documentation preview 📚: https://cpython-previews--112096.org.readthedocs.build/