Skip to content

Commit 655d274

Browse files
committed
Reduce hashing API surface
Now, only `std::hash` is supported to hash `fine::Term` and `fine::Atom` data structures. Due to the way `fine::Atom` is implemented, we cannot delegate to `enif_hash` to hash an atom, but must instead hash the atom's string representation.
1 parent 7391800 commit 655d274

File tree

4 files changed

+30
-52
lines changed

4 files changed

+30
-52
lines changed

c_include/fine.hpp

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,6 @@ template <typename T> ERL_NIF_TERM encode(ErlNifEnv *env, const T &value);
4444
template <typename T, typename SFINAE = void> struct Decoder;
4545
template <typename T, typename SFINAE = void> struct Encoder;
4646

47-
enum class HashAlgorithm {
48-
INTERNAL = ERL_NIF_INTERNAL_HASH,
49-
PHASH2 = ERL_NIF_PHASH2,
50-
};
51-
52-
namespace __private__ {
53-
template <typename T> struct Hasher;
54-
}
55-
5647
namespace __private__ {
5748
std::vector<ErlNifFunc> &get_erl_nif_funcs();
5849
int load(ErlNifEnv *env, void **priv_data, ERL_NIF_TERM load_info);
@@ -74,13 +65,16 @@ inline ERL_NIF_TERM make_atom(ErlNifEnv *env, const char *msg) {
7465
// A representation of an atom term.
7566
class Atom {
7667
public:
77-
Atom(std::string name) : name(name), term(std::nullopt) {
68+
Atom(std::string name) : name(std::move(name)), term(std::nullopt) {
7869
if (!Atom::initialized) {
7970
Atom::atoms.push_back(this);
8071
}
8172
}
8273

83-
std::string to_string() const { return this->name; }
74+
public:
75+
const std::string &to_string() const & noexcept { return this->name; }
76+
77+
std::string to_string() && noexcept { return this->name; }
8478

8579
bool operator==(const Atom &other) const { return this->name == other.name; }
8680

@@ -99,7 +93,8 @@ class Atom {
9993
}
10094

10195
friend struct Encoder<Atom>;
102-
friend struct __private__::Hasher<Atom>;
96+
friend struct Decoder<Atom>;
97+
friend struct ::std::hash<Atom>;
10398

10499
friend int __private__::load(ErlNifEnv *env, void **priv_data,
105100
ERL_NIF_TERM load_info);
@@ -1066,34 +1061,6 @@ inline int load(ErlNifEnv *env, void **, ERL_NIF_TERM) {
10661061
}
10671062
} // namespace __private__
10681063

1069-
// Hash
1070-
namespace __private__ {
1071-
template <> struct Hasher<Atom> {
1072-
static std::uint64_t hash(HashAlgorithm algorithm, const Atom &atom,
1073-
std::uint64_t salt = 0) noexcept {
1074-
return enif_hash(static_cast<ErlNifHash>(algorithm), *atom.term, salt);
1075-
}
1076-
};
1077-
1078-
template <> struct Hasher<Term> {
1079-
static std::uint64_t hash(HashAlgorithm algorithm, const Term &term,
1080-
std::uint64_t salt = 0) noexcept {
1081-
return enif_hash(static_cast<ErlNifHash>(algorithm), term, salt);
1082-
}
1083-
};
1084-
} // namespace __private__
1085-
1086-
template <HashAlgorithm A = HashAlgorithm::PHASH2, typename T>
1087-
inline static std::uint64_t hash(const T &value, std::uint64_t salt = 0) {
1088-
return __private__::Hasher<T>::hash(A, value, salt);
1089-
}
1090-
1091-
template <typename T>
1092-
inline static std::uint64_t hash(HashAlgorithm algorithm, const T &value,
1093-
std::uint64_t salt = 0) {
1094-
return __private__::Hasher<T>::hash(algorithm, value, salt);
1095-
}
1096-
10971064
// Macros
10981065

10991066
#define FINE_NIF(name, flags) \
@@ -1154,13 +1121,13 @@ inline static std::uint64_t hash(HashAlgorithm algorithm, const T &value,
11541121
namespace std {
11551122
template <> struct hash<::fine::Term> {
11561123
size_t operator()(const ::fine::Term &term) noexcept {
1157-
return ::fine::hash<::fine::HashAlgorithm::PHASH2>(term);
1124+
return enif_hash(ERL_NIF_INTERNAL_HASH, term, 0);
11581125
}
11591126
};
11601127

11611128
template <> struct hash<::fine::Atom> {
1162-
size_t operator()(const ::fine::Term &term) noexcept {
1163-
return ::fine::hash<::fine::HashAlgorithm::PHASH2>(term);
1129+
size_t operator()(const ::fine::Atom &atom) noexcept {
1130+
return std::hash<std::string_view>{}(atom.to_string());
11641131
}
11651132
};
11661133
} // namespace std

test/c_src/finest.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include <cstring>
22
#include <exception>
3+
#include <functional>
34
#include <optional>
45
#include <stdexcept>
56
#include <thread>
@@ -295,12 +296,15 @@ std::nullopt_t shared_mutex_shared_lock_test(ErlNifEnv *) {
295296
}
296297
FINE_NIF(shared_mutex_shared_lock_test, 0);
297298

298-
std::uint64_t hash_test(ErlNifEnv *, fine::Term term) noexcept {
299-
// Ensure the use of PHASH2. INTERNAL is not guaranteed to be stable across
300-
// ERTS instances, even less so ERTS versions.
301-
return fine::hash<fine::HashAlgorithm::PHASH2>(term);
299+
std::uint64_t term_hash_test(ErlNifEnv *, fine::Term term) noexcept {
300+
return std::invoke(std::hash<fine::Term>{}, term);
302301
}
303-
FINE_NIF(hash_test, 0);
302+
FINE_NIF(term_hash_test, 0);
303+
304+
std::uint64_t atom_hash_test(ErlNifEnv *, fine::Atom atom) noexcept {
305+
return std::invoke(std::hash<fine::Atom>{}, atom);
306+
}
307+
FINE_NIF(atom_hash_test, 0);
304308

305309
} // namespace finest
306310

test/lib/finest/nif.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ defmodule Finest.NIF do
5656
def shared_mutex_unique_lock_test(), do: err!()
5757
def shared_mutex_shared_lock_test(), do: err!()
5858

59-
def hash_test(_term), do: err!()
59+
def term_hash_test(_term), do: err!()
60+
def atom_hash_test(_term), do: err!()
6061

6162
defp err!(), do: :erlang.nif_error(:not_loaded)
6263
end

test/test/finest_test.exs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,15 @@ defmodule FinestTest do
309309
end
310310

311311
describe "hash" do
312-
test "phash2" do
313-
for elem <- [42, "fine", ["it", %{"should" => {"just", "work"}}]] do
314-
assert NIF.hash_test(elem) == :erlang.phash2(elem)
312+
test "term" do
313+
for value <- [42, "fine", ["it", %{"should" => {"just", "work"}}], :atom] do
314+
assert NIF.term_hash_test(value) == NIF.term_hash_test(value)
315+
end
316+
end
317+
318+
test "atom" do
319+
for value <- [:ok, :error, :"with spaces", Enum, nil, true, false] do
320+
assert NIF.atom_hash_test(value) == NIF.atom_hash_test(value)
315321
end
316322
end
317323
end

0 commit comments

Comments
 (0)