ETLCPP / etl

Embedded Template Library
https://www.etlcpp.com
MIT License
2.05k stars 373 forks source link

Iterator returned by crc.input() does not satisfy `std::output_iterator` #799

Closed jaskij closed 6 months ago

jaskij commented 7 months ago

I have a function which accepts an std::output_iterator:

template<std::endian endianness, std::integral T, std::input_iterator InIt>
    requires std::same_as<std::iter_value_t<InIt>, T>
constexpr auto int_arr_to_bytes(std::size_t items, InIt in, std::output_iterator<uint8_t> auto bytes) {
    for (std::size_t i = 0; i < items; ++i) {
        bytes = int_to_bytes<endianness, T>(in[i], bytes);
    }

    return bytes;
}

Which I wanted to call like so:

uint32_t data_crc(std::span<const uint32_t> data) {
    etl::crc32_posix_t256 crc;
    int_arr_to_bytes<std::endian::little, uint32_t>(data.size(), data.begin(), crc.input());

    return 0;
}

But it didn't work due to an unsatisfied constraint:

/home/jaskij/projects/457/workspace/code-v2/app/dev-tests/src/flash-test.cpp:70:68:   required from here
/usr/arm-none-eabi/include/c++/13.2.1/bits/iterator_concepts.h:620:13:   required for the satisfaction of 'weakly_incrementable<_Iter>' [with _Iter = etl::private_frame_check_sequence::add_insert_iterator<etl::frame_check_sequence<etl::private_crc::crc_policy<etl::private_crc::crc_parameters<long unsigned int, 79764919, 0, 4294967295, false>, 256> > >]
/usr/arm-none-eabi/include/c++/13.2.1/bits/iterator_concepts.h:634:13:   required for the satisfaction of 'input_or_output_iterator<_Iter>' [with _Iter = etl::private_frame_check_sequence::add_insert_iterator<etl::frame_check_sequence<etl::private_crc::crc_policy<etl::private_crc::crc_parameters<long unsigned int, 79764919, 0, 4294967295, false>, 256> > >]
/usr/arm-none-eabi/include/c++/13.2.1/bits/iterator_concepts.h:662:13:   required for the satisfaction of 'output_iterator<auto:64, unsigned char>' [with auto:64 = etl::private_frame_check_sequence::add_insert_iterator<etl::frame_check_sequence<etl::private_crc::crc_policy<etl::private_crc::crc_parameters<long unsigned int, 79764919, 0, 4294967295, false>, 256> > >]
/usr/arm-none-eabi/include/c++/13.2.1/bits/iterator_concepts.h:621:10:   in requirements with '_Iter __i' [with _Iter = etl::private_frame_check_sequence::add_insert_iterator<etl::frame_check_sequence<etl::private_crc::crc_policy<etl::private_crc::crc_parameters<long unsigned int, 79764919, 0, 4294967295, false>, 256> > >]
/usr/arm-none-eabi/include/c++/13.2.1/bits/iterator_concepts.h:624:18: note: nested requirement '__is_signed_integer_like<typename std::__detail::__iter_traits_impl<typename std::remove_cvref<_Tp>::type, std::incrementable_traits<typename std::remove_cvref<_Tp>::type> >::type::difference_type>' is not satisfied
  624 |         requires __detail::__is_signed_integer_like<iter_difference_t<_Iter>>;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

After some messing around, I realized it's missing difference_type in the iterator and came up with this hacky patch:

index dd639550..ef27fa98 100644
--- a/include/etl/frame_check_sequence.h
+++ b/include/etl/frame_check_sequence.h
@@ -52,6 +52,7 @@ namespace etl
     class add_insert_iterator : public etl::iterator<ETL_OR_STD::output_iterator_tag, void, void, void, void>
     {
     public:
+      using difference_type = ptrdiff_t;

       //***********************************
       explicit add_insert_iterator(TFCS& fcs) ETL_NOEXCEPT
jwellbelove commented 7 months ago

Thanks, I'll check that out.

jwellbelove commented 7 months ago

That's odd, as the base etl::iterator class defines it.

template <typename TCategory, typename T, typename TDistance = ptrdiff_t, typename TPointer = T* , typename TReference = T& >
struct iterator
{
  typedef T          value_type;
  typedef TDistance  difference_type;
  typedef TPointer   pointer;
  typedef TReference reference;
  typedef TCategory  iterator_category;
};
jaskij commented 7 months ago

That's odd, as the base etl::iterator class defines it.

Yes, and add_insert_iterator passes void for the TDistance argument.

jwellbelove commented 7 months ago

Of course! 👍

jaskij commented 7 months ago

While we're at it, shouldn't value_type be set to uint8_t? Then TPointer and TReference can be entirely skipped for add_insert_iterator.

jaskij commented 7 months ago

Had some issues with my IDE earlier today, but finally went in and checked. Yup, this works as well.

diff --git a/include/etl/frame_check_sequence.h b/include/etl/frame_check_sequence.h
index dd639550..c040d77c 100644
--- a/include/etl/frame_check_sequence.h
+++ b/include/etl/frame_check_sequence.h
@@ -49,7 +49,7 @@ namespace etl
     /// An output iterator used to add new values.
     //***************************************************
     template <typename TFCS>
-    class add_insert_iterator : public etl::iterator<ETL_OR_STD::output_iterator_tag, void, void, void, void>
+    class add_insert_iterator : public etl::iterator<ETL_OR_STD::output_iterator_tag, uint8_t>
     {
     public:
jwellbelove commented 7 months ago

I think the best solution is where the value type is derived from the frame check sequence template type.

template <typename TFCS>
class add_insert_iterator : public etl::iterator<ETL_OR_STD::output_iterator_tag, typename TFCS::value_type>
jaskij commented 7 months ago

You're probably right, I'm not familiar with the internals of ETL and threw in what felt correct.

jaskij commented 7 months ago

Have tried using TFCS::value_type as you proposed, and it works at least in my setup.

jwellbelove commented 6 months ago

Fixed 20.38.9