Skip to content
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.

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

Cannot implicitly convert between a subclass of std::vector<T*> and tcb::span<const T* const> #56

Open
matthew-wozniczka opened this issue Aug 14, 2023 · 7 comments

Comments

@matthew-wozniczka
Copy link

matthew-wozniczka commented Aug 14, 2023

I have an AutoVector<T> template class in our codebase which inherits publicly from std::vector<T*> and handles ownership (created before the days of C++11).

I've noticed that I get compiler errors when I try to have an instance of this type implicitly convert to the associated span type, for example, something like

namespace Simba { namespace Support { class SqlTypeMetadata; } }

void UseMeta(simba_thirdparty_tcb::span<const Foo* const> meta);

void bar()
{
    AutoVector<Foo> meta(GetMeta());
    UseMeta(meta); // error C2440: 'return': cannot convert from 'Simba::Support::AutoVector<Simba::Support::SqlTypeMetadata>' to 'simba_thirdparty_tcb::span<const Simba::Support::SqlTypeMetadata *const ,18446744073709551615>'
    // UseMeta({ meta.data(), meta.size() }); // This works
}

similar code works fine when using std::vector<T*> directly. Looking at the C++20 std::span documentation, this should work as long as the AutoVector<T> type models the Range concept, and I think if std::vector does, so should a subclass (that doesn't explicitly delete any member functions)?

I'm having the problem in Visual Studio 2019, haven't tried any other compilers/platforms yet.

@tcbrindle
Copy link
Owner

Subclassing std::vector<int*> and converting to span<const int* const> seems to work correctly in a quick test:

https://gcc.godbolt.org/z/Y68boPvYo

Are you able to give any more information about the error message that might help me recreate the problem, or perhaps try with another compiler?

@matthew-wozniczka
Copy link
Author

I was trying to reproduce it w/ something closer to my code, but it was still working, but when I switched to MSVC even the std::vector case broke! (maybe I got confused earlier about it working)

See https://gcc.godbolt.org/z/e3q5e4PW5

It looks like it doesn't like it when it needs to add const?

@kimci86
Copy link
Contributor

kimci86 commented Aug 15, 2023

I think the problem boils down to this:

#include <type_traits>
static_assert(std::is_convertible<int* (*)[], const int* const (*)[]>::value, "");
int main() {}

On compiler explorer, this compiles fine with:

  • x64 MSVC 19.14 with /std:c++17
  • GCC >= 10.1 with -std:c++11
  • clang >= 7.0.0 with -std:c++11

This does not compile with x64 MSVC 19.14 with /std:c++14 nor with older GCC or clang.

If we remove the first const, it compile everywhere.

@tcbrindle
Copy link
Owner

Thank you very much for the detective work @kimci86!

After some more investigation, this seems to be caused by CWG issue 330, which was reported in 2002 and finally resolved via N4261 in 2014.

So that's the cause, but unfortunately there are no simple solutions that I can think of.

The condition that's being checked -- can we convert a pointer-to-array-of-U into a pointer-to-array-of-T -- is directly lifted from the std::span specification, and I'm very reluctant to change it as it what prevents, for example, instantiating a span<Base> from an array of Derived.

Unless anyone has any clever ideas, I'm afraid that this may just be something that we have to say is unsupported in older compilers.

(Note that gsl::span uses the exact same check and so I guess it has precisely the same issue.)

@matthew-wozniczka
Copy link
Author

Wouldn't it be possible to 'wrap' std::is_convertible in something which handles this case by first removing all the consts from the type? IMO it would make sense to try & make this work for older compilers since this library is meant to support older C++ standards?

@tcbrindle
Copy link
Owner

The goal is to provide span for older compilers to the extent supported by the language. If the core language is telling us that it doesn't think the pointers are convertible, well, we probably shouldn't do the conversion.

@matthew-wozniczka
Copy link
Author

matthew-wozniczka commented Nov 19, 2023

As an update, I tried to implement the algorithm from the linked paper myself, and it seems to work? I included the following before span.hpp and changed its usages of std::is_convertible to IsConvertible

template <typename From, typename To, bool ConstCanDiffer, bool VolatileCanDiffer, typename Enabler = void>
    struct IsConvertibleForArray
    {
        static constexpr bool value = false;
    };

    template <typename T, bool ConstCanDiffer, bool VolatileCanDiffer>
    struct IsConvertibleForArray<T, T, ConstCanDiffer, VolatileCanDiffer>
    {
        static constexpr bool value = true;
    };

    template <typename T, bool VolatileCanDiffer>
    struct IsConvertibleForArray<T, const T, true, VolatileCanDiffer, typename std::enable_if<!std::is_const<T>::value>::type>
    {
        static constexpr bool value = true;
    };

    template <typename T, bool ConstCanDiffer>
    struct IsConvertibleForArray<T, volatile T, ConstCanDiffer, true, typename std::enable_if<!std::is_volatile<T>::value>::type>
    {
        static constexpr bool value = true;
    };

    template <typename T1, typename T2, bool VolatileCanDiffer>
    struct IsConvertibleForArray<const T1, const T2, true, VolatileCanDiffer, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, true, VolatileCanDiffer>::value;
    };

    template <typename T1, typename T2, bool VolatileCanDiffer>
    struct IsConvertibleForArray<T1, const T2, true, VolatileCanDiffer, typename std::enable_if<!std::is_const<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, false, VolatileCanDiffer>::value;
    };

    template <typename T1, typename T2, bool ConstCanDiffer>
    struct IsConvertibleForArray<volatile T1, volatile T2, ConstCanDiffer, true, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, true>::value;
    };

    template <typename T1, typename T2, bool ConstCanDiffer>
    struct IsConvertibleForArray<T1, volatile T2, ConstCanDiffer, true, typename std::enable_if<!std::is_volatile<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, false>::value;
    };

    template <typename T1, typename T2, bool VolatileCanDiffer>
    struct IsConvertibleForArray<const T1*, const T2*, true, VolatileCanDiffer, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, true, VolatileCanDiffer>::value;
    };

    template <typename T1, typename T2, bool VolatileCanDiffer>
    struct IsConvertibleForArray<T1*, const T2*, true, VolatileCanDiffer, typename std::enable_if<!std::is_const<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, false, VolatileCanDiffer>::value;
    };

    template <typename T1, typename T2, bool ConstCanDiffer>
    struct IsConvertibleForArray<volatile T1*, volatile T2*, ConstCanDiffer, true, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, true>::value;
    };

    template <typename T1, typename T2, bool ConstCanDiffer>
    struct IsConvertibleForArray<T1*, volatile T2*, ConstCanDiffer, true, typename std::enable_if<!std::is_volatile<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, false>::value;
    };

    template <typename C, typename T1, typename T2, bool VolatileCanDiffer>
    struct IsConvertibleForArray<const T1 C::*, const T2 C::*, true, VolatileCanDiffer, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, true, VolatileCanDiffer>::value;
    };

    template <typename C, typename T1, typename T2, bool VolatileCanDiffer>
    struct IsConvertibleForArray<T1 C::*, const T2 C::*, true, VolatileCanDiffer, typename std::enable_if<!std::is_const<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, false, VolatileCanDiffer>::value;
    };

    template <typename C, typename T1, typename T2, bool ConstCanDiffer>
    struct IsConvertibleForArray<volatile T1 C::*, volatile T2 C::*, ConstCanDiffer, true, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, true>::value;
    };

    template <typename C, typename T1, typename T2, bool ConstCanDiffer>
    struct IsConvertibleForArray<T1 C::*, volatile T2 C::*, ConstCanDiffer, true, typename std::enable_if<!std::is_volatile<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, false>::value;
    };

    template <typename T1, typename T2, size_t N, bool VolatileCanDiffer>
    struct IsConvertibleForArray<const T1[N], const T2[N], true, VolatileCanDiffer, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, true, VolatileCanDiffer>::value;
    };

    template <typename T1, typename T2, size_t N, bool VolatileCanDiffer>
    struct IsConvertibleForArray<T1[N], const T2[N], true, VolatileCanDiffer, typename std::enable_if<!std::is_const<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, false, VolatileCanDiffer>::value;
    };

    template <typename T1, typename T2, size_t N, bool ConstCanDiffer>
    struct IsConvertibleForArray<volatile T1[N], volatile T2[N], ConstCanDiffer, true, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, true>::value;
    };

    template <typename T1, typename T2, size_t N, bool ConstCanDiffer>
    struct IsConvertibleForArray<T1[N], volatile T2[N], ConstCanDiffer, true, typename std::enable_if<!std::is_volatile<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, false>::value;
    };

    template <typename T1, typename T2, bool VolatileCanDiffer>
    struct IsConvertibleForArray<const T1[], const T2[], true, VolatileCanDiffer, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, true, VolatileCanDiffer>::value;
    };

    template <typename T1, typename T2, bool VolatileCanDiffer>
    struct IsConvertibleForArray<T1[], const T2[], true, VolatileCanDiffer, typename std::enable_if<!std::is_const<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, false, VolatileCanDiffer>::value;
    };

    template <typename T1, typename T2, bool ConstCanDiffer>
    struct IsConvertibleForArray<volatile T1[], volatile T2[], ConstCanDiffer, true, typename std::enable_if<!std::is_same<T1, T2>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, true>::value;
    };

    template <typename T1, typename T2, bool ConstCanDiffer>
    struct IsConvertibleForArray<T1[], volatile T2[], ConstCanDiffer, true, typename std::enable_if<!std::is_volatile<T1>::value>::type>
    {
        static constexpr bool value = IsConvertibleForArray<T1, T2, ConstCanDiffer, false>::value;
    };

    // Workaround for lack of N4261 in C++11 (See https://github.com/tcbrindle/span/issues/56)
    template <typename From, typename To>
    struct IsConvertible;

    template <typename From, typename To>
    struct IsConvertible<From(*)[], To(*)[]>
    {
        static constexpr bool value = IsConvertibleForArray<From, To, true, true>::value;
    };

Just putting it out there (and would love to hear if this approach is flawed or I missed something).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants