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

[tkoeppe]

Bugzilla Link	20508
Version	unspecified
OS	All
Attachments	Compilation failure of basic_string with fancy pointer allocator
CC	@Quuxplusone,@mclow

Extended Description

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

The pointer type, std::allocator_traits::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.

mentioned in issue llvm/llvm-bugzilla-archive#20616

[Quuxplusone]

In particular, basic_string does not work with boost::interprocess::offset_ptr 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), 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, my::allocator >' must explicitly initialize the member 'r_' which does not have a default constructor

https://wandbox.org/permlink/gCyhJ8904LMfpD53

include

// #define FIRST_FAILURE // #define SECOND_FAILURE

namespace my {

template 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 allocator : public std::allocator { using base = std::allocator; public: using pointer = nontrivial_ptr; 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, my::allocator> s; }

[tkoeppe]

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.

[tkoeppe]

(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.)