[Bug libstdc++/64865] New: std::allocator::construct/destroy not called for specialization of std::allocator<trivialtype>

[Bug libstdc++/64865] New: std::allocator::construct/destroy not called for specialization of std::allocator<trivialtype>

[Casey at Carter dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64865

            Bug ID: 64865
           Summary: std::allocator::construct/destroy not called for
                    specialization of std::allocator<trivialtype>
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Casey at Carter dot net

Per N4296 [container.requirements.general]/3:

For the components affected by this subclause that declare an allocator_type,
objects stored in these components shall be constructed using the
allocator_traits<allocator_type>::construct function and destroyed using the
allocator_traits<allocator_type>::destroy function (20.7.8.2).

libstdc++ optimizes out calls to `construct` and `destroy` for types with
trivial construction/destruction when the allocator type is a specialization of
std::allocator: see implementation of __uninitialized_copy_a(InputIterator,
InputIterator, ForwardIterator, allocator<>&) in bits/stl_uninitialized.h, and
_Destroy(ForwardIterator, ForwardIterator, allocator<>&) in
bits/stl_construct.h. However, not every instance of std::allocator necessarily
has side-effect free implementation of construct/destruct, since
[namespace.std]/1 allows users to specialize templates in the standard
namespace if the declaration depends on a user-defined type. For example, this
program:

#include <cassert>
#include <memory>
#include <vector>

struct mytype {
    int value;

    mytype(int v) : value{v} {}
    operator int() const { return value; }
};

int constructions = 0, destructions = 0;

namespace std {
template <>
struct allocator<::mytype> {
    using value_type = mytype;

    allocator() = default;
    template <typename U>
    allocator(const allocator<U>&) {}

    mytype* allocate(std::size_t n) {
        return static_cast<mytype*>(::operator new(n * sizeof(mytype)));
    }

    void deallocate(mytype* ptr, std::size_t) noexcept {
        ::operator delete(ptr);
    }

    template <typename U, typename...Args>
    void construct(U* ptr, Args&&...args) {
        ::new ((void*)ptr) U(std::forward<Args>(args)...);
        ++::constructions;
    }

    template <typename U>
    void destroy(U* ptr) noexcept {
        ++::destructions;
        ptr->~U();
    }

    friend constexpr bool operator == (const allocator&, const allocator&)
noexcept {
        return true;
    }
    friend constexpr bool operator != (const allocator&, const allocator&)
noexcept {
        return false;
    }
};
} // namespace std

int main() {
    {
        std::vector<mytype>{1,2,3};
        // assert(constructions == 3); // assert would fire
    }
    // assert(destructions == 3); // assert would fire
    return constructions != 3 || destructions != 3;
}

always returns non-zero, and either of the asserts will fire if uncommented.

Some possible solutions:

* Change all such optimizations in the library to presume that specializations
of std::allocator<T> are the "default allocator" only if they are derived from
__allocator_base<T>. This is the case for the base template definition as
implemented in bits/allocator.h.

* Remove all such optimizations from the library, and let the compiler optimize
away trivial construction and destruction. This has the unfortunate side effect
of slowing down debug builds of user programs.

* Close this bug report as WONTFIX since it is horrible design to specialize
std::allocator instead of declaring a new allocator type; given that container
implementations are free to rebind to a different specialization, there is no
guarantee that functionality added to a user-defined specialization will even
be used.

[Bug libstdc++/64865] std::allocator::construct/destroy not called for specialization of std::allocator<trivialtype>

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64865

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Casey Carter from comment #0)
> * Close this bug report as WONTFIX since it is horrible design to specialize
> std::allocator instead of declaring a new allocator type; given that
> container implementations are free to rebind to a different specialization,
> there is no guarantee that functionality added to a user-defined
> specialization will even be used.

This.

[Bug libstdc++/64865] std::allocator::construct/destroy not called for specialization of std::allocator<trivialtype>

[rs2740 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64865

TC <rs2740 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rs2740 at gmail dot com

--- Comment #2 from TC <rs2740 at gmail dot com> ---
(In reply to Casey Carter from comment #0)
> * Close this bug report as WONTFIX since it is horrible design to specialize
> std::allocator instead of declaring a new allocator type; given that
> container implementations are free to rebind to a different specialization,
> there is no guarantee that functionality added to a user-defined
> specialization will even be used.

Since the allocator_type member must be the Alloc template argument provided by
the user and not any rebound types, though, it seems to me that it is in fact
guaranteed that their versions of construct() and destroy() will be called, as
the paragraph you cite requires the use of
allocator_traits<allocator_type>::construct and
allocator_traits<allocator_type>::destroy.

[Bug libstdc++/64865] std::allocator::construct/destroy not called for specialization of std::allocator<trivialtype>

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64865

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to TC from comment #2)
> Since the allocator_type member must be the Alloc template argument provided
> by the user and not any rebound types, though,

N.B. libstdc++ supports std::vector<int, std::allocator<char>> and so always
rebinds the _Alloc parameter to get an allocator for the value_type. That
doesn't change your argument though, because it will rebind it to
allocator<value_type> and that's  the type that's specialized.

> it seems to me that it is in
> fact guaranteed that their versions of construct() and destroy() will be
> called, as the paragraph you cite requires the use of
> allocator_traits<allocator_type>::construct and
> allocator_traits<allocator_type>::destroy.

That's a defect, see http://cplusplus.github.io/LWG/lwg-active.html#2218

That defect is a problem for node-based containers, but in principle any
container should be able to use
allocator_traits<allocator_type>::rebind_traits<SomeInternalType>::construct()
to construct the elements, in which case your allocator<value_type>
specialization is not used.