ETLCPP / etl

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

etl::span subspan could statically check extents #842

Closed mike919192 closed 7 months ago

mike919192 commented 7 months ago

Hi, I noticed that the etl::span subspan function does not currently statically check extents. std::span does check this.

Here is a code demonstration:

std::array<int, 10> array {0};

//std::span std_span1 = std::span(array).subspan<15, 1>(); //compile time error
//std::span std_span2 = std::span(array).subspan<5, 8>(); //compile time error

etl::span etl_span1 = etl::span(array).subspan<15, 1>(); //currently no compile time error
etl::span etl_span2 = etl::span(array).subspan<5, 8>(); //currently no compile time error

Seems like it should be fairly easy to add. I tested a modification to add the static checking, at least for c++17 and newer. Not sure if there is a way to add it to 11 and 14.

#if ETL_USING_CPP11
    //*************************************************************************
    /// Obtains a span that is a view from OFFSET over the next COUNT elements of this span.
    //*************************************************************************
    template <size_t OFFSET, size_t COUNT = etl::dynamic_extent>
    ETL_NODISCARD ETL_CONSTEXPR
    etl::span<element_type, COUNT != etl::dynamic_extent ? COUNT : Extent - OFFSET> subspan() const ETL_NOEXCEPT
    {
//ADDED THESE LINES
#if ETL_USING_CPP17
      if constexpr(extent != etl::dynamic_extent)
      {
        static_assert(OFFSET <= extent);
        if constexpr(COUNT != etl::dynamic_extent)
        {
          static_assert(COUNT <= extent);
          static_assert(COUNT <= (extent - OFFSET));
        }
      }
#endif
//END ADDED LINES       
      return (COUNT == etl::dynamic_extent) ? etl::span<element_type, COUNT != etl::dynamic_extent ? COUNT : Extent - OFFSET>(pbegin + OFFSET, (pbegin + Extent))
                                            : etl::span<element_type, COUNT != etl::dynamic_extent ? COUNT : Extent - OFFSET>(pbegin + OFFSET, pbegin + OFFSET + COUNT);
    }
#else
    //*************************************************************************
    /// Obtains a span that is a view from OFFSET over the next COUNT elements of this span.
    //*************************************************************************
    template <size_t OFFSET, size_t COUNT>
    etl::span<element_type, COUNT != etl::dynamic_extent ? COUNT : Extent - OFFSET> subspan() const
    {
      if (COUNT == etl::dynamic_extent)
      {
        return etl::span<element_type, (COUNT != etl::dynamic_extent ? COUNT : Extent - OFFSET)>(pbegin + OFFSET, (pbegin + Extent));
      }
      else
      {
        return etl::span<element_type, COUNT != etl::dynamic_extent ? COUNT : Extent - OFFSET>(pbegin + OFFSET, pbegin + OFFSET + COUNT);
      }
    }
#endif
jwellbelove commented 7 months ago

Thanks, you can use if ETL_IF_CONSTEXPR in place of #if ETL_USING_CPP17 and if constexpr.

mike919192 commented 7 months ago

OK, I'll setup a c++14 build later and see if that works. I thought there might be an issue if it tries to compile that block as a regular if statement, but I'll test it.

mike919192 commented 7 months ago

I realized a way to implement without needing if constexpr, so it should now work for c++11 and newer. See #843

mike919192 commented 7 months ago

PR was accepted. Thanks