Questions on span constructors

[mike919192]

Hi. I was investigating one issue and maybe have run into a few more with the span constructors.

First I was trying to figure out why the following does not generate a compiler error:

int arr[5]{};
etl::span span1(arr);
etl::span<int, 10> span2(span1);  //no compile error here despite different extents.  equivalent std span on gcc does have compile error

In the previous code this is the constructor being used for span2:

    //*************************************************************************
    /// Construct from a container or other type that supports
    /// data() and size() member functions.
    //*************************************************************************
    template <typename TContainer, typename = typename etl::enable_if<!etl::is_pointer<etl::remove_reference_t<TContainer>>::value &&
                                                                      !etl::is_array<etl::remove_reference_t<TContainer>>::value&&
                                                                      etl::is_same<etl::remove_cv_t<T>, etl::remove_cv_t<typename etl::remove_reference_t<TContainer>::value_type>>::value, void>::type>
      ETL_CONSTEXPR span(TContainer&& a) ETL_NOEXCEPT
      : pbegin(a.data())
    {
    }

This seems to me like it is choosing the wrong constructor, especially since further on there are copy constructors defined that do check that the extents are matching (if they are static).

    //*************************************************************************
    /// Copy constructor
    //*************************************************************************
    ETL_CONSTEXPR span(const span& other) ETL_NOEXCEPT
      : pbegin(other.pbegin)
    {
    }

    //*************************************************************************
    /// Copy constructor
    //*************************************************************************
    template <typename U, size_t N>
    ETL_CONSTEXPR span(const etl::span<U, N>& other, typename etl::enable_if<(Extent == etl::dynamic_extent) || (N == etl::dynamic_extent) || (N == Extent), void>::type) ETL_NOEXCEPT
      : pbegin(other.data())
    {
    }

I made a quick test to remove the first constructor to force the use of the last two. Using the last two constructors I get the compile error I was looking for in the first code example.

So just to make sure I am on the right track. Does this all make sense that the last 2 constructors should be preferred in this case over the first one? If so what would be the best way to get the last 2 to be chosen?

Just as a quick a dirty test I tried modifying the enable_if like this:

    //*************************************************************************
    /// Construct from a container or other type that supports
    /// data() and size() member functions.
    /// Modifed the enable_if statement to exclude if TContainer is derived from a common "span_base"
    /// So that this constructor is not chosen when constructing from another span
    //*************************************************************************
    template <typename TContainer, typename = typename etl::enable_if<!etl::is_base_of<span_base, etl::remove_reference_t<TContainer>>::value &&
                                                                      !etl::is_pointer<etl::remove_reference_t<TContainer>>::value &&
                                                                      !etl::is_array<etl::remove_reference_t<TContainer>>::value&&
                                                                      etl::is_same<etl::remove_cv_t<T>, etl::remove_cv_t<typename etl::remove_reference_t<TContainer>::value_type>>::value, void>::type>
      ETL_CONSTEXPR span(TContainer&& a) ETL_NOEXCEPT
      : pbegin(a.data())
    {
    }

Let me know what you think about all this. Thanks

[jwellbelove]

Thanks, I'll take a look at that.

[mike919192]

I'll open a PR so you can see the diff of test that I did. One more thing, I think there is an error with the enable_if of the second copy constructor that causes it to always evaluate as false. You can look at the diff and then decide how to proceed.