Deduction for vecmem vector type

[beomki-yeo]

@krasznaa @stephenswat @paulgessinger
In detray side, I was trying to deduct vector type of vecmem::vector but it doesn't work due to mismatch in allocator type. (std::vector does work)
How can I achieve polymorphic allocator deduction in a concise way?
It's going to be much easier to understand my motivation if you look at the codes and error below

template <template <typename...> class vector_type, typename T>
struct foo {
    foo(vector_type<T>& I) : data(I) {}

    vector_type<T> data;
};

vecmem::cuda::managed_memory_resource managed_resource;
std::vector< int > std_data({ 1, 2, 3 });
vecmem::vector< int > vecmem_data({ 1, 2, 3 }, &managed_resource);

foo A(std_data); // does work
foo B(vecmem_data); // doesn't work 

/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp: In member function ‘virtual void cuda_containers_test_extendable_memory_Test::TestBody()’:
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:174:22: error: class template argument deduction failed:
     foo B(vecmem_data);
                      ^
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:174:22: error: no matching function for call to ‘foo(vecmem::vector<int>&)’
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:100:5: note: candidate: ‘template<template<class ...> class vector_type, class T> foo(vector_type<T>&)-> foo<vector_type, T>’
     foo(vector_type<T>& I) : data(I) {
     ^~~
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:100:5: note:   template argument deduction/substitution failed:
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:174:22: note:   mismatched types ‘std::allocator<_T2>’ and ‘std::experimental::fundamentals_v2::pmr::polymorphic_allocator<int>’
     foo B(vecmem_data);
                      ^
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:174:22: note:   ‘vecmem::vector<int>’ {aka ‘std::vector<int, std::experimental::fundamentals_v2::pmr::polymorphic_allocator<int> >’} is not derived from ‘vector_type<T>’
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:99:8: note: candidate: ‘template<template<class ...> class vector_type, class T> foo(foo<vector_type, T>)-> foo<vector_type, T>’
 struct foo {
        ^~~
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:99:8: note:   template argument deduction/substitution failed:
/home/beomki/projects/vecmem/vecmem/tests/cuda/test_cuda_containers.cpp:174:22: note:   ‘std::vector<int, std::experimental::fundamentals_v2::pmr::polymorphic_allocator<int> >’ is not derived from ‘foo<vector_type, T>’
     foo B(vecmem_data);
                      ^

[stephenswat]

Hi Beomki,

Your structure only captures the first template argument for the std::vector class, which is the internal type. What it does not capture is the rest of the type arguments. This works for the non-vecmem vector because its default allocator type is std::allocator, but it will not work for vecmem vectors. What is happening is the compiler sees you are passing a vecmem::vector, but then immediately forgets that vecmem::vector uses a polymorphic allocator, and reverts to using the default allocator type.

I would propose the following fix to your struct:

template <template <typename...> class vector_type, typename T, typename ...As>
struct foo {
    foo(vector_type<T, As...>& I) : data(I) {}

    vector_type<T, As...> data;
};

[stephenswat]

As an alternative solution, we can simply accept any type into our foo class, and reject any non-vector types via a SFINAE mechanism:

template<typename T>
struct is_vector : std::false_type {};

template<typename ...As>
struct is_vector<std::vector<As...>> : std::true_type {};

template <class vector_type, std::enable_if_t<is_vector<vector_type>::value, bool> = true>
struct foo {
    foo(vector_type& I) : data(I) {}

    vector_type data;
};

It's your decision to make which you think is more appropriate or elegant. 😄

[krasznaa]

A number of formalisms are possible. But I would vote for something a bit more concrete. Like:

template <template <typename, class> class vector_type, typename T, class A>
struct foo {
    foo(vector_type<T, A>& I) : data(I) {}

    vector_type<T, A> data;
};

All "vector types" (that we want to use in our code) have 2 template parameters. The code can reflect that. We shouldn't use variadic templates when we don't absolutely need to.

The is_vector type trait is a good one. The question is if your code is best to use this type, or the formalism that I proposed above. Since "my" formalism gives you convenient access to the vector element type as well. While with Stephen's latter solution you'd need to access that with:

using value_type = typename vector_type::value_type;

Which is definitely not the end of the world, but we don't need to complicate things necessarily with this.

[paulgessinger]

I agree it's probably safest not to use a template parameter pack if not needed.

[stephenswat]

At the risk of once more outing myself as an obnoxious pedant, it depends a little bit on what Beomki is trying to do. If the point is to support only std::vector and vecmem::vector, then having non-packed parameters works well. If you're looking for a more general solution, I would argue that the semantics of "forward all remaining template parameters" is more sound than "forward the single remaining template parameters". Of course that relies on a lot of assumptions, including the assumption that the value type is always the first type in the list, and that there are no non-type template arguments. Arguably, accepting the container type as a whole (without deconstructing it at deduction time) is the most flexible solution, but until we move to C++20 we still won't have all the tools necessary to really come up with a robust solution for this.

[paulgessinger]

Technically you're right of course.
My only argument would be to not be as inclusive as possible out the door, but only as inclusive as necessary.

[beomki-yeo]

Hmm I should have mentioned this - The design should work with std::vector, vecmem::vector and vecmem::device_vector, so non-packed parameters may not be the solution for this. Anyway it is not as trivial as I expected