Skip to content

Commit 52a3d3e

Browse files
committed
Revert "[cppyy] Fix hash collision in TemplateProxy dispatch for CPPOverload objects"
This reverts commit 87380b5.
1 parent a99d02e commit 52a3d3e

File tree

3 files changed

+24
-72
lines changed

3 files changed

+24
-72
lines changed

bindings/pyroot/cppyy/CPyCppyy/src/CPPOverload.cxx

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -17,63 +17,16 @@
1717
#include "CPPInstance.h"
1818
#include "CallContext.h"
1919
#include "PyStrings.h"
20-
#include "TemplateProxy.h"
2120
#include "Utility.h"
2221

2322
// Standard
2423
#include <algorithm>
25-
#include <functional>
2624
#include <sstream>
2725
#include <vector>
2826

2927

3028
namespace CPyCppyy {
3129

32-
//----------------------------------------------------------------------------
33-
// Combine name hash and method count for stable identity
34-
35-
namespace {
36-
constexpr uint64_t kGoldenRatioHash = 0x9e3779b9ULL;
37-
38-
inline void hash_combine(uint64_t &seed, uint64_t value)
39-
{
40-
seed ^= value + kGoldenRatioHash + (seed << 6) + (seed >> 2);
41-
}
42-
} // anonymous namespace
43-
44-
uint64_t HashSignature(CPyCppyy_PyArgs_t args, size_t nargsf)
45-
{
46-
uint64_t hash = 0;
47-
std::hash<std::string> str_hash;
48-
49-
Py_ssize_t nargs = CPyCppyy_PyArgs_GET_SIZE(args, nargsf);
50-
for (Py_ssize_t i = 0; i < nargs; ++i) {
51-
PyObject *pyobj = CPyCppyy_PyArgs_GET_ITEM(args, i);
52-
53-
if (CPPOverload_Check(pyobj)) {
54-
// Hash the method name and overload count for uniqueness
55-
// All O(1) access - no RTTI, no string construction
56-
auto *ol = static_cast<CPPOverload *>(pyobj);
57-
hash_combine(hash, str_hash(ol->fMethodInfo->fName));
58-
hash_combine(hash, static_cast<uint64_t>(ol->fMethodInfo->fMethods.size()));
59-
} else if (TemplateProxy_Check(pyobj)) {
60-
// Hash the stable template name (fCppName includes scope for templates)
61-
auto *tp = static_cast<TemplateProxy *>(pyobj);
62-
hash_combine(hash, str_hash(tp->fTI->fCppName));
63-
} else {
64-
// Standard type-based hashing for other objects
65-
hash_combine(hash, reinterpret_cast<uint64_t>(Py_TYPE(pyobj)));
66-
#if PY_VERSION_HEX >= 0x030e0000
67-
hash_combine(hash, PyUnstable_Object_IsUniqueReferencedTemporary(pyobj) ? 1ULL : 0ULL);
68-
#else
69-
hash_combine(hash, Py_REFCNT(pyobj) == 1 ? 1ULL : 0ULL);
70-
#endif
71-
}
72-
}
73-
74-
return hash;
75-
}
76-
7730
namespace {
7831

7932
// from CPython's instancemethod: Free list for method objects to safe malloc/free overhead

bindings/pyroot/cppyy/CPyCppyy/src/CPPOverload.h

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#include "PyCallable.h"
66

77
// Standard
8-
#include <cstdint>
98
#include <map>
109
#include <string>
1110
#include <utility>
@@ -14,8 +13,30 @@
1413

1514
namespace CPyCppyy {
1615

17-
// Signature hashing for memoization (implementation in CPPOverload.cxx)
18-
uint64_t HashSignature(CPyCppyy_PyArgs_t args, size_t nargsf);
16+
// signature hashes are also used by TemplateProxy
17+
inline uint64_t HashSignature(CPyCppyy_PyArgs_t args, size_t nargsf)
18+
{
19+
// Build a hash from the types of the given python function arguments.
20+
uint64_t hash = 0;
21+
22+
Py_ssize_t nargs = CPyCppyy_PyArgs_GET_SIZE(args, nargsf);
23+
for (Py_ssize_t i = 0; i < nargs; ++i) {
24+
// TODO: hashing in the ref-count is for moves; resolve this together with the
25+
// improved overloads for implicit conversions
26+
PyObject* pyobj = CPyCppyy_PyArgs_GET_ITEM(args, i);
27+
hash += (uint64_t)Py_TYPE(pyobj);
28+
#if PY_VERSION_HEX >= 0x030e0000
29+
hash += (uint64_t)(PyUnstable_Object_IsUniqueReferencedTemporary(pyobj) ? 1 : 0);
30+
#else
31+
hash += (uint64_t)(Py_REFCNT(pyobj) == 1 ? 1 : 0);
32+
#endif
33+
hash += (hash << 10); hash ^= (hash >> 6);
34+
}
35+
36+
hash += (hash << 3); hash ^= (hash >> 11); hash += (hash << 15);
37+
38+
return hash;
39+
}
1940

2041
class CPPOverload {
2142
public:

bindings/pyroot/cppyy/cppyy/test/test_templates.py

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,28 +1159,6 @@ def test34_cstring_template_argument(self):
11591159
assert ns.stringify["const char*"]("Aap") == "Aap "
11601160
assert ns.stringify(ctypes.c_char_p(bytes("Noot", "ascii"))) == "Noot "
11611161

1162-
def test35_issue_20526_hash_collision(self):
1163-
"""Fix #20526: Ensure distinct global functions have distinct hashes"""
1164-
1165-
import cppyy
1166-
1167-
cppyy.cppdef("""
1168-
namespace Issue20526 {
1169-
int funcA() { return 101; }
1170-
int funcB() { return 202; }
1171-
template<typename T> int call_it(T f) { return f(); }
1172-
}
1173-
""")
1174-
1175-
ns = cppyy.gbl.Issue20526
1176-
1177-
# If hashes collide, the cache returns the wrong instantiation
1178-
resA = ns.call_it(ns.funcA)
1179-
resB = ns.call_it(ns.funcB)
1180-
1181-
assert resA == 101
1182-
assert resB == 202
1183-
11841162

11851163
class TestTEMPLATED_TYPEDEFS:
11861164
def setup_class(cls):

0 commit comments

Comments
 (0)