[libcxx-commits] [PATCH] D132149: [String] Allow fancy pointer as pointer type of basic_string allocator

Wed Aug 31 00:37:55 PDT 2022

bemeryesr created this revision.
Herald added a project: All.
bemeryesr edited the summary of this revision.
bemeryesr edited the summary of this revision.
bemeryesr edited the summary of this revision.
bemeryesr edited the summary of this revision.
bemeryesr edited the summary of this revision.
bemeryesr edited the summary of this revision.
bemeryesr edited the summary of this revision.
bemeryesr updated this revision to Diff 453896.
bemeryesr edited the summary of this revision.
bemeryesr added a comment.
bemeryesr updated this revision to Diff 453931.
bemeryesr updated this revision to Diff 453939.
bemeryesr updated this revision to Diff 454047.
bemeryesr edited the summary of this revision.
bemeryesr updated this revision to Diff 454401.
bemeryesr updated this revision to Diff 454412.
bemeryesr updated this revision to Diff 454440.
bemeryesr updated this revision to Diff 454450.
bemeryesr updated this revision to Diff 454459.
bemeryesr updated this revision to Diff 456424.
bemeryesr updated this revision to Diff 456573.
bemeryesr updated this revision to Diff 456582.
bemeryesr updated this revision to Diff 456628.
bemeryesr published this revision for review.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

Use regular instead of curly braces in member initializer list

bemeryesr added a comment.

Change noexcept specifier to _NOEXCEPT macro to support c++03

bemeryesr added a comment.

Fix clang tidy formatting issues

bemeryesr added a comment.

Support fancy pointers only for C++11 and later

The C++03 standard says "An object of a class with a non-trivial default constructor, 
a non-trivial copy constructor or a non-trivial copy assignment operator cannot be a 
member of a union. Therefore, we cannot support fancy pointers in C++03.

bemeryesr added a comment.

Include string before checking preprocessor macro definition

bemeryesr added a comment.

Apply clang formatting

bemeryesr added a comment.

Incorporate changes from master

bemeryesr added a comment.

Move test mains outside ifdef check

bemeryesr added a comment.

Add expected-no-diagnostics to empty fail test

bemeryesr added a comment.

Add templated copy constructor

bemeryesr added a comment.

Add pointer and iterator traits to test classes

bemeryesr added a comment.

Use remove_cv::type for c++11 compatibility.

bemeryesr added a comment.

Use add_lvalue_reference::type for c++11 compatibility.

Currently, basic_string does not support custom allocators which use fancy pointers that are not trivially constructible and destructible as the pointer type. This is due to requirements of the internal string representation, `__rep`. In order to use a pointer type which is not trivially constructible, `__rep` should:

- be a literal type (required by the implementation). This requires that `__rep` has at least one constexpr constructor and that its destructor is trivial.
- provide an explicit constructor, copy constructor and copy assignment operator (default special member functions are deleted since `__long`, a variant member of `__rep`, is not trivially constructible as it contains a fancy pointer as a member variable).

This change allows the use of fancy pointers which are not trivially constructible by adding an explicit and constexpr constructor, copy constructor and copy assignment operator. It also adds a static assert which will fail if the fancy pointer is not trivially destructible.

Addresses bug report: https://bugs.llvm.org/show_bug.cgi?id=20508

**Implementation details:**

**Constant evaluation:** From above, we have the requirement that `__rep` has at least one constexpr constructor. This means that we cannot use non-const functions such as memcpy or memset in the copy constructor / copy assigment operators. Additionally, the active element of a union is not allowed to be changed inside a constexpr constructor. Since during constant evaluation, short string optimization is disabled for basic_string, the active element of the `__rep` union can be `__l` and we can manually copy values from `other.__l` to `this->__l`.

**Non-constant evaluation:** During non-constant evaluation, the active member of `__rep` can be `__rep.__s` or `__rep.__l` i.e. a short or long string. In this case, we rely on type punning in the copy constructor and copy assignment operators to read the value of `other.__l`, even if it isn't the active member. This appears to be legal and also follows the same rationale used when reading / writing memory in `__rep.__r`.

**C++03 / C++11 and on support:**

The C++11 standard says "If any non-static data member of a union has a non-trivial default constructor (12.1), copy constructor (12.8), move constructor (12.8), copy assignment operator (12.8), move assignment operator (12.8), or destructor (12.4), the corresponding member function of the union must be user-provided or it will be implicitly deleted (8.4.3) for the union." (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf, 9.5.2).

However, the C++03 standard says "An object of a class with a non-trivial default constructor (12.1), a non-trivial copy constructor (12.8), a non-trivial destructor (12.4), or a non-trivial copy assignment operator (13.5.3, 12.8) cannot be a member of a union". Since the `__long` contains a fancy pointer which is not trivially constructible, `__long` itself is not trivially constructible and therefore `__rep` cannot include it as a member in C++03. Therefore, fancy pointers can only be supported for C++11 and later.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132149

Files:
  libcxx/include/string
  libcxx/test/libcxx/strings/basic.string/allocator_with_fancy_pointer.pass.cpp
  libcxx/test/libcxx/strings/basic.string/allocator_with_fancy_pointer_non_trivial_destructor.fail.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132149.456628.patch
Type: text/x-patch
Size: 9211 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220831/1ef9051e/attachment.bin>