Bevel Lemelisk
Bevel Lemelisk

Reputation: 193

Do gcc and clang STL implementations violate rules about allocator?

In 23.2.1p3 C++11 Standart we can read:

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.6.8.2). These functions are called only for the container’s element type, not for internal types used by the container. [ Note: This means, for example, that a node-based container might need to construct nodes containing aligned buffers and call construct to place the element into the buffer. —end note ]


allocator_traits<allocator_type>::construct just call passed allocator's construct method, if allocator defines one. I tried to use this and create allocator, which use list-initialization for construction, so I can utilize emplace for aggregate initialization:

#include <memory>
#include <vector>
#include <string>
#include <iostream>
#include <cmath>

template<typename T>
struct init_list_allocator : public std::allocator<T> {
    template<typename... Args>
    void construct(T* p, Args&&... args)
        { ::new((void *)p) T{std::forward<Args>(args)...}; }

    // Fix copy-constructors usage for aggregates
    void construct(T* p, T& copy_construct_arg)
        { std::allocator<T>::construct(p, copy_construct_arg); }

    void construct(T* p, const T& copy_construct_arg)
        { std::allocator<T>::construct(p, copy_construct_arg); }

    void construct(T* p, const T&& copy_construct_arg)
        { std::allocator<T>::construct(p, std::move(copy_construct_arg)); }

    void construct(T *p, T&& move_construct_arg)
        { std::allocator<T>::construct(p, std::move(move_construct_arg)); }
};

template<class T>
using improved_vector = std::vector<T, init_list_allocator<T>>;

struct A {
    int x;
    double y;
    const char* z;
};

int main()
{
    using namespace std;
    vector<string> strings;
    improved_vector<A> v;
    for (int i = 0; i < 21; ++i) {
        strings.emplace_back(to_string(i*i));
        v.emplace_back(i, sqrt(i), strings.back().c_str());
    };
    for (const auto& elem : v)
        cout << elem.x << ' ' << elem.y << ' ' << elem.z << '\n';
}


However, at least in gcc and clang, this doesn't work. The problem is, that their implementations of vector use Allocator::rebind<T>::other::construct instead of Allocator::construct. And, because of our inheritance from std::allocator, this rebind gives std::allocator<T>::construct. Ok, no problem, just add

template<typename U>
struct rebind {
    using other = init_list_allocator<U>;
};

in our allocator's definition and this code will work. Great, now let's change vector to list. Here we have the unsolvable problem, because instead of Allocator::construct object is initialized inside std::_List_node<_Tp> constuctor in direct-initialization form (form with brackets).

Are this 2 issues a standard violations or I miss something?

Upvotes: 2

Views: 623

Answers (2)

Bevel Lemelisk
Bevel Lemelisk

Reputation: 193

After some research, I have found the answer myself and want to provide it.

For the first issue (using Allocator::rebind<T>::other::construct instead of Allocator::construct), my first allocator implementation (second one is OK) doesn't satisfy Allocator requirements in part of rebind, see 17.6.3.5 Table 28:

+------------------+-------------+-------------------------------------+
|    Expression    | Return type | Assertion/note pre-/post- condition |
+------------------+-------------+-------------------------------------+
| typename         | Y           | For all U (including T),            |
| X::template      |             | Y::template rebind<T>::other is X.  |
| rebind<U>::other |             |                                     |
+------------------+-------------+-------------------------------------+

For the second issue: GCC has old, pre-C++11 implementation of std::list, which will be fixed only in GCC 5.0, because this change breaks ABI (see Should std::list::size have constant complexity in C++11? for more info)

However, the quoted standard requirement, that container must call construct function for exactly allocator_type and not for some rebinded type seems like a standard defect (http://cplusplus.github.io/LWG/lwg-active.html#2218). Libstdc++ implementations of std::set, multiset, map and multimap rely on this fact and use rebinded allocator for construct (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64096).

Upvotes: 0

Billy ONeal
Billy ONeal

Reputation: 106559

To my understanding, libstdc++ and MSVC++ are correct here. The point of rebind is, as the note indicates, that containers may be required to construct things that are not T. For example, std::list<T> needs to construct a list node containing T, not T. Similar cases exist for the associative and unordered containers. That's why the rebind structure exists in the first place. Your allocator was nonconforming before that was in place.


For the second issue, your reference

These functions are called only for the container’s element type, not for internal types used by the container.

seems to indicate that standard library implementations aren't allowed to call construct for rebound allocators, however. This may be a bug in libstdc++.


As for the actual solution to this problem, give A a constructor that has the behavior you want, and don't bother with allocators for this purpose. People may want to create instances of A outside of a container with the special allocator:

#include <vector>

struct A {
    int x;
    double y;
    const char* z;
    A() = default; // This allows A to still be a POD because the default constructor
                   // is not "user-provided", see 8.4.2 [dcl.fct.def.default]/4
    A(int x_, double y_, char const* z_) : x(x_), y(y_), z(z_) {}

};

int main()
{
    using namespace std;
    vector<string> strings;
    vector<A> v;
    for (int i = 0; i < 21; ++i) {
        strings.emplace_back(to_string(i*i));
        v.emplace_back(i, sqrt(i), strings.back().c_str());
    };
    for (const auto& elem : v)
        cout << elem.x << ' ' << elem.y << ' ' << elem.z << '\n';
}

Upvotes: 1

Related Questions