Contra P0339 “polymorphic_allocator<> as a vocabulary type”

Pablo Halpern’s P0339R6 “polymorphic_allocator<> as a vocabulary type” (February 2019) was undergoing LWG wording review in Kona this February. It’s still on track to make C++2a. It should be stopped.

(For WG21 members, the notes from LWG wording review are here.)

In this paper, we propose an explicit specialization of pmr::polymrophic_allocator for use as a vocabulary type. This type meets the requirements of an allocator in the standard but is easier to use in contexts where it is not necessary or desirable to fix the allocator type at compile time.

First of all, this use of the phrase “vocabulary type” doesn’t jibe with my understanding of the term’s meaning. A “vocabulary type” is supposed to be a type that is a lingua franca between different library interfaces (e.g. double, std::string). P0339’s proposed std::pmr::polymorphic_allocator<std::byte> is a convenience type for the writers of implementation details; it isn’t expected to appear in interfaces at all.

[EDIT: Second of all, P0339R6 no longer proposes an explicit specialization of anything! It proposes that the convenience API go into the primary template, as per my “idea number one” below. I have edited out one section below that was merely objecting to the explicit specialization.]

Historically, P0339 is a “compromise paper” that grew out of P0148 “memory_resource_ptr: A Limited Smart Pointer for memory_resource Correctness” (October 2015). To be fair, that paper wasn’t a good idea either; but I do think that “compromise papers” tend (1) to be written prematurely and (2) to be of inferior quality. When you introduce a secondary goal of “get votes from faction X in XWG,” the primary goal of “make a good and usable product” tends to be watered down. (And I say this as the author of a successful compromise paper myself! I still write code about once a year where N4537 shared_ptr::unlock() would be useful, and I still have never used P0163 shared_ptr::weak_type.)

P0339’s example code

Okay, so, what does P0339 propose to allow us to write? From the paper: Here is the “before” code, and here is the “after” code. P0339 shows a dramatic difference between the “before” version of allocating a linked-list node:

using node_alloc = typename alloc_traits::template rebind_alloc<node>;
node_alloc  m_alloc;

using alloc_node_traits =
    typename alloc_traits::template rebind_traits<node>;
node *n = alloc_node_traits::allocate(m_alloc, 1);
alloc_node_traits::construct(m_alloc, &n->m_value, v);
n->m_next = m_head;

and the “after” version:

using allocator_type = std::pmr::polymorphic_allocator<>;
allocator_type m_alloc;

node *n = m_alloc.allocate_object<node>();
m_alloc.construct(&n->m_value, v);
n->m_next = m_head;

However, notice that the “before” version was an STL-style class template taking an allocator parameter, whereas the “after” version is a concrete class type restricted to dealing only with a single allocator type — std::pmr::polymorphic_allocator.

So the main difference between P0339’s StringList1 and StringList2 is that StringList2 removes the allocator template parameter. Removing template parameters does indeed dramatically simplify code, but you don’t need to modify polymorphic_allocator to get that benefit! Let’s compare how the non-parameterized StringList2 would look in pure vanilla C++17: here.

using node_alloc = std::pmr::polymorphic_allocator<node>;
node_alloc m_alloc;

node *n = m_alloc.allocate(1);
m_alloc.construct(&n->m_value, v);
n->m_next = m_head;

That’s right — the vanilla C++17 version of StringList2 is actually simpler than P0339’s proposed C++2a version!

Notice that because StringList2 is not aware of allocator types other than its hard-coded one, we don’t have to go through allocator_traits to get at construct. We know that polymorphic_allocator<node> provides allocate and construct methods that fit our needs here exactly.

The new_object<T> API

For some reason, P0339’s code example chooses not to address the C++ allocator model’s biggest pain point for casual STL users — the separation of allocate from construct. This separation is necessary if you’re implementing std::vector, but it’s not necessary for something like StringList2.

Well, P0339’s example can’t use new_object because struct node is not allocator-aware. node deliberately lacks the constructors that would be needed to pipe the allocator from m_alloc down into node::m_value. That’s why P0339’s example code explicitly calls construct on the n->m_value object, instead of just letting it be constructed by node::node or by new_object.

So it would be cool if we could write simply

node *n = m_alloc.new_object(m_head, v);

And in fact P0339 proposes almost exactly this interface!

However — and this is where it gets icky — P0339 proposes to add this new interface only to polymorphic_allocator<std::byte>, not to any arbitrary polymorphic_allocator. [EDIT: Well, not anymore; but the examples and the overall design haven’t caught up yet.] What this means is that if you want to use polymorphic_allocator<node> in some parts of your code and the-new-interface in other parts, you’ll still have to do a type conversion. Furthermore, because polymorphic_allocator<std::byte> doesn’t know anything about struct node, you’ll have to tell it about node, via a template argument to the new_object function template. That is, rather than writing simply what I wrote above, P0339 actually proposes that you should write

node *n = m_alloc.new_object<node>(m_head, v);

This looks worse again!

Now, there can be an advantage to specifying the object type at new_object’s call-site instead of in the type of m_alloc. The advantage would be if you are going to be allocating many different types of objects. Rather than constantly rebinding m_alloc, as in

A *pa = std::pmr::polymorphic_allocator<A>(m_alloc).new_object(args);
B *pb = std::pmr::polymorphic_allocator<B>(m_alloc).new_object(args);

you could write simply

A *pa = m_alloc.new_object<A>(args);
B *pb = m_alloc.new_object<B>(args);

Perhaps the “best of both worlds” interface, just in terms of concise code (although certainly not in terms of teachability, implementation surface area, or user confusion) would be to provide both, via a single member function template with a defaulted template parameter U:

template<class T>
class polymorphic_allocator {
    template<class U = T, class... Args>
    U *new_object(Args&&...);
};

This would add concrete and idiosyncratic functionality to polymorphic_allocator<T>. Now, I’m not a fan of idiosyncratic functionality. For example, std::allocator<T> would not get new_object automatically. Programmers would have to choose between proper C++11 allocator-awareness and getting to use the shiny toy that is new_object.

(The usual way you add new functionality to a bunch of classes at once is to use a traits class — we could add allocator_traits<A>::new_object(Args&&...) in C++2a — but that wouldn’t help with P0339’s use-case, which is where they don’t want to be parameterized on an allocator type and they don’t want the verbosity of allocator_traits.)

However, there is nothing intrinsically wrong with idiosyncratic functionality. polymorphic_allocator is also the only kind of allocator that supports a .resource() member function. Making it the only kind of allocator that supports .new_object() wouldn’t be terribly different, in principle.

The interaction with CTAD and common typos

Most confusingly, P0339 proposes to give the existing polymorphic_allocator a defaulted template parameter. Before P0339, the following code snippet would be a syntax error. (You forgot the <T>!)

template<class T>
void *allocate_space_for_n_Ts_with_the_default_resource(int n) {
    std::pmr::polymorphic_allocator alloc;
    return alloc.allocate(n);
}

After P0339, thanks to the defaulted template parameter, and partly thanks to CTAD, that code snippet would compile quietly and allocate n bytes of memory, rather than the intended n*sizeof(T) bytes.

Notice that even in a world without CTAD, accidentally writing polymorphic_allocator<> instead of polymorphic_allocator<T> is not unthinkable. A significant number of C++ developers are already confused about which of polymorphic_allocator and memory_resource are templates, which are type-erased, and which are classically polymorphic. (Hint: polymorphic_allocator is not the polymorphic one!) Allowing these developers to write std::pmr::polymorphic_allocator a; as if it were a concrete class type does them a grave disservice.

In short:

  • P0339’s proposed convenience functionality is not as optimally designed as it could be.

  • P0339’s own example shows the inferiority of P0339’s allocate_object<T> functionality, compared to what’s already in C++17.

  • P0339’s library design pointlessly privileges std::byte over all other T.

  • P0339 proposes to add a default template parameter that interacts badly with CTAD, and serves merely to hide bugs.

What might a convenience interface look like?

I would suggest three ways that the C++17 API could be extended to be more user-friendly. But I haven’t thought too deeply about these. You can see from the code snippets above that C++17’s API is already pretty ergonomic, as soon as you abandon the idea of supporting different allocator types; so we’re arguing over very tiny improvements here. I don’t propose any of these ideas for standardization.

Idea number one: Add new_object(Args&&...) to all specializations of polymorphic_allocator, not just to one specialization. [EDIT: P0339R6’s formal wording had already adopted this idea, even though the English text hadn’t caught up.]

template<class T>
class polymorphic_allocator {

    template<class U = T, class... Args>
    U *new_object(Args&&... args) const {
        auto a = polymorphic_allocator<U>(*this);
        U *p = a.allocate(1);
        try {
            return a.construct(p, std::forward<Args>(args)...);
        } catch (...) { a.deallocate(p, 1); throw; }
    }

};

Idea number two: Introduce a non-templated memory_resource_handle which can be used by anyone who wants the convenience functionality, without messing with polymorphic_allocator and the allocator model at all. Using the “handle” model instead of the “allocator” model, we could write a StringList3 that looks like this:

node *n = m_res.allocate<node>(1);
m_res.construct(&n->m_value, v);
n->m_next = m_head;

Here, m_res is a data member of type memory_resource_handle. It doesn’t pretend to be an Allocator, because it doesn’t need to. All accesses to its underlying resource go through the new convenience API, never through the old C++11 allocator API.

Notice that idea number two can be implemented entirely in user code. If you want to use this idiom today in C++17, go right ahead! You don’t need to change the library for this one.


Idea number three: If we weren’t trying to throw up a firewall in between memory_resource and the allocator model, we could just add the convenience API directly to memory_resource itself! For example:

class memory_resource {
    // The base class of every memory resource.
private:
    // The vtable members remain unchanged versus C++17.
    virtual void* do_allocate(size_t bytes, size_t alignment) = 0;
    virtual void do_deallocate(void *p, size_t bytes, size_t alignment) = 0;
    virtual bool do_is_equal(const memory_resource& other) const noexcept = 0;
public:
    // The public API is given extra non-virtual member functions,
    // such as:

    template<class U>
    polymorphic_allocator<U> get_allocator() noexcept {
        return polymorphic_allocator<U>(this);
    }

    template<class U, class... Args>
    U *new_object(Args&&... args) {
        auto a = get_allocator<U>();
        U *p = a.allocate(1);
        try {
            return a.construct(p, std::forward<Args>(args)...);
        } catch (...) { a.deallocate(p, 1); throw; }
    }
};

Using this memory_resource-centric model, a hypothetical StringList4 wouldn’t bother with allocators at all. It would store memory_resource *m_mr internally, and do:

node *n = m_mr->allocate<node>(1);
m_mr->construct(&n->m_value, v);
n->m_next = m_head;

You can simulate this idea in terms of idea number two. All you have to do is implement memory_resource_handle, and then you can deal in terms of memory_resource * internally but wrap the pointer in-line whenever it’s time to use the new API:

node *n = memory_resource_handle(m_mr).allocate<node>(1);
memory_resource_handle(m_mr).construct(&n->m_value, v);
n->m_next = m_head;

Anyway, I think any of these three ideas would be far better than P0339 (because they do not suffer from any of the disadvantages in my bulleted list above). But I don’t want to see them in C++2a. I just don’t want to see P0339 get into C++2a and mess up the situation worse than it was in C++17.

If it ain’t broke, don’t fix it!

Posted 2019-07-02