Skip to content

Commit

Permalink
Remove use of nullptr static cast
Browse files Browse the repository at this point in the history
We used some shady cast that involved casting a nullptr, in order to
pass lambdas to an object and keep it copy-assignable/copyable. Instead,
I now store the function objects in the parent object. This means that
if we simply pass a lambda, the struct won't be copyable. The solution
for that is to append + in front of the lambda, which casts it into a
function pointer.
  • Loading branch information
elshize committed Jan 20, 2023
1 parent 427d208 commit 22b371b
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 15 deletions.
4 changes: 2 additions & 2 deletions include/pisa/codec/strict_elias_fano.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ struct strict_elias_fano {
using value_type = typename std::iterator_traits<Iterator>::value_type;
auto new_begin = make_function_iterator(
std::make_pair(value_type(0), begin),
[](std::pair<value_type, Iterator>& state) {
+[](std::pair<value_type, Iterator>& state) {
++state.first;
++state.second;
},
[](std::pair<value_type, Iterator> const& state) { return *state.second - state.first; });
+[](std::pair<value_type, Iterator> const& state) { return *state.second - state.first; });
compact_elias_fano::write(bvb, new_begin, new_universe, n, params);
}

Expand Down
4 changes: 2 additions & 2 deletions include/pisa/sequence/positive_sequence.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ struct positive_sequence {
assert(n > 0);
auto cumulative_begin = make_function_iterator(
std::make_pair(uint64_t(0), begin),
[](std::pair<uint64_t, Iterator>& state) { state.first += *state.second++; },
[](std::pair<uint64_t, Iterator> const& state) { return state.first + *state.second; });
+[](std::pair<uint64_t, Iterator>& state) { state.first += *state.second++; },
+[](std::pair<uint64_t, Iterator> const& state) { return state.first + *state.second; });
base_sequence_type::write(bvb, cumulative_begin, universe, n, params);
}

Expand Down
27 changes: 16 additions & 11 deletions include/pisa/util/util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,24 @@ class function_iterator {

function_iterator() = default;

explicit function_iterator(State initial_state) : m_state(initial_state) {}
explicit function_iterator(
State&& initial_state, AdvanceFunctor&& advance_functor, ValueFunctor&& value_functor)
: m_state(std::forward<State>(initial_state)),
m_advance_functor(std::forward<AdvanceFunctor>(advance_functor)),
m_value_functor(std::forward<ValueFunctor>(value_functor))
{}

friend inline void swap(function_iterator& lhs, function_iterator& rhs)
{
using std::swap;
swap(lhs.m_state, rhs.m_state);
}

value_type operator*() const
{
// XXX I do not know if this trick is legal for stateless lambdas,
// but it seems to work on GCC and Clang
return (*static_cast<ValueFunctor*>(nullptr))(m_state);
}
value_type operator*() const { return m_value_functor(m_state); }

function_iterator& operator++()
{
(*static_cast<AdvanceFunctor*>(nullptr))(m_state);
m_advance_functor(m_state);
return *this;
}

Expand All @@ -106,13 +106,18 @@ class function_iterator {

private:
State m_state;
AdvanceFunctor m_advance_functor;
ValueFunctor m_value_functor;
};

template <typename State, typename AdvanceFunctor, typename ValueFunctor>
function_iterator<State, AdvanceFunctor, ValueFunctor>
make_function_iterator(State initial_state, AdvanceFunctor, ValueFunctor)
function_iterator<State, AdvanceFunctor, ValueFunctor> make_function_iterator(
State&& initial_state, AdvanceFunctor&& advance_functor, ValueFunctor&& value_functor)
{
return function_iterator<State, AdvanceFunctor, ValueFunctor>(initial_state);
return function_iterator<State, AdvanceFunctor, ValueFunctor>(
std::forward<State>(initial_state),
std::forward<AdvanceFunctor>(advance_functor),
std::forward<ValueFunctor>(value_functor));
}

struct stats_line {
Expand Down

0 comments on commit 22b371b

Please sign in to comment.