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

Mon Jan 9 02:02:44 PST 2023

bemeryesr added a comment.

In D132149#4010578 <https://reviews.llvm.org/D132149#4010578>, @philnik wrote:

> I think you misunderstood what I was asking for regarding the tests. I wanted you to move the refactoring into it's own patch, so that the test changes here would be just a single line per test.

Ok, got it now, sorry for the misunderstanding. I created 2 additional PRs. The first refactors the unit tests (D140550 <https://reviews.llvm.org/D140550>) and the second applies clang formatting to all the string tests (D140612 <https://reviews.llvm.org/D140612>).

================
Comment at: libcxx/include/string:778

+#ifdef _LIBCPP_CXX03_LANG
     union __ulx{__long __lx; __short __lxx;};
----------------
philnik wrote:
> bemeryesr wrote:
> > philnik wrote:
> > > Why is this special-casing needed? AFAICT the code below is valid in C++03.
> > This is needed because 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 __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. 
> That doesn't change anything between the two versions though. C++03 doesn't work with fancy pointers either way. Actually, why do we need a default constructor in the union at all?
Ah yeah, you're right. It seems that __ulx is only used by the sizeof operator below so we don't need a default constructor. I've removed the special casing.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132149/new/

https://reviews.llvm.org/D132149