Why does this wrapping allocator's constructor take in the wrong type during template substitution (perfect forwarding ctor)?

Viewed 114

For some context, I'm writing an allocator that takes in a base allocator as a template type, and does nothing but forward the allocate() and deallocate() calls to the underlying allocator member. Creating std::vectors with this custom allocator works fine. I tried writing a make_shared wrapper that will use dummy_allocator<T, std::allocator<T>> by default but it wasn't successful. Here is a reproducible example:

#include <memory>
namespace test {

template<typename T, typename base_allocator=std::allocator<T>>
class dummy_allocator {
public:
    typedef typename std::allocator_traits<base_allocator>::size_type size_type;
    typedef typename std::allocator_traits<base_allocator>::difference_type difference_type;
    typedef typename std::allocator_traits<base_allocator>::pointer pointer;
    typedef typename std::allocator_traits<base_allocator>::const_pointer const_pointer;
    typedef typename std::allocator_traits<base_allocator>::value_type value_type;

    template<class U>
    struct rebind {
        typedef dummy_allocator<U,
                typename std::allocator_traits<base_allocator>::template rebind_alloc<U>> other;
    };

    template<typename... Args>
    dummy_allocator(Args &&... args) noexcept : alloc(std::forward<Args>(args)...) {}


    dummy_allocator(const dummy_allocator& a) = default;

    [[nodiscard]] T *allocate(std::size_t n) {
        T *p = alloc.allocate(n);
        return p;
    }

    void deallocate(T *p, std::size_t size) noexcept {
        alloc.deallocate(p, size);
    }

private:
    base_allocator alloc;
};

/// Allocate using a wrapped version of passed in allocator
template <typename T, typename Alloc, typename... Args>
std::shared_ptr<T> allocate_shared(const Alloc& alloc, Args&&... args) {
    auto dummy_alloc = dummy_allocator<T, Alloc>(alloc);
    return std::allocate_shared<T>(dummy_alloc, std::forward<Args>(args)...);
}

/// Create a shared pointer from a default stl allocator wrapped in profile allocator.
template <typename T, typename... Args>
std::shared_ptr<T> make_shared(Args&&... args) {
    return test::allocate_shared<T>(std::allocator<T>(), std::forward<Args>(args)...);
}

} // namespace test

int main() {
    auto ptr = test::make_shared<double>();
    return 0;
}

When I ran the above code, the compiler generated some bizarre template substitution failure errors:

/usr/include/c++/10.1.0/bits/shared_ptr_base.h:679:43:   required from ‘std::__shared_count<_Lp>::__shared_count(_Tp*&, std::_Sp_alloc_shared_tag<_Alloc>, _Args&& ...) [with _Tp = double; _Alloc = test::dummy_allocator<double, std::allocator<double> >; _Args = {}; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’
/usr/include/c++/10.1.0/bits/shared_ptr_base.h:1371:71:   required from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = test::dummy_allocator<double, std::allocator<double> >; _Args = {}; _Tp = double; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’
/usr/include/c++/10.1.0/bits/shared_ptr.h:408:59:   required from ‘std::shared_ptr<_Tp>::shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = test::dummy_allocator<double, std::allocator<double> >; _Args = {}; _Tp = double]’
/usr/include/c++/10.1.0/bits/shared_ptr.h:859:14:   required from ‘std::shared_ptr<_Tp> std::allocate_shared(const _Alloc&, _Args&& ...) [with _Tp = double; _Alloc = test::dummy_allocator<double, std::allocator<double> >; _Args = {}]’
/home/ray/home/testing/src/alloc.cpp:44:35:   required from ‘std::shared_ptr<_Tp> test::allocate_shared(const Alloc&, Args&& ...) [with T = double; Alloc = std::allocator<double>; Args = {}]’
/home/ray/home/testing/src/alloc.cpp:50:36:   required from ‘std::shared_ptr<_Tp> test::make_shared(Args&& ...) [with T = double; Args = {}]’
/home/ray/home/testing/src/alloc.cpp:86:46:   required from here
/home/ray/home/testing/src/alloc.cpp:25:82: error: no matching function for call to ‘std::allocator<std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic> >::allocator(const test::dummy_allocator<double, std::allocator<double> >&)’
   25 |     dummy_allocator(Args &&... args) noexcept : alloc(std::forward<Args>(args)...) {}
      |                                                                                  ^
In file included from /usr/include/c++/10.1.0/list:61,
                 from /home/ray/home/testing/src/alloc.cpp:3:
/usr/include/c++/10.1.0/bits/allocator.h:157:2: note: candidate: ‘template<class _Tp1> constexpr std::allocator< <template-parameter-1-1> >::allocator(const std::allocator<_Tp1>&) [with _Tp1 = _Tp1; _Tp = std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic>]’
  157 |  allocator(const allocator<_Tp1>&) _GLIBCXX_NOTHROW { }
      |  ^~~~~~~~~
/usr/include/c++/10.1.0/bits/allocator.h:157:2: note:   template argument deduction/substitution failed:
/home/ray/home/testing/src/alloc.cpp:25:82: note:   ‘const test::dummy_allocator<double, std::allocator<double> >’ is not derived from ‘const std::allocator<_Up>’
   25 |     dummy_allocator(Args &&... args) noexcept : alloc(std::forward<Args>(args)...) {}
      |                                                                                  ^
In file included from /usr/include/c++/10.1.0/list:61,
                 from /home/ray/home/testing/src/alloc.cpp:3:
/usr/include/c++/10.1.0/bits/allocator.h:147:7: note: candidate: ‘constexpr std::allocator< <template-parameter-1-1> >::allocator(const std::allocator< <template-parameter-1-1> >&) [with _Tp = std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic>]’
  147 |       allocator(const allocator& __a) _GLIBCXX_NOTHROW
      |       ^~~~~~~~~
/usr/include/c++/10.1.0/bits/allocator.h:147:34: note:   no known conversion for argument 1 from ‘const test::dummy_allocator<double, std::allocator<double> >’ to ‘const std::allocator<std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic> >&’
  147 |       allocator(const allocator& __a) _GLIBCXX_NOTHROW
      |                 ~~~~~~~~~~~~~~~~~^~~
/usr/include/c++/10.1.0/bits/allocator.h:144:7: note: candidate: ‘constexpr std::allocator< <template-parameter-1-1> >::allocator() [with _Tp = std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic>]’
  144 |       allocator() _GLIBCXX_NOTHROW { }
      |       ^~~~~~~~~
/usr/include/c++/10.1.0/bits/allocator.h:144:7: note:   candidate expects 0 arguments, 1 provided
... (The above error basically is repeated 2 more times)

In particular, this error seemed representative of the problem occurring:

error: no matching function for call to ‘std::allocator<std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic> >::allocator(const test::dummy_allocator<double, std::allocator<double> >&)’

Where it basically says "in the constructor of dummy_allocator, you can't pass in a dummy_allocator to the std::allocator's constructor". But I'm not doing that. In allocate_shared, I'm passing in the std::allocator into the dummy_allocator.

I've really scratched my head reading the compiler error but came to no conclusions on what I'm doing wrong. Any help would be greatly appreciated!

EDIT: I think I have a hunch that whatever magic shared_ptr is doing underneath, it's trying to do a copy constructor of my dummy_allocator, and perfect forwarding is capturing the copy construction instead an actual copy constructor. However, I have no idea how to solve this as it's a variadic template and I can't use std::is_same<Args, dummy_allocator> as a requires clause in the perfect forwarding constructor.

So as Daniel Langr has pointed out, just copy constructing the dummy_allocator will fail. I fixed that issue with the following requires clause:

...
    template <typename T1, typename ...TV>
    struct is_dummy : std::is_same<typename std::decay<T1>::type, dummy_allocator<T, base_allocator>>{
    };

    template<typename... Args>
    requires (!is_dummy<Args...>::value)
    dummy_allocator(Args &&... args) noexcept : alloc(std::forward<Args>(args)...) {}

    dummy_allocator() = default;
    dummy_allocator(const dummy_allocator& a) = default;
...

However, that's not enough to fix the shared_ptr issue, where the error is a bit longer...*

So here is the error you get:

#include <memory>
namespace test {

template<typename T, typename base_allocator=std::allocator<T>>
class dummy_allocator {
public:
    /// Necessary for allocators, propagate exactly what the base_allocator
    /// wants.
    typedef typename std::allocator_traits<base_allocator>::size_type size_type;
    typedef typename std::allocator_traits<base_allocator>::difference_type difference_type;
    typedef typename std::allocator_traits<base_allocator>::pointer pointer;
    typedef typename std::allocator_traits<base_allocator>::const_pointer const_pointer;
    typedef typename std::allocator_traits<base_allocator>::value_type value_type;

    template<class U>
    struct rebind {
        typedef dummy_allocator<U,
                typename std::allocator_traits<base_allocator>::template rebind_alloc<U>> other;
    };

    template <typename T1, typename ...TV>
    struct is_dummy : std::is_same<typename std::decay<T1>::type, dummy_allocator<T, base_allocator>>{
    };

    template<typename... Args>
    requires (!is_dummy<Args...>::value)
    dummy_allocator(Args &&... args) noexcept : alloc(std::forward<Args>(args)...) {}

    dummy_allocator() = default;
    dummy_allocator(const dummy_allocator& a) = default;

    [[nodiscard]] T *allocate(std::size_t n) {
        T *p = alloc.allocate(n);
        return p;
    }

    void deallocate(T *p, std::size_t size) noexcept {
        alloc.deallocate(p, size);
    }

private:
    base_allocator alloc;
};

/// Allocate using a wrapped version of passed in allocator
template <typename T, typename Alloc, typename... Args>
auto allocate_shared(const Alloc& alloc, Args&&... args) {
    auto dummy_alloc = dummy_allocator<T, Alloc>(alloc);
    return std::allocate_shared<T>(dummy_alloc, std::forward<Args>(args)...);
}

/// Create a shared pointer from a default stl allocator wrapped in profile allocator.
template <typename T, typename... Args>
auto make_shared(Args&&... args) {
    return test::allocate_shared<T>(std::allocator<T>(), std::forward<Args>(args)...);
}

} // namespace test

int main() {
    // This will fail
    auto ptr = test::make_shared<double>();

    // This will now work
    auto dummy_alloc = test::dummy_allocator<int, std::allocator<int>>();
    auto dummy_alloc2 = test::dummy_allocator<int, std::allocator<int>>(dummy_alloc);
    return 0;
}

The associated error:

/usr/include/c++/10.1.0/bits/shared_ptr_base.h:679:43:   required from ‘std::__shared_count<_Lp>::__shared_count(_Tp*&, std::_Sp_alloc_shared_tag<_Alloc>, _Args&& ...) [with _Tp = double; _Alloc = test::dummy_allocator<double, std::allocator<double> >; _Args = {}; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’
/usr/include/c++/10.1.0/bits/shared_ptr_base.h:1371:71:   required from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = test::dummy_allocator<double, std::allocator<double> >; _Args = {}; _Tp = double; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’
/usr/include/c++/10.1.0/bits/shared_ptr.h:408:59:   required from ‘std::shared_ptr<_Tp>::shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = test::dummy_allocator<double, std::allocator<double> >; _Args = {}; _Tp = double]’
/usr/include/c++/10.1.0/bits/shared_ptr.h:859:14:   required from ‘std::shared_ptr<_Tp> std::allocate_shared(const _Alloc&, _Args&& ...) [with _Tp = double; _Alloc = test::dummy_allocator<double, std::allocator<double> >; _Args = {}]’
/home/ray/home/test/src/alloc.cpp:53:35:   required from ‘auto test::allocate_shared(const Alloc&, Args&& ...) [with T = double; Alloc = std::allocator<double>; Args = {}]’
/home/ray/home/test/src/alloc.cpp:59:36:   required from ‘auto test::make_shared(Args&& ...) [with T = double; Args = {}]’
/home/ray/home/test/src/alloc.cpp:95:46:   required from here
/home/ray/home/test/src/alloc.cpp:31:82: error: no matching function for call to ‘std::allocator<std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic> >::allocator(const test::dummy_allocator<double, std::allocator<double> >&)’
   31 |     dummy_allocator(Args &&... args) noexcept : alloc(std::forward<Args>(args)...) {}
      |                                                                                  ^
In file included from /usr/include/c++/10.1.0/list:61,
                 from /home/ray/home/test/src/alloc.cpp:3:
/usr/include/c++/10.1.0/bits/allocator.h:157:2: note: candidate: ‘template<class _Tp1> constexpr std::allocator< <template-parameter-1-1> >::allocator(const std::allocator<_Tp1>&) [with _Tp1 = _Tp1; _Tp = std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic>]’
  157 |  allocator(const allocator<_Tp1>&) _GLIBCXX_NOTHROW { }
      |  ^~~~~~~~~
/usr/include/c++/10.1.0/bits/allocator.h:157:2: note:   template argument deduction/substitution failed:
/home/ray/home/test/src/alloc.cpp:31:82: note:   ‘const test::dummy_allocator<double, std::allocator<double> >’ is not derived from ‘const std::allocator<_Up>’
   31 |     dummy_allocator(Args &&... args) noexcept : alloc(std::forward<Args>(args)...) {}
      |                                                                                  ^
In file included from /usr/include/c++/10.1.0/list:61,
                 from /home/ray/home/test/src/alloc.cpp:3:
/usr/include/c++/10.1.0/bits/allocator.h:147:7: note: candidate: ‘constexpr std::allocator< <template-parameter-1-1> >::allocator(const std::allocator< <template-parameter-1-1> >&) [with _Tp = std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic>]’
  147 |       allocator(const allocator& __a) _GLIBCXX_NOTHROW
      |       ^~~~~~~~~
/usr/include/c++/10.1.0/bits/allocator.h:147:34: note:   no known conversion for argument 1 from ‘const test::dummy_allocator<double, std::allocator<double> >’ to ‘const std::allocator<std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic> >&’
  147 |       allocator(const allocator& __a) _GLIBCXX_NOTHROW
      |                 ~~~~~~~~~~~~~~~~~^~~
/usr/include/c++/10.1.0/bits/allocator.h:144:7: note: candidate: ‘constexpr std::allocator< <template-parameter-1-1> >::allocator() [with _Tp = std::_Sp_counted_ptr_inplace<double, test::dummy_allocator<double, std::allocator<double> >, __gnu_cxx::_S_atomic>]’
  144 |       allocator() _GLIBCXX_NOTHROW { }
      |       ^~~~~~~~~
/usr/include/c++/10.1.0/bits/allocator.h:144:7: note:   candidate expects 0 arguments, 1 provided
3 Answers

Just remove the variadic constructor and add these two:

dummy_allocator(const base_allocator& a) : alloc(a)
{}

template<class U, class Alloc>
dummy_allocator(const dummy_allocator<U, Alloc>& a) : alloc(a.alloc)
{}

Demo


This doesn't solve the issue if I wanted to forward arguments to an allocator that requires arguments :(

Then you can use std::is_constructible:

template<class... Args>
dummy_allocator(Args&&... args)
requires(std::is_constructible_v<base_allocator, Args...>) 
    : alloc(std::forward<Args>(args)...)
{}

template<class U, class Alloc>
dummy_allocator(const dummy_allocator<U, Alloc>& a) : alloc(a.alloc)
{}

Demo


Note that template<class U, class Alloc> dummy_allocator constuctor is needed in both cases, because dummy_allocator a(b) should be well-formed for any b of rebinded allocator type.

I think your hunch is correct. One always has to be careful with perfect forwarding constructors and the normal copy constructor. I think there are multiple solutions but I haven't tested any of them yet.

First you could provide a better match than the perfect forwarding constructor by adding dummy_allocator(dummy_allocator&) forwarding to the normal copy constructor. The perfect forwarding constructor matches better than the copy constructor, when the passed dummy_allocator is not const and hence the copy constructor has to do const conversion. But this would require also overloading the move constructor (I think) since otherwise you get the same error when someone somewhere tries to move the dummy_allocator.

A bit more tricky but probably safer is doing the following:

template<class... Args>
dummy_allocator(Args&&... args) requires (sizeof...(Args) != 1) : alloc(std::forward<Args>(args)...) {}

template<class Arg>
dummy_allocator(Arg&& arg) requires (!std::is_same_v<std::decay_t<Arg>, dummy_allocator>) : alloc(std::forward<Arg>(arg)) {}

dummy_allocator(const dummy_allocator&) = default;

I hope I got the requires clauses right, I haven't worked much with that yet. But doing SFINAE in constructors is a royal pain in the ass.

@Evg's answer is correct in the implementation for making the arguments constructible, but it did not yet solve the problem of why I was getting the shared_ptr issue. I solved using his implementation because I felt like using is_constructible::value is the best way to approach it, but we need to add a constructor that allows for different types to rebind:

template<typename T, typename base_allocator=std::allocator<T>>
class dummy_allocator {
public:
    /// Necessary for allocators, propagate exactly what the base_allocator
    /// wants.
    typedef typename std::allocator_traits<base_allocator>::size_type size_type;
    typedef typename std::allocator_traits<base_allocator>::difference_type difference_type;
    typedef typename std::allocator_traits<base_allocator>::pointer pointer;
    typedef typename std::allocator_traits<base_allocator>::const_pointer const_pointer;
    typedef typename std::allocator_traits<base_allocator>::value_type value_type;

    template<class U>
    struct rebind {
        typedef dummy_allocator<U,
                typename std::allocator_traits<base_allocator>::template rebind_alloc<U>> other;
    };

    template <typename T1, typename ...TV>
    struct is_dummy : std::is_same<typename std::decay<T1>::type, dummy_allocator<T, base_allocator>>{
    };

    template<typename... Args>

    dummy_allocator(Args &&... args) requires (std::is_constructible_v<base_allocator, Args...>) : alloc(std::forward<Args>(args)...) {}

    template <typename U, typename A> friend class dummy_allocator;

    // Construct a dummy allocator from another dummy allocator with the same base_allocator but with different type.
    template <typename U>
    dummy_allocator(
            const dummy_allocator<U,
                    typename std::allocator_traits<base_allocator>::template rebind_alloc<U>>& other) noexcept :
            alloc(other.alloc) {}

    dummy_allocator() = default;
    dummy_allocator(const dummy_allocator& a) : alloc(a.alloc) {}

    [[nodiscard]] T *allocate(std::size_t n) {
        T *p = alloc.allocate(n);
        return p;
    }

    void deallocate(T *p, std::size_t size) noexcept {
        alloc.deallocate(p, size);
    }

private:
    base_allocator alloc;
};

In particular, these lines here:

    template <typename U, typename A> friend class dummy_allocator;

    // Construct a dummy allocator from another dummy allocator with the same base_allocator but with different type.
    template <typename U>
    dummy_allocator(
            const dummy_allocator<U,
                    typename std::allocator_traits<base_allocator>::template rebind_alloc<U>>& other) noexcept :
            alloc(other.alloc) {}

Because we need to have constructors between different underlying types for allocators.

Related