basic_string does not respect NullablePointer requirements of the allocator's pointer

[Quuxplusone]

Bugzilla Link	PR20508
Status	NEW
Importance	P normal
Reported by	Thomas Koeppe (tkoeppe@google.com)
Reported on	2014-08-01 04:38:11 -0700
Last modified on	2017-10-05 12:13:55 -0700
Version	unspecified
Hardware	All All
CC	arthur.j.odwyer@gmail.com, llvm-bugs@lists.llvm.org, mclow.lists@gmail.com
Fixed by commit(s)
Attachments	string_offptr.cpp (3685 bytes, text/x-c++src)
Blocks
Blocked by
See also	PR20616

Created attachment 12843
Compilation failure of basic_string with fancy pointer allocator

I'm attaching a complete example, but the problem is easily described:

The pointer type, std::allocator_traits<A>::pointer, is not required to be
trivially constructible. In fact, it is *impossible* for this type to be
trivially constructible if it is a class type, since it must value-initialize
to the null value.

However, the current implementation of basic_string assumes that the pointer
type is trivially constructible (since it puts the type into a union without
any user-provided constructors). This makes it impossible to use basic_string
with fancy pointers.

I haven't thought too much about a solution, but I think that adding suitable
constructors to the SSO union __rep and to the type __long would make this work
and wouldn't be too invasive a change.

[Quuxplusone]

Attached string_offptr.cpp (3685 bytes, text/x-c++src): Compilation failure of basic_string with fancy pointer allocator

[Quuxplusone]

(I think this bug is much harder than 20616. 20616 is about type erased allocators, and the class design is mostly good already, and there's only a small amount of interface detail that needs generalizing. This bug here affects the actual class design of the SSO implementation.)

[Quuxplusone]

Jonathan Wakely pointed out an error in my analysis: NullablePointers *may*
indeed be trivially constructible (as long as their null state can be obtained
by value initialization). However, they are not required to be trivially
constructible.

So the current libc++ string only works with trivially constructible fancy
pointers.

[Quuxplusone]

In particular, basic_string does not work with
boost::interprocess::offset_ptr<char> because offset_ptr's "NULL pointer"
representation is not all-bits-zero and thus offset_ptr cannot have a trivial
default constructor.

Likewise, basic_string does not work with any fancy pointer type which is not
trivially DEstructible, either. This could be made compilable-but-not-
necessarily-correct by giving "struct __rep" a non-defaulted destructor. IMO it
would be better in the short term to
static_assert(is_trivially_destructible_v<pointer>), so as to improve the user
experience and document libc++'s assumptions.

string:750:31: error: constructor for 'std::__1::basic_string<char,
std::__1::char_traits<char>, my::allocator<char> >' must explicitly initialize
the member '__r_' which does not have a default constructor

https://wandbox.org/permlink/gCyhJ8904LMfpD53
====
#include <string>

// #define FIRST_FAILURE
// #define SECOND_FAILURE

namespace my {

template<class T>
class nontrivial_ptr {
    T *ptr;
public:
#ifdef FIRST_FAILURE
    nontrivial_ptr() {}  // deliberately non-trivial ctor (e.g. offset_ptr)
#else
    nontrivial_ptr() = default;
#endif
    nontrivial_ptr(T *p) : ptr(p) {}
    operator T*() const { return ptr; }
    nontrivial_ptr(nontrivial_ptr&&) = default;
    nontrivial_ptr(const nontrivial_ptr&) = default;
    nontrivial_ptr& operator=(nontrivial_ptr&&) = default;
    nontrivial_ptr& operator=(const nontrivial_ptr&) = default;
#ifdef SECOND_FAILURE
    ~nontrivial_ptr() {}  // deliberately non-trivial dtor (e.g. imperfect programming?)
#else
    ~nontrivial_ptr() = default;
#endif
    T& operator*() const { return *ptr; }
    T* operator->() const { return ptr; }
    operator bool() const { return ptr; }
};

template<class T>
class allocator : public std::allocator<T> {
    using base = std::allocator<T>;
public:
    using pointer = nontrivial_ptr<T>;
    pointer allocate(size_t n) { return base::allocate(n); }
    void deallocate(pointer p, size_t n) { return base::deallocate(p, n); }
};
} // namespace my

int main()
{
    std::basic_string<char, std::char_traits<char>, my::allocator<char>> s;
}