Skip to content

Commit 17a2090

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 186edd1 commit 17a2090

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);
@@ -1111,34 +1106,6 @@ inline int load(ErlNifEnv *env, void **, ERL_NIF_TERM) {
11111106
}
11121107
} // namespace __private__
11131108

1114-
// Hash
1115-
namespace __private__ {
1116-
template <> struct Hasher<Atom> {
1117-
static std::uint64_t hash(HashAlgorithm algorithm, const Atom &atom,
1118-
std::uint64_t salt = 0) noexcept {
1119-
return enif_hash(static_cast<ErlNifHash>(algorithm), *atom.term, salt);
1120-
}
1121-
};
1122-
1123-
template <> struct Hasher<Term> {
1124-
static std::uint64_t hash(HashAlgorithm algorithm, const Term &term,
1125-
std::uint64_t salt = 0) noexcept {
1126-
return enif_hash(static_cast<ErlNifHash>(algorithm), term, salt);
1127-
}
1128-
};
1129-
} // namespace __private__
1130-
1131-
template <HashAlgorithm A = HashAlgorithm::PHASH2, typename T>
1132-
inline static std::uint64_t hash(const T &value, std::uint64_t salt = 0) {
1133-
return __private__::Hasher<T>::hash(A, value, salt);
1134-
}
1135-
1136-
template <typename T>
1137-
inline static std::uint64_t hash(HashAlgorithm algorithm, const T &value,
1138-
std::uint64_t salt = 0) {
1139-
return __private__::Hasher<T>::hash(algorithm, value, salt);
1140-
}
1141-
11421109
// Macros
11431110

11441111
#define FINE_NIF(name, flags) \
@@ -1199,13 +1166,13 @@ inline static std::uint64_t hash(HashAlgorithm algorithm, const T &value,
11991166
namespace std {
12001167
template <> struct hash<::fine::Term> {
12011168
size_t operator()(const ::fine::Term &term) noexcept {
1202-
return ::fine::hash<::fine::HashAlgorithm::PHASH2>(term);
1169+
return enif_hash(ERL_NIF_INTERNAL_HASH, term, 0);
12031170
}
12041171
};
12051172

12061173
template <> struct hash<::fine::Atom> {
1207-
size_t operator()(const ::fine::Term &term) noexcept {
1208-
return ::fine::hash<::fine::HashAlgorithm::PHASH2>(term);
1174+
size_t operator()(const ::fine::Atom &atom) noexcept {
1175+
return std::hash<std::string_view>{}(atom.to_string());
12091176
}
12101177
};
12111178
} // 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>
@@ -355,12 +356,15 @@ bool compare_ge(ErlNifEnv *, fine::Term lhs, fine::Term rhs) noexcept {
355356
}
356357
FINE_NIF(compare_ge, 0);
357358

358-
std::uint64_t hash_test(ErlNifEnv *, fine::Term term) noexcept {
359-
// Ensure the use of PHASH2. INTERNAL is not guaranteed to be stable across
360-
// ERTS instances, even less so ERTS versions.
361-
return fine::hash<fine::HashAlgorithm::PHASH2>(term);
359+
std::uint64_t term_hash_test(ErlNifEnv *, fine::Term term) noexcept {
360+
return std::invoke(std::hash<fine::Term>{}, term);
362361
}
363-
FINE_NIF(hash_test, 0);
362+
FINE_NIF(term_hash_test, 0);
363+
364+
std::uint64_t atom_hash_test(ErlNifEnv *, fine::Atom atom) noexcept {
365+
return std::invoke(std::hash<fine::Atom>{}, atom);
366+
}
367+
FINE_NIF(atom_hash_test, 0);
364368

365369
} // namespace finest
366370

test/lib/finest/nif.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ defmodule Finest.NIF do
6666
def compare_gt(_lhs, _rhs), do: err!()
6767
def compare_ge(_lhs, _rhs), do: err!()
6868

69-
def hash_test(_term), do: err!()
69+
def term_hash_test(_term), do: err!()
70+
def atom_hash_test(_term), do: err!()
7071

7172
defp err!(), do: :erlang.nif_error(:not_loaded)
7273
end

test/test/finest_test.exs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,15 @@ defmodule FinestTest do
353353
end
354354

355355
describe "hash" do
356-
test "phash2" do
357-
for elem <- [42, "fine", ["it", %{"should" => {"just", "work"}}]] do
358-
assert NIF.hash_test(elem) == :erlang.phash2(elem)
356+
test "term" do
357+
for value <- [42, "fine", ["it", %{"should" => {"just", "work"}}], :atom] do
358+
assert NIF.term_hash_test(value) == NIF.term_hash_test(value)
359+
end
360+
end
361+
362+
test "atom" do
363+
for value <- [:ok, :error, :"with spaces", Enum, nil, true, false] do
364+
assert NIF.atom_hash_test(value) == NIF.atom_hash_test(value)
359365
end
360366
end
361367
end

0 commit comments

Comments
 (0)