From 279158384d44794ce44c24a9d8ad6db1a45c8754 Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Wed, 19 Aug 2026 17:21:21 +0200 Subject: [PATCH 1/5] Add static assertions about Expr's range properties --- SeQuant/core/expressions/expr.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/SeQuant/core/expressions/expr.hpp b/SeQuant/core/expressions/expr.hpp index 852498c8dc..32b9f11ddf 100644 --- a/SeQuant/core/expressions/expr.hpp +++ b/SeQuant/core/expressions/expr.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -531,6 +532,10 @@ class Expr : public std::enable_shared_from_this, Exception not_implemented(const char *fn) const; }; // class Expr +static_assert(std::ranges::sized_range); +static_assert(std::ranges::bidirectional_range); +static_assert(std::ranges::random_access_range); + template <> struct Expr::is_shared_ptr_of_expr : std::true_type {}; template <> From d72787c98f054ae0a73b6ed2bcc3b88ea2c694af Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Wed, 19 Aug 2026 19:53:09 +0200 Subject: [PATCH 2/5] Use const ref --- SeQuant/core/io/serialization/v1/serialize.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SeQuant/core/io/serialization/v1/serialize.cpp b/SeQuant/core/io/serialization/v1/serialize.cpp index 01dff04f1a..3b4f56c044 100644 --- a/SeQuant/core/io/serialization/v1/serialize.cpp +++ b/SeQuant/core/io/serialization/v1/serialize.cpp @@ -196,7 +196,7 @@ std::wstring to_string(Sum const& sum, const SerializationOptions& options) { std::wstring serialized; for (std::size_t i = 0; i < sum.size(); ++i) { - ExprPtr& current = sum[i]; + const ExprPtr& current = sum[i]; const bool parenthesize = current->is(); From d47f1af2bba84420c7f9f3591672f39c662efbbd Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Wed, 19 Aug 2026 19:53:26 +0200 Subject: [PATCH 3/5] Add ExprIterator impl --- SeQuant/core/expressions/expr_iterator.hpp | 140 +++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 SeQuant/core/expressions/expr_iterator.hpp diff --git a/SeQuant/core/expressions/expr_iterator.hpp b/SeQuant/core/expressions/expr_iterator.hpp new file mode 100644 index 0000000000..b5ec4db847 --- /dev/null +++ b/SeQuant/core/expressions/expr_iterator.hpp @@ -0,0 +1,140 @@ +#ifndef SEQUANT_EXPRESSIONS_EXPR_ITERATOR_HPP +#define SEQUANT_EXPRESSIONS_EXPR_ITERATOR_HPP + +#include + +#include +#include +#include + +namespace sequant { + +class ExprPtr; + +namespace detail { + +template +class ExprIteratorImpl { + public: + using value_type = ExprPtr; + using reference = std::add_lvalue_reference_t< + std::conditional_t, value_type>>; + using const_reference = + std::add_lvalue_reference_t>; + using pointer = std::add_pointer_t< + std::conditional_t, value_type>>; + using difference_type = std::ptrdiff_t; + + explicit ExprIteratorImpl(pointer ptr = nullptr) : ptr_(ptr) {} + + ExprIteratorImpl &operator+=(difference_type val) { + ptr_ += val; + return *this; + } + + friend ExprIteratorImpl operator+(const ExprIteratorImpl &it, + difference_type val) { + return ExprIteratorImpl(it.ptr_ + val); + } + + friend ExprIteratorImpl operator+(difference_type val, + const ExprIteratorImpl &it) { + return ExprIteratorImpl(it.ptr_ + val); + } + + ExprIteratorImpl &operator++() { + ++ptr_; + return *this; + } + + ExprIteratorImpl operator++(int) { + ExprIteratorImpl copy = *this; + + ++ptr_; + + return copy; + } + + ExprIteratorImpl &operator-=(difference_type val) { + ptr_ -= val; + return *this; + } + + friend ExprIteratorImpl operator-(const ExprIteratorImpl &it, + difference_type val) { + return ExprIteratorImpl(it.ptr_ - val); + } + + friend ExprIteratorImpl operator-(difference_type val, + const ExprIteratorImpl &it) { + return ExprIteratorImpl(it.ptr_ - val); + } + + ExprIteratorImpl &operator--() { + --ptr_; + return *this; + } + + ExprIteratorImpl operator--(int) { + ExprIteratorImpl copy = *this; + + --ptr_; + + return copy; + } + + reference operator*() const { + SEQUANT_ASSERT(ptr_); + return *ptr_; + } + + pointer operator->() const { + SEQUANT_ASSERT(ptr_); + return ptr_; + } + + difference_type operator-(const ExprIteratorImpl &other) const { + return ptr_ - other.ptr_; + } + difference_type operator-(const ExprIteratorImpl &other) const { + return ptr_ - other.ptr_; + } + + bool operator==(const ExprIteratorImpl &other) const { + return ptr_ == other.ptr_; + } + bool operator==(const ExprIteratorImpl &other) const { + return ptr_ == other.ptr_; + } + + reference operator[](difference_type offset) const { + SEQUANT_ASSERT(ptr_); + return *(ptr_ + offset); + } + + std::strong_ordering operator<=>( + const ExprIteratorImpl &other) const { + return ptr_ <=> other.ptr_; + } + std::strong_ordering operator<=>( + const ExprIteratorImpl &other) const { + return ptr_ <=> other.ptr_; + } + + private: + pointer ptr_ = nullptr; +}; + +} // namespace detail + +using ExprIterator = detail::ExprIteratorImpl; +using ConstExprIterator = detail::ExprIteratorImpl; + +static_assert(std::bidirectional_iterator); +static_assert(std::random_access_iterator); +static_assert(std::bidirectional_iterator); +static_assert(std::random_access_iterator); + +} // namespace sequant + +#endif // SEQUANT_EXPRESSIONS_EXPR_ITERATOR_HPP From ce18ae2d24c32d6fb1941cfb6898d4b697dd3526 Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Wed, 19 Aug 2026 19:55:24 +0200 Subject: [PATCH 4/5] Turn Expr into a proper range instead of a view Previously, the Expr implementation used range-v3's view facade to implement range semantics on Expr objects. However, views are different things from containers which had some more or less subtle consequences. See also https://stackoverflow.com/a/31462435 This commit replaces the view facade with virtual begin/end implementations returning ExprIterator objects. Hence, Expr is now a fully fledged container that is usable as a random-access range. --- SeQuant/core/expr.hpp | 1 + SeQuant/core/expressions/expr.cpp | 47 ++++++++++++ SeQuant/core/expressions/expr.hpp | 111 +++++++++------------------ SeQuant/core/expressions/product.hpp | 46 +++++------ SeQuant/core/expressions/sum.hpp | 36 +++++---- tests/unit/test_expr.cpp | 100 +++++++++++++++++------- 6 files changed, 196 insertions(+), 145 deletions(-) diff --git a/SeQuant/core/expr.hpp b/SeQuant/core/expr.hpp index 8b225a40df..9c8e4ed763 100644 --- a/SeQuant/core/expr.hpp +++ b/SeQuant/core/expr.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/SeQuant/core/expressions/expr.cpp b/SeQuant/core/expressions/expr.cpp index dda110b13d..bf5fca38d4 100644 --- a/SeQuant/core/expressions/expr.cpp +++ b/SeQuant/core/expressions/expr.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -30,6 +31,52 @@ namespace sequant { +ExprIterator Expr::begin() { return ExprIterator{}; } + +ExprIterator Expr::end() { return ExprIterator{}; } + +ConstExprIterator Expr::begin() const { return ConstExprIterator{}; } + +ConstExprIterator Expr::end() const { return ConstExprIterator{}; } + +ConstExprIterator Expr::cbegin() const { return begin(); } + +ConstExprIterator Expr::cend() const { return end(); } + +ExprIterator Expr::begin_subexpr() { return begin(); } + +ExprIterator Expr::end_subexpr() { return end(); } + +ConstExprIterator Expr::begin_subexpr() const { return begin(); } + +ConstExprIterator Expr::end_subexpr() const { return end(); } + +std::size_t Expr::size() const { return end() - begin(); } + +bool Expr::empty() const { return size() == 0; } + +ExprPtr &Expr::operator[](std::size_t idx) { + SEQUANT_ASSERT(idx < size()); + return begin()[idx]; +} + +const ExprPtr &Expr::operator[](std::size_t idx) const { + SEQUANT_ASSERT(idx < size()); + return begin()[idx]; +} + +ExprPtr &Expr::at(std::size_t idx) { return (*this)[idx]; } + +const ExprPtr &Expr::at(std::size_t idx) const { return (*this)[idx]; } + +ExprPtr &Expr::front() { return at(0); } + +const ExprPtr &Expr::front() const { return at(0); } + +ExprPtr &Expr::back() { return at(size() - 1); } + +const ExprPtr &Expr::back() const { return at(size() - 1); } + ExprPtr ExprPtr::clone() const & { if (!*this) return {}; return ExprPtr(as_shared_ptr()->clone()); diff --git a/SeQuant/core/expressions/expr.hpp b/SeQuant/core/expressions/expr.hpp index 32b9f11ddf..f4f3e2b299 100644 --- a/SeQuant/core/expressions/expr.hpp +++ b/SeQuant/core/expressions/expr.hpp @@ -1,6 +1,7 @@ #ifndef SEQUANT_EXPRESSIONS_EXPR_HPP #define SEQUANT_EXPRESSIONS_EXPR_HPP +#include #include #include #include @@ -12,9 +13,6 @@ #include #include -#include -#include - namespace sequant { /// @brief the wchar used for labeling adjoints, i.e. the superscript + sign @@ -59,10 +57,8 @@ static const wchar_t adjoint_label = L'\u207A'; /// for(const auto& e: c.expr()) { // iterates over subexpressions /// } /// @endcode -class Expr : public std::enable_shared_from_this, - public ranges::view_facade { +class Expr : public std::enable_shared_from_this { public: - using range_type = ranges::view_facade; using hash_type = std::size_t; using type_id_type = int; // to speed up comparisons @@ -70,7 +66,7 @@ class Expr : public std::enable_shared_from_this, virtual ~Expr() = default; /// @return true if this is a leaf - bool is_atom() const { return ranges::empty(*this); } + bool is_atom() const { return empty(); } /// @return true if this is zero virtual bool is_zero() const { return false; } @@ -150,14 +146,6 @@ class Expr : public std::enable_shared_from_this, return visit_impl(*this, std::forward(visitor), atoms_only); } - auto begin_subexpr() { return range_type::begin(); } - - auto end_subexpr() { return range_type::end(); } - - auto begin_subexpr() const { return range_type::begin(); } - - auto end_subexpr() const { return range_type::end(); } - Expr &expr() { return *this; } const Expr &expr() const { return *this; } @@ -181,7 +169,7 @@ class Expr : public std::enable_shared_from_this, /// overridden for scalar leaf types. virtual bool is_scalar() const { if (is_atom()) return false; - for (auto it = begin_subexpr(); it != end_subexpr(); ++it) { + for (auto it = begin(); it != end(); ++it) { if (!(*it)->is_scalar()) return false; } return true; @@ -200,7 +188,7 @@ class Expr : public std::enable_shared_from_this, return true; else { bool result = true; - for (auto it = begin_subexpr(); result && it != end_subexpr(); ++it) { + for (auto it = begin(); result && it != end(); ++it) { result &= (*it)->is_cnumber(); } return result; @@ -227,14 +215,12 @@ class Expr : public std::enable_shared_from_this, this->is_cnumber() || that.is_cnumber() || commutes_with_atom(that); } else if (this_is_atom) { if (!this->is_cnumber()) { - for (auto it = that.begin_subexpr(); result && it != that.end_subexpr(); - ++it) { + for (auto it = that.begin(); result && it != that.end(); ++it) { result &= this->commutes_with(**it); } } } else { - for (auto it = this->begin_subexpr(); result && it != this->end_subexpr(); - ++it) { + for (auto it = this->begin(); result && it != this->end(); ++it) { result &= (*it)->commutes_with(that); } } @@ -376,9 +362,35 @@ class Expr : public std::enable_shared_from_this, ///@} - private: - friend ranges::range_access; + virtual ExprIterator begin(); + virtual ExprIterator end(); + virtual ConstExprIterator begin() const; + virtual ConstExprIterator end() const; + ConstExprIterator cbegin() const; + ConstExprIterator cend() const; + + ExprIterator begin_subexpr(); + ExprIterator end_subexpr(); + ConstExprIterator begin_subexpr() const; + ConstExprIterator end_subexpr() const; + + std::size_t size() const; + + bool empty() const; + + ExprPtr &operator[](std::size_t idx); + const ExprPtr &operator[](std::size_t idx) const; + ExprPtr &at(std::size_t idx); + const ExprPtr &at(std::size_t idx) const; + + ExprPtr &front(); + const ExprPtr &front() const; + + ExprPtr &back(); + const ExprPtr &back() const; + + private: template < typename E, typename Visitor, typename = std::enable_if_t, Expr>>> @@ -415,59 +427,6 @@ class Expr : public std::enable_shared_from_this, Expr &operator=(Expr &&) = default; Expr &operator=(const Expr &) = default; - struct cursor { - using value_type = ExprPtr; - - cursor() = default; - constexpr explicit cursor(ExprPtr *subexpr_ptr) noexcept - : ptr_{subexpr_ptr} {} - /// when take const ptr note runtime const flag - constexpr explicit cursor(const ExprPtr *subexpr_ptr) noexcept - : ptr_{const_cast(subexpr_ptr)}, const_{true} {} - bool equal(const cursor &that) const { return ptr_ == that.ptr_; } - void next() { ++ptr_; } - void prev() { --ptr_; } - // TODO figure out why can't return const here if want to be able to assign - // to *begin(Expr&) - ExprPtr &read() const { - RANGES_EXPECT(ptr_); - return *ptr_; - } - ExprPtr &read() { - RANGES_EXPECT(const_ == false); - RANGES_EXPECT(ptr_); - return *ptr_; - } - void assign(const ExprPtr &that_ptr) { - RANGES_EXPECT(ptr_); - *ptr_ = that_ptr; - } - std::ptrdiff_t distance_to(cursor const &that) const { - return that.ptr_ - ptr_; - } - void advance(std::ptrdiff_t n) { ptr_ += n; } - - private: - ExprPtr *ptr_ = - nullptr; // both begin and end will be represented by this, so Expr - // without subexpressions begin() equals end() automatically - bool const_ = false; // assert in nonconst ops - }; - - /// @return the cursor for the beginning of the range (must override in a - /// derived Expr that has subexpressions) - virtual cursor begin_cursor() { return cursor{}; } - /// @return the cursor for the end of the range (must override in a derived - /// Expr that has subexpressions) - virtual cursor end_cursor() { return cursor{}; } - - /// @return the cursor for the beginning of the range (must override in a - /// derived Expr that has subexpressions) - virtual cursor begin_cursor() const { return cursor{}; } - /// @return the cursor for the end of the range (must override in a derived - /// Expr that has subexpressions) - virtual cursor end_cursor() const { return cursor{}; } - mutable std::optional hash_value_; // not initialized by default virtual hash_type memoizing_hash() const { static const hash_type default_hash_value = 0; diff --git a/SeQuant/core/expressions/product.hpp b/SeQuant/core/expressions/product.hpp index 83825acc15..65c800df08 100644 --- a/SeQuant/core/expressions/product.hpp +++ b/SeQuant/core/expressions/product.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -388,35 +389,30 @@ class Product : public Expr { scalar_ += 1; } + ExprIterator begin() override { + if (!factors_.empty()) { + reset_hash_value(); + } + + return ExprIterator{factors_.data()}; + } + + ExprIterator end() override { + return ExprIterator{factors_.data() + factors_.size()}; + } + + ConstExprIterator begin() const override { + return ConstExprIterator{factors_.data()}; + } + + ConstExprIterator end() const override { + return ConstExprIterator{factors_.data() + factors_.size()}; + } + private: scalar_type scalar_ = {1, 0}; container::svector factors_{}; - cursor begin_cursor() override { - if (factors_.empty()) { - return Expr::begin_cursor(); - } else { - reset_hash_value(); - return cursor{&factors_[0]}; - } - }; - cursor end_cursor() override { - if (factors_.empty()) { - return Expr::begin_cursor(); - } else { - reset_hash_value(); - return cursor{&factors_[0] + factors_.size()}; - } - }; - - cursor begin_cursor() const override { - return factors_.empty() ? Expr::begin_cursor() : cursor{&factors_[0]}; - }; - cursor end_cursor() const override { - return factors_.empty() ? Expr::end_cursor() - : cursor{&factors_[0] + factors_.size()}; - }; - /// @return the hash of this object, by hashing only the factors, /// not the scalar to make possible rapid finding of Products that only /// differ by a factor diff --git a/SeQuant/core/expressions/sum.hpp b/SeQuant/core/expressions/sum.hpp index 96e48d9ed9..70c95ea9dc 100644 --- a/SeQuant/core/expressions/sum.hpp +++ b/SeQuant/core/expressions/sum.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -274,27 +275,32 @@ class Sum : public Expr { return *this; } + ExprIterator begin() override { + if (!summands_.empty()) { + reset_hash_value(); + } + + return ExprIterator{summands_.data()}; + } + + ExprIterator end() override { + return ExprIterator{summands_.data() + summands_.size()}; + } + + ConstExprIterator begin() const override { + return ConstExprIterator{summands_.data()}; + } + + ConstExprIterator end() const override { + return ConstExprIterator{summands_.data() + summands_.size()}; + } + private: summands_type summands_{}; std::optional constant_summand_idx_{}; // points to the constant summand, if any; used // to sum up constants in append/prepend - cursor begin_cursor() override { - return summands_.empty() ? Expr::begin_cursor() : cursor{&summands_[0]}; - }; - cursor end_cursor() override { - return summands_.empty() ? Expr::end_cursor() - : cursor{&summands_[0] + summands_.size()}; - }; - cursor begin_cursor() const override { - return summands_.empty() ? Expr::begin_cursor() : cursor{&summands_[0]}; - }; - cursor end_cursor() const override { - return summands_.empty() ? Expr::end_cursor() - : cursor{&summands_[0] + summands_.size()}; - }; - /// @return the hash of this object /// @note this ensures that hash of a Sum of a single summand is /// identical to the hash of the summand itself. diff --git a/tests/unit/test_expr.cpp b/tests/unit/test_expr.cpp index bf588b26c8..bac77201a4 100644 --- a/tests/unit/test_expr.cpp +++ b/tests/unit/test_expr.cpp @@ -41,19 +41,22 @@ struct Dummy : public sequant::Expr { bool static_equal(const sequant::Expr &) const override { return true; } }; -template -struct VecExpr : public std::vector, public sequant::Expr { +// Note: we have to use template specialization in order to conditionally +// provide custom begin/end implementations as virtual functions aren't allowed +// to be shadowed in the child class via explicit implementations. +template +struct VecExprImpl : public std::vector, public sequant::Expr { using base_type = std::vector; using base_type::begin; using base_type::end; using base_type::size; - VecExpr() = default; + VecExprImpl() = default; template - VecExpr(std::initializer_list elements) : std::vector(elements) {} + VecExprImpl(std::initializer_list elements) : std::vector(elements) {} template - VecExpr(Iter begin, Iter end) : std::vector(begin, end) {} - virtual ~VecExpr() = default; + VecExprImpl(Iter begin, Iter end) : std::vector(begin, end) {} + virtual ~VecExprImpl() = default; std::wstring to_latex() const override { std::wstring result = L"{\\text{VecExpr}\\{"; for (const auto &e : *this) { @@ -67,42 +70,81 @@ struct VecExpr : public std::vector, public sequant::Expr { return result; } - type_id_type type_id() const override { return get_type_id>(); }; + type_id_type type_id() const override { + return get_type_id>(); + }; private: - cursor begin_cursor() const override { - if constexpr (sequant::Expr::is_shared_ptr_of_expr::value) { - return base_type::empty() ? Expr::begin_cursor() - : cursor{&base_type::at(0)}; - } else { - return Expr::begin_cursor(); - } - }; - cursor end_cursor() const override { - if constexpr (sequant::Expr::is_shared_ptr_of_expr::value) { - return base_type::empty() ? Expr::end_cursor() - : cursor{&base_type::at(0) + base_type::size()}; - } else { - return Expr::end_cursor(); + bool static_equal(const sequant::Expr &that) const override { + return static_cast(*this) == + static_cast( + static_cast(that)); + } + + sequant::ExprPtr clone() const override { + return sequant::ex(this->begin(), this->end()); + } +}; + +template +struct VecExprImpl : public std::vector, public sequant::Expr { + using base_type = std::vector; + using sequant::Expr::size; + + VecExprImpl() = default; + template + VecExprImpl(std::initializer_list elements) : std::vector(elements) {} + template + VecExprImpl(Iter begin, Iter end) : std::vector(begin, end) {} + virtual ~VecExprImpl() = default; + std::wstring to_latex() const override { + std::wstring result = L"{\\text{VecExpr}\\{"; + for (const auto &e : *this) { + if constexpr (sequant::Expr::is_shared_ptr_of_expr_or_derived::value) { + result += e->to_latex() + L" "; + } else { + result += std::to_wstring(e) + L" "; + } } - }; - cursor begin_cursor() override { - return const_cast(*this).begin_cursor(); - }; - cursor end_cursor() override { - return const_cast(*this).end_cursor(); + result += L"\\}}"; + return result; + } + + type_id_type type_id() const override { + return get_type_id>(); }; + sequant::ExprIterator begin() override { + return sequant::ExprIterator{base_type::data()}; + } + + sequant::ExprIterator end() override { + return sequant::ExprIterator{base_type::data() + base_type::size()}; + } + + sequant::ConstExprIterator begin() const override { + return sequant::ConstExprIterator{base_type::data()}; + } + + sequant::ConstExprIterator end() const override { + return sequant::ConstExprIterator{base_type::data() + base_type::size()}; + } + + private: bool static_equal(const sequant::Expr &that) const override { return static_cast(*this) == - static_cast(static_cast(that)); + static_cast( + static_cast(that)); } sequant::ExprPtr clone() const override { - return sequant::ex(this->begin(), this->end()); + return sequant::ex(this->begin(), this->end()); } }; +template +using VecExpr = VecExprImpl::value>; + struct Adjointable : public sequant::Expr { Adjointable() = default; Adjointable(int v) : v(v) {} From 69ba9e4fcea0944953176e73b4d38ed722f5207b Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Thu, 20 Aug 2026 10:13:16 +0200 Subject: [PATCH 5/5] Make begin_subexr/end_subexpr virtual instead of begin/end This avoids the issue of not being able to shadow begin/end in subclasses as shadowing is not allowed for virtual functions. --- SeQuant/core/expressions/expr.cpp | 20 +++--- SeQuant/core/expressions/expr.hpp | 16 ++--- SeQuant/core/expressions/product.hpp | 8 +-- SeQuant/core/expressions/sum.hpp | 8 +-- tests/unit/test_expr.cpp | 97 +++++++++------------------- 5 files changed, 58 insertions(+), 91 deletions(-) diff --git a/SeQuant/core/expressions/expr.cpp b/SeQuant/core/expressions/expr.cpp index bf5fca38d4..a8ce23862e 100644 --- a/SeQuant/core/expressions/expr.cpp +++ b/SeQuant/core/expressions/expr.cpp @@ -31,25 +31,25 @@ namespace sequant { -ExprIterator Expr::begin() { return ExprIterator{}; } +ExprIterator Expr::begin() { return begin_subexpr(); } -ExprIterator Expr::end() { return ExprIterator{}; } +ExprIterator Expr::end() { return end_subexpr(); } -ConstExprIterator Expr::begin() const { return ConstExprIterator{}; } +ConstExprIterator Expr::begin() const { return begin_subexpr(); } -ConstExprIterator Expr::end() const { return ConstExprIterator{}; } +ConstExprIterator Expr::end() const { return end_subexpr(); } -ConstExprIterator Expr::cbegin() const { return begin(); } +ConstExprIterator Expr::cbegin() const { return begin_subexpr(); } -ConstExprIterator Expr::cend() const { return end(); } +ConstExprIterator Expr::cend() const { return end_subexpr(); } -ExprIterator Expr::begin_subexpr() { return begin(); } +ExprIterator Expr::begin_subexpr() { return ExprIterator{}; } -ExprIterator Expr::end_subexpr() { return end(); } +ExprIterator Expr::end_subexpr() { return ExprIterator{}; } -ConstExprIterator Expr::begin_subexpr() const { return begin(); } +ConstExprIterator Expr::begin_subexpr() const { return ConstExprIterator{}; } -ConstExprIterator Expr::end_subexpr() const { return end(); } +ConstExprIterator Expr::end_subexpr() const { return ConstExprIterator{}; } std::size_t Expr::size() const { return end() - begin(); } diff --git a/SeQuant/core/expressions/expr.hpp b/SeQuant/core/expressions/expr.hpp index f4f3e2b299..7bee7e8bc3 100644 --- a/SeQuant/core/expressions/expr.hpp +++ b/SeQuant/core/expressions/expr.hpp @@ -362,17 +362,17 @@ class Expr : public std::enable_shared_from_this { ///@} - virtual ExprIterator begin(); - virtual ExprIterator end(); - virtual ConstExprIterator begin() const; - virtual ConstExprIterator end() const; + ExprIterator begin(); + ExprIterator end(); + ConstExprIterator begin() const; + ConstExprIterator end() const; ConstExprIterator cbegin() const; ConstExprIterator cend() const; - ExprIterator begin_subexpr(); - ExprIterator end_subexpr(); - ConstExprIterator begin_subexpr() const; - ConstExprIterator end_subexpr() const; + virtual ExprIterator begin_subexpr(); + virtual ExprIterator end_subexpr(); + virtual ConstExprIterator begin_subexpr() const; + virtual ConstExprIterator end_subexpr() const; std::size_t size() const; diff --git a/SeQuant/core/expressions/product.hpp b/SeQuant/core/expressions/product.hpp index 65c800df08..8d9e1a629a 100644 --- a/SeQuant/core/expressions/product.hpp +++ b/SeQuant/core/expressions/product.hpp @@ -389,7 +389,7 @@ class Product : public Expr { scalar_ += 1; } - ExprIterator begin() override { + ExprIterator begin_subexpr() override { if (!factors_.empty()) { reset_hash_value(); } @@ -397,15 +397,15 @@ class Product : public Expr { return ExprIterator{factors_.data()}; } - ExprIterator end() override { + ExprIterator end_subexpr() override { return ExprIterator{factors_.data() + factors_.size()}; } - ConstExprIterator begin() const override { + ConstExprIterator begin_subexpr() const override { return ConstExprIterator{factors_.data()}; } - ConstExprIterator end() const override { + ConstExprIterator end_subexpr() const override { return ConstExprIterator{factors_.data() + factors_.size()}; } diff --git a/SeQuant/core/expressions/sum.hpp b/SeQuant/core/expressions/sum.hpp index 70c95ea9dc..4cfcda2c8b 100644 --- a/SeQuant/core/expressions/sum.hpp +++ b/SeQuant/core/expressions/sum.hpp @@ -275,7 +275,7 @@ class Sum : public Expr { return *this; } - ExprIterator begin() override { + ExprIterator begin_subexpr() override { if (!summands_.empty()) { reset_hash_value(); } @@ -283,15 +283,15 @@ class Sum : public Expr { return ExprIterator{summands_.data()}; } - ExprIterator end() override { + ExprIterator end_subexpr() override { return ExprIterator{summands_.data() + summands_.size()}; } - ConstExprIterator begin() const override { + ConstExprIterator begin_subexpr() const override { return ConstExprIterator{summands_.data()}; } - ConstExprIterator end() const override { + ConstExprIterator end_subexpr() const override { return ConstExprIterator{summands_.data() + summands_.size()}; } diff --git a/tests/unit/test_expr.cpp b/tests/unit/test_expr.cpp index bac77201a4..b9112c7ee6 100644 --- a/tests/unit/test_expr.cpp +++ b/tests/unit/test_expr.cpp @@ -41,22 +41,19 @@ struct Dummy : public sequant::Expr { bool static_equal(const sequant::Expr &) const override { return true; } }; -// Note: we have to use template specialization in order to conditionally -// provide custom begin/end implementations as virtual functions aren't allowed -// to be shadowed in the child class via explicit implementations. -template -struct VecExprImpl : public std::vector, public sequant::Expr { +template +struct VecExpr : public std::vector, public sequant::Expr { using base_type = std::vector; using base_type::begin; using base_type::end; using base_type::size; - VecExprImpl() = default; + VecExpr() = default; template - VecExprImpl(std::initializer_list elements) : std::vector(elements) {} + VecExpr(std::initializer_list elements) : std::vector(elements) {} template - VecExprImpl(Iter begin, Iter end) : std::vector(begin, end) {} - virtual ~VecExprImpl() = default; + VecExpr(Iter begin, Iter end) : std::vector(begin, end) {} + virtual ~VecExpr() = default; std::wstring to_latex() const override { std::wstring result = L"{\\text{VecExpr}\\{"; for (const auto &e : *this) { @@ -70,81 +67,51 @@ struct VecExprImpl : public std::vector, public sequant::Expr { return result; } - type_id_type type_id() const override { - return get_type_id>(); - }; + type_id_type type_id() const override { return get_type_id>(); }; - private: - bool static_equal(const sequant::Expr &that) const override { - return static_cast(*this) == - static_cast( - static_cast(that)); + sequant::ConstExprIterator begin_subexpr() const override { + if constexpr (sequant::Expr::is_shared_ptr_of_expr::value) { + return sequant::ConstExprIterator{base_type::data()}; + } else { + return Expr::begin_subexpr(); + } } - sequant::ExprPtr clone() const override { - return sequant::ex(this->begin(), this->end()); + sequant::ConstExprIterator end_subexpr() const override { + if constexpr (sequant::Expr::is_shared_ptr_of_expr::value) { + return sequant::ConstExprIterator{base_type::data() + base_type::size()}; + } else { + return Expr::end_subexpr(); + } } -}; -template -struct VecExprImpl : public std::vector, public sequant::Expr { - using base_type = std::vector; - using sequant::Expr::size; - - VecExprImpl() = default; - template - VecExprImpl(std::initializer_list elements) : std::vector(elements) {} - template - VecExprImpl(Iter begin, Iter end) : std::vector(begin, end) {} - virtual ~VecExprImpl() = default; - std::wstring to_latex() const override { - std::wstring result = L"{\\text{VecExpr}\\{"; - for (const auto &e : *this) { - if constexpr (sequant::Expr::is_shared_ptr_of_expr_or_derived::value) { - result += e->to_latex() + L" "; - } else { - result += std::to_wstring(e) + L" "; - } + sequant::ExprIterator begin_subexpr() override { + if constexpr (sequant::Expr::is_shared_ptr_of_expr::value) { + return sequant::ExprIterator{base_type::data()}; + } else { + return Expr::begin_subexpr(); } - result += L"\\}}"; - return result; } - type_id_type type_id() const override { - return get_type_id>(); + sequant::ExprIterator end_subexpr() override { + if constexpr (sequant::Expr::is_shared_ptr_of_expr::value) { + return sequant::ExprIterator{base_type::data() + base_type::size()}; + } else { + return Expr::end_subexpr(); + } }; - sequant::ExprIterator begin() override { - return sequant::ExprIterator{base_type::data()}; - } - - sequant::ExprIterator end() override { - return sequant::ExprIterator{base_type::data() + base_type::size()}; - } - - sequant::ConstExprIterator begin() const override { - return sequant::ConstExprIterator{base_type::data()}; - } - - sequant::ConstExprIterator end() const override { - return sequant::ConstExprIterator{base_type::data() + base_type::size()}; - } - private: bool static_equal(const sequant::Expr &that) const override { return static_cast(*this) == - static_cast( - static_cast(that)); + static_cast(static_cast(that)); } sequant::ExprPtr clone() const override { - return sequant::ex(this->begin(), this->end()); + return sequant::ex(this->begin(), this->end()); } }; -template -using VecExpr = VecExprImpl::value>; - struct Adjointable : public sequant::Expr { Adjointable() = default; Adjointable(int v) : v(v) {}