Skip to content

Commit d01f3e4

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 6ebc546 commit d01f3e4

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);
@@ -1081,34 +1076,6 @@ inline int load(ErlNifEnv *env, void **, ERL_NIF_TERM) {
10811076
}
10821077
} // namespace __private__
10831078

1084-
// Hash
1085-
namespace __private__ {
1086-
template <> struct Hasher<Atom> {
1087-
static std::uint64_t hash(HashAlgorithm algorithm, const Atom &atom,
1088-
std::uint64_t salt = 0) noexcept {
1089-
return enif_hash(static_cast<ErlNifHash>(algorithm), *atom.term, salt);
1090-
}
1091-
};
1092-
1093-
template <> struct Hasher<Term> {
1094-
static std::uint64_t hash(HashAlgorithm algorithm, const Term &term,
1095-
std::uint64_t salt = 0) noexcept {
1096-
return enif_hash(static_cast<ErlNifHash>(algorithm), term, salt);
1097-
}
1098-
};
1099-
} // namespace __private__
1100-
1101-
template <HashAlgorithm A = HashAlgorithm::PHASH2, typename T>
1102-
inline static std::uint64_t hash(const T &value, std::uint64_t salt = 0) {
1103-
return __private__::Hasher<T>::hash(A, value, salt);
1104-
}
1105-
1106-
template <typename T>
1107-
inline static std::uint64_t hash(HashAlgorithm algorithm, const T &value,
1108-
std::uint64_t salt = 0) {
1109-
return __private__::Hasher<T>::hash(algorithm, value, salt);
1110-
}
1111-
11121079
// Macros
11131080

11141081
#define FINE_NIF(name, flags) \
@@ -1169,13 +1136,13 @@ inline static std::uint64_t hash(HashAlgorithm algorithm, const T &value,
11691136
namespace std {
11701137
template <> struct hash<::fine::Term> {
11711138
size_t operator()(const ::fine::Term &term) noexcept {
1172-
return ::fine::hash<::fine::HashAlgorithm::PHASH2>(term);
1139+
return enif_hash(ERL_NIF_INTERNAL_HASH, term, 0);
11731140
}
11741141
};
11751142

11761143
template <> struct hash<::fine::Atom> {
1177-
size_t operator()(const ::fine::Term &term) noexcept {
1178-
return ::fine::hash<::fine::HashAlgorithm::PHASH2>(term);
1144+
size_t operator()(const ::fine::Atom &atom) noexcept {
1145+
return std::hash<std::string_view>{}(atom.to_string());
11791146
}
11801147
};
11811148
} // 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 <memory_resource>
45
#include <optional>
56
#include <stdexcept>
@@ -325,12 +326,15 @@ std::nullopt_t shared_mutex_shared_lock_test(ErlNifEnv *) {
325326
}
326327
FINE_NIF(shared_mutex_shared_lock_test, 0);
327328

328-
std::uint64_t hash_test(ErlNifEnv *, fine::Term term) noexcept {
329-
// Ensure the use of PHASH2. INTERNAL is not guaranteed to be stable across
330-
// ERTS instances, even less so ERTS versions.
331-
return fine::hash<fine::HashAlgorithm::PHASH2>(term);
329+
std::uint64_t term_hash_test(ErlNifEnv *, fine::Term term) noexcept {
330+
return std::invoke(std::hash<fine::Term>{}, term);
332331
}
333-
FINE_NIF(hash_test, 0);
332+
FINE_NIF(term_hash_test, 0);
333+
334+
std::uint64_t atom_hash_test(ErlNifEnv *, fine::Atom atom) noexcept {
335+
return std::invoke(std::hash<fine::Atom>{}, atom);
336+
}
337+
FINE_NIF(atom_hash_test, 0);
334338

335339
} // namespace finest
336340

test/lib/finest/nif.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ defmodule Finest.NIF do
5959
def shared_mutex_unique_lock_test(), do: err!()
6060
def shared_mutex_shared_lock_test(), do: err!()
6161

62-
def hash_test(_term), do: err!()
62+
def term_hash_test(_term), do: err!()
63+
def atom_hash_test(_term), do: err!()
6364

6465
defp err!(), do: :erlang.nif_error(:not_loaded)
6566
end

test/test/finest_test.exs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,15 @@ defmodule FinestTest do
315315
end
316316

317317
describe "hash" do
318-
test "phash2" do
319-
for elem <- [42, "fine", ["it", %{"should" => {"just", "work"}}]] do
320-
assert NIF.hash_test(elem) == :erlang.phash2(elem)
318+
test "term" do
319+
for value <- [42, "fine", ["it", %{"should" => {"just", "work"}}], :atom] do
320+
assert NIF.term_hash_test(value) == NIF.term_hash_test(value)
321+
end
322+
end
323+
324+
test "atom" do
325+
for value <- [:ok, :error, :"with spaces", Enum, nil, true, false] do
326+
assert NIF.atom_hash_test(value) == NIF.atom_hash_test(value)
321327
end
322328
end
323329
end

0 commit comments

Comments
 (0)