Skip to content

Modernize internal implementation: iterator facade deducing this #1148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Oct 13, 2025
Merged

Modernize internal implementation: iterator facade deducing this #1148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0cd4dee
rewrite most iterator facade stuff to deducing this
mgovers Sep 26, 2025
9126a83
also clean-up three-way-comparison
mgovers Oct 2, 2025
fb40667
direct iterator dereference
mgovers Oct 2, 2025
07c1eca
expose three-way directly
mgovers Oct 2, 2025
479df2d
remove CRTP part of iterator facade
mgovers Oct 2, 2025
0ecdd54
delete incorrect constructors
mgovers Oct 2, 2025
f177f2b
fully deducing this
mgovers Oct 3, 2025
eecc55e
tests for iterator facade
mgovers Oct 3, 2025
2dba904
fix requires clause
mgovers Oct 6, 2025
7fc378b
fix other compilation errors
mgovers Oct 6, 2025
fad5ae9
constexpr iterator facade constructor
mgovers Oct 7, 2025
8a36a30
fix sonar cloud
mgovers Oct 7, 2025
698b5cd
fix clang-tidy
mgovers Oct 7, 2025
d0690b5
more sonar cloud
mgovers Oct 7, 2025
ef80b20
let iterator facade use only public functions; declare compound and d…
mgovers Oct 8, 2025
bc61883
fix typo
mgovers Oct 8, 2025
42af8b0
give up on increment/decrement: requires friend class
mgovers Oct 8, 2025
d9df47a
fix sonar + clang-tidy
mgovers Oct 8, 2025
4185c7d
fix
mgovers Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions power_grid_model_c/power_grid_model/include/power_grid_model/auxiliary/dataset.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,28 +117,38 @@ class ColumnarAttributeRange : public std::ranges::view_interface<ColumnarAttrib
std::span<AttributeBuffer<Data> const> attribute_buffers_{};
};

class iterator
: public IteratorFacade<iterator, std::conditional_t<is_data_mutable_v<dataset_type>, Proxy, Proxy const>,
Idx> {
class iterator : public IteratorFacade {
public:
using value_type = Proxy;
using value_type = std::conditional_t<is_data_mutable_v<dataset_type>, Proxy, Proxy const>;
using difference_type = Idx;
using pointer = std::add_pointer_t<value_type>;
using reference = std::add_lvalue_reference_t<value_type>;

iterator() = default;
iterator() : IteratorFacade{*this} {};
iterator(difference_type idx, std::span<AttributeBuffer<Data> const> attribute_buffers)
: current_{idx, attribute_buffers} {}
: IteratorFacade{*this}, current_{idx, attribute_buffers} {}

private:
friend class IteratorFacade<iterator, std::conditional_t<is_data_mutable_v<dataset_type>, Proxy, Proxy const>,
Idx>;

constexpr auto dereference() -> value_type& { return current_; }
constexpr auto dereference() const -> std::add_lvalue_reference_t<std::add_const_t<value_type>> {
constexpr auto operator*() -> reference { return current_; }
constexpr auto operator*() const -> std::add_lvalue_reference_t<std::add_const_t<value_type>> {
return current_;
}
constexpr auto three_way_compare(iterator const& other) const { return current_.idx_ <=> other.current_.idx_; }
constexpr auto distance_to(iterator const& other) const { return other.current_.idx_ - current_.idx_; }
constexpr void advance(difference_type n) { current_.idx_ += n; }

friend constexpr auto operator<=>(iterator const& first, iterator const& second) {
return first.current_idx() <=> second.current_idx();
}

constexpr auto operator+=(difference_type n) -> std::add_lvalue_reference_t<iterator> {
current_idx() += n;
return *this;
}

friend constexpr auto operator-(iterator const& first, iterator const& second) -> difference_type {
return first.current_idx() - second.current_idx();
}

private:
constexpr auto current_idx() const { return current_.idx_; }
constexpr auto& current_idx() { return current_.idx_; }

Proxy current_;
};
Expand Down
122 changes: 72 additions & 50 deletions power_grid_model_c/power_grid_model/include/power_grid_model/common/grouped_index_vector.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,26 +94,20 @@ constexpr auto from_dense = from_dense_t{};

class SparseGroupedIdxVector {
private:
class GroupIterator : public IteratorFacade<GroupIterator, IdxRange const, Idx> {
friend class IteratorFacade<GroupIterator, IdxRange const, Idx>;
using Base = IteratorFacade<GroupIterator, IdxRange const, Idx>;

class GroupIterator : public IteratorFacade {
public:
using value_type = typename Base::value_type;
using difference_type = typename Base::difference_type;
using reference = typename Base::reference;

GroupIterator() = default;
explicit constexpr GroupIterator(IdxVector const& indptr, Idx group) : indptr_{&indptr}, group_{group} {}

private:
IdxVector const* indptr_{};
Idx group_{};
mutable value_type latest_value_{}; // making this mutable allows us to delay out-of-bounds checks until
// dereferencing instead of update methods. Note that the value will be
// invalidated at first update

constexpr auto dereference() const -> reference {
using iterator = GroupIterator;
using const_iterator = std::add_const_t<GroupIterator>;
using value_type = IdxRange const;
using difference_type = Idx;
using pointer = std::add_pointer_t<value_type>;
using reference = std::add_lvalue_reference_t<value_type>;

GroupIterator() : IteratorFacade{*this} {};
explicit constexpr GroupIterator(IdxVector const& indptr, Idx group)
: IteratorFacade{*this}, indptr_{&indptr}, group_{group} {}

constexpr auto operator*() const -> reference {
assert(indptr_ != nullptr);
assert(0 <= group_);
assert(group_ < static_cast<Idx>(indptr_->size() - 1));
Expand All @@ -123,15 +117,29 @@ class SparseGroupedIdxVector {
latest_value_ = value_type{(*indptr_)[group_], (*indptr_)[group_ + 1]};
return latest_value_;
}
constexpr std::strong_ordering three_way_compare(GroupIterator const& other) const {
assert(indptr_ == other.indptr_);
return group_ <=> other.group_;

friend constexpr std::strong_ordering operator<=>(GroupIterator const& first, GroupIterator const& second) {
assert(first.indptr_ == second.indptr_);
return first.group_ <=> second.group_;
}

constexpr auto operator+=(difference_type n) -> std::add_lvalue_reference_t<GroupIterator> {
group_ += n;
return *this;
}
constexpr auto distance_to(GroupIterator const& other) const {
assert(indptr_ == other.indptr_);
return other.group_ - group_;

friend auto operator-(GroupIterator const& first, GroupIterator const& second) -> difference_type {
assert(first.indptr_ == second.indptr_);
return first.group_ - second.group_;
}
constexpr void advance(Idx n) { group_ += n; }

private:
IdxVector const* indptr_{};
Idx group_{};
mutable std::remove_const_t<value_type>
latest_value_{}; // making this mutable allows us to delay out-of-bounds checks until
// dereferencing instead of update methods. Note that the value will be
// invalidated at first update
};

constexpr auto group_iterator(Idx group) const { return GroupIterator{indptr_, group}; }
Expand Down Expand Up @@ -168,32 +176,25 @@ class SparseGroupedIdxVector {

class DenseGroupedIdxVector {
private:
class GroupIterator : public IteratorFacade<GroupIterator, IdxRange const, Idx> {
friend class IteratorFacade<GroupIterator, IdxRange const, Idx>;
using Base = IteratorFacade<GroupIterator, IdxRange const, Idx>;
class GroupIterator : public IteratorFacade {
friend class IteratorFacade; // to expose increment and decrement

public:
using value_type = typename Base::value_type;
using difference_type = typename Base::difference_type;
using reference = typename Base::reference;

GroupIterator() = default;
using iterator = GroupIterator;
using const_iterator = std::add_const_t<GroupIterator>;
using value_type = IdxRange const;
using difference_type = Idx;
using pointer = std::add_pointer_t<value_type>;
using reference = std::add_lvalue_reference_t<value_type>;

GroupIterator() : IteratorFacade{*this} {};
explicit constexpr GroupIterator(IdxVector const& dense_vector, Idx group)
: dense_vector_{&dense_vector},
: IteratorFacade{*this},
dense_vector_{&dense_vector},
group_{group},
group_range_{std::ranges::equal_range(*dense_vector_, group)} {}

private:
using group_iterator = IdxVector::const_iterator;

IdxVector const* dense_vector_{};
Idx group_{};
std::ranges::subrange<group_iterator> group_range_;
mutable value_type latest_value_{}; // making this mutable allows us to delay out-of-bounds checks until
// dereferencing instead of update methods. Note that the value will be
// invalidated at first update

constexpr auto dereference() const -> reference {
constexpr auto operator*() const -> reference {
assert(dense_vector_ != nullptr);

// delaying out-of-bounds checking until dereferencing while still returning a reference type requires
Expand All @@ -203,10 +204,31 @@ class DenseGroupedIdxVector {
narrow_cast<Idx>(std::distance(std::cbegin(*dense_vector_), group_range_.end()))};
return latest_value_;
}
constexpr std::strong_ordering three_way_compare(GroupIterator const& other) const {
return group_ <=> other.group_;

friend constexpr std::strong_ordering operator<=>(GroupIterator const& first, GroupIterator const& second) {
assert(first.dense_vector_ == second.dense_vector_);
return first.group_ <=> second.group_;
}

constexpr auto operator+=(difference_type n) -> std::add_lvalue_reference_t<GroupIterator> {
advance(n);
return *this;
}
constexpr auto distance_to(GroupIterator const& other) const { return other.group_ - group_; }
friend auto operator-(GroupIterator const& first, GroupIterator const& second) -> difference_type {
assert(first.dense_vector_ == second.dense_vector_);
return first.group_ - second.group_;
}

private:
using group_iterator = IdxVector::const_iterator;

IdxVector const* dense_vector_{};
Idx group_{};
std::ranges::subrange<group_iterator> group_range_;
mutable std::remove_const_t<value_type>
latest_value_{}; // making this mutable allows us to delay out-of-bounds checks until
// dereferencing instead of update methods. Note that the value will be
// invalidated at first update

constexpr void increment() {
++group_;
Expand All @@ -220,7 +242,7 @@ class DenseGroupedIdxVector {
.base(),
group_range_.begin()};
}
constexpr void advance(Idx n) {
constexpr void advance(difference_type n) {
auto const start = n > 0 ? group_range_.end() : std::cbegin(*dense_vector_);
auto const stop = n < 0 ? group_range_.begin() : std::cend(*dense_vector_);

Expand Down
140 changes: 80 additions & 60 deletions power_grid_model_c/power_grid_model/include/power_grid_model/common/iterator_facade.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,93 +8,113 @@

#include <concepts>
#include <ranges>
#include <utility>

namespace power_grid_model {
template <class Impl, typename ValueType, std::integral DifferenceType> class IteratorFacade {
namespace detail {
template <typename T>
concept iterator_facadeable_c = std::integral<typename T::difference_type> && std::is_pointer_v<typename T::pointer> &&
std::is_lvalue_reference_v<typename T::reference> &&
requires(T t, std::remove_cvref_t<T> mt, std::add_const_t<T> ct,
std::add_const_t<T> ct2, typename std::remove_cvref_t<T>::difference_type d) {
typename T::value_type;
{ *t } -> std::same_as<typename T::reference>;
{ &*t } -> std::same_as<typename T::pointer>;
{ ct <=> ct2 } -> std::same_as<std::strong_ordering>;
{ mt += d } -> std::same_as<std::add_lvalue_reference_t<T>>;
{ ct - ct2 } -> std::same_as<typename T::difference_type>;
};

template <typename T, typename Int>
concept compound_assignable_c = requires(T t, Int n) {
{ t += n } -> std::same_as<std::add_lvalue_reference_t<T>>;
};
} // namespace detail

class IteratorFacade {
public:
using iterator = Impl; // CRTP
using const_iterator = std::add_const_t<iterator>;
using value_type = std::remove_cvref_t<ValueType>;
using difference_type = DifferenceType;
using iterator_category = std::random_access_iterator_tag;
using pointer = std::add_pointer_t<ValueType>;
using reference = std::add_lvalue_reference_t<ValueType>;

constexpr auto operator*() const -> decltype(auto) { return static_cast<const_iterator*>(this)->dereference(); }
constexpr auto operator*() -> reference
requires requires(iterator it) {
{ it.dereference() } -> std::same_as<reference>;
}
{
return static_cast<iterator*>(this)->dereference();
template <typename Self> constexpr decltype(auto) operator->(this Self&& self) {
return &(*std::forward<Self>(self));
}
constexpr auto operator->() const -> decltype(auto) { return &(*(*this)); }
constexpr auto operator->() -> decltype(auto) { return &(*(*this)); }

friend constexpr bool operator==(IteratorFacade const& first, IteratorFacade const& second) {
return (first <=> second) == std::strong_ordering::equivalent;
}
friend constexpr std::strong_ordering operator<=>(IteratorFacade const& first, IteratorFacade const& second) {
return first.three_way_compare(second);
template <typename Self, typename Other>
requires std::same_as<std::remove_cvref_t<Self>, std::remove_cvref_t<Other>>
constexpr bool operator==(this Self const& self, Other const& other) {
return (self <=> other) == std::strong_ordering::equivalent;
}

constexpr auto operator++() -> iterator& {
if constexpr (requires(iterator it) { it.increment(); }) {
static_cast<iterator*>(this)->increment();
// NOLINTNEXTLINE(cert-dcl21-cpp) // pre-decrement but clang-tidy incorrectly sees this as post-decrement
template <typename Self> constexpr std::add_lvalue_reference_t<Self> operator++(this Self& self) {
if constexpr (requires { self.increment(); }) { // NOTE: IteratorFacade should be a friend class
self.increment();
} else {
static_cast<iterator*>(this)->advance(1);
return (self += 1);
}
return *static_cast<iterator*>(this);
return self;
}
constexpr auto operator--() -> iterator& {
if constexpr (requires(iterator it) { it.decrement(); }) {
static_cast<iterator*>(this)->decrement();
// NOLINTNEXTLINE(cert-dcl21-cpp) // pre-decrement but clang-tidy incorrectly sees this as post-decrement
template <typename Self> constexpr std::add_lvalue_reference_t<Self> operator--(this Self& self) {
if constexpr (requires { self.decrement(); }) { // NOTE: IteratorFacade should be a friend class
self.decrement();
} else {
static_cast<iterator*>(this)->advance(-1);
return (self += -1);
}
return *static_cast<iterator*>(this);
return self;
}
constexpr auto operator++(std::integral auto /*idx*/) -> iterator {
iterator result{*static_cast<iterator*>(this)};
++(*this);
template <typename Self>
constexpr std::remove_cvref_t<Self> operator++(this Self& self, std::integral auto /*idx*/) {
using Result = std::remove_cvref_t<Self>;
Result result{self};
++self;
return result;
}
constexpr auto operator--(std::integral auto /*idx*/) -> iterator {
iterator result{*static_cast<iterator*>(this)};
--(*this);
template <typename Self>
constexpr std::remove_cvref_t<Self> operator--(this Self& self, std::integral auto /*idx*/) {
using Result = std::remove_cvref_t<Self>;
Result result{self};
--self;
return result;
}
constexpr auto operator+=(std::integral auto offset) -> iterator& {
static_cast<iterator*>(this)->advance(offset);
return *static_cast<iterator*>(this);
template <typename Self>
constexpr std::add_lvalue_reference_t<Self> operator-=(this Self& self, std::integral auto idx) {
return (self += (-idx));
}
constexpr auto operator-=(std::integral auto idx) -> iterator& { return ((*this) += (-idx)); }

friend constexpr auto operator+(iterator const& it, difference_type offset) -> iterator {
iterator result{it};
template <typename Self, std::integral Int>
friend constexpr std::remove_cvref_t<Self> operator+(Self&& self, Int offset)
requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> //&& detail::iterator_facadeable_c<Self>
{
using Result = std::remove_cvref_t<Self>;
Result result{std::forward<Self>(self)};
result += offset;
return result;
}
friend constexpr auto operator+(difference_type offset, iterator it) -> iterator { return (it += offset); }
friend constexpr auto operator-(iterator const& it, difference_type idx) -> iterator { return it + (-idx); }
friend constexpr auto operator-(IteratorFacade const& first, IteratorFacade const& second) -> difference_type {
return second.distance_to(first);
template <typename Self, std::integral Int>
requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> //&& detail::iterator_facadeable_c<Self>
friend constexpr std::remove_cvref_t<Self> operator+(Int offset, Self&& self) {
return std::forward<Self>(self) + offset;
}
template <typename Self, std::integral Int>
requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> //&& detail::iterator_facadeable_c<Self>
friend constexpr std::remove_cvref_t<Self> operator-(Self&& self, Int idx) {
return (std::forward<Self>(self)) + (-idx);
}

constexpr auto operator[](difference_type idx) const -> value_type const& { return *(*this + idx); }

private:
IteratorFacade() = default; // default constructor is private to prevent non-CRTP instantiation
friend Impl; // allow Impl to access private members; this is necessary for CRTP

// overloads for public bidirectional exposure (difference between MSVC and ClangCL)
constexpr std::strong_ordering three_way_compare(IteratorFacade const& other) const {
return static_cast<std::add_lvalue_reference_t<const_iterator>>(*this).three_way_compare(
static_cast<std::add_lvalue_reference_t<const_iterator>>(other));
template <typename Self>
constexpr decltype(auto) operator[](this Self const& self, typename Self::difference_type idx) {
return *(self + idx);
}
constexpr auto distance_to(IteratorFacade const& other) const -> difference_type {
return static_cast<std::add_lvalue_reference_t<const_iterator>>(*this).distance_to(
static_cast<std::add_lvalue_reference_t<const_iterator>>(other));

// prevent construction by non-derived and non-iterator-facadeable types
IteratorFacade() = delete;
template <typename Self> constexpr explicit IteratorFacade(Self& /*self*/) {
// cannot be done using constraints because the type is not fully instantiated yet when the compiler
// instantiates the constructor. Note that this is different from the other methods because those are only
// instantiated when used.
static_assert(std::derived_from<std::remove_cvref_t<Self>, IteratorFacade>);
static_assert(detail::iterator_facadeable_c<std::remove_cvref_t<Self>>);
}
};

Expand Down
Loading
Loading