Hey folks,

If anyone is interested in seeing this API applied in real life code:
https://github.com/llvm/llvm-project/compare/main...brenoguim:llvm-project:breno.mco?diff=split#diff-19c001df6058f7f3e4c8d1cd2856da344c1bfc52a06b8c144540b0d4cc99ff1d

This is my branch of llvm-project where I change the libc++ implementation
of std::shared_ptr<T[]> (and friends) to use xtd::make_contiguous_object.
All shared pointer tests are passing (and I learned so much to make them
pass!)

The highlights in my opinion are:
1. Removed `__bytes_for` method, including a large comment and a link to
wikipedia about padding.
2. Removed the member "union { _Tp data[1]; }" which is a known workaround
to simplify alignment calculation and access.
3. Removed manual exception guard for the whole buffer
4. I *suspect* it's slightly more correct since now the control block is
also constructed with the allocator.

The lowlights:
1. The destructor is a bit odd. Shared_ptr destroys the elements in a
different time than it destroys the control block and deallocates the
memory. Because of that, I cannot pass the layout to
xtd::destroy_contiguous_objects since it would destroy the array (again) +
control block + deallocate. Instead I pass a dummy type.
But that's expected. I think all users will have quirks.
2. The library implementation still has a long way to go. Not even ready
for review.

Overall, the code looks more "average person writable".

Let me know if that adds more clarity to the proposal goal, and if you
agree that the code looks more readable now.

Best Regards,
Breno G.








On Sun, Mar 12, 2023 at 8:34 PM Breno Guimarães <brenorg_at_[hidden]> wrote:

> Gosh, I'll have to rewrite a lot of the library to add _proper_ allocator
> support. But the initial result is very encouraging.
> Even though a few tests are still failing due to allocator support not
> being 100%, I can see how the code will look like in the end.
>
> My patch removes a wikipedia link explaining how to do proper padding, a
> union that is only to simplify alignment, and the code is much more
> readable.
>
> I'll come back once it's ready.
>
>
> On Sun, Mar 12, 2023 at 4:19 PM Breno Guimarães <brenorg_at_[hidden]> wrote:
>
>> Oh ok. The library does provide multiple ways to construct the objects in
>> the array. So I don't think that's an issue.
>> See the test here:
>> https://github.com/brenoguim/make_contiguous_objects/blob/a6a7024348450a72c77c5bd9857aa03aaa8894a0/tests/unit/basic.test.cpp#L91
>> I'm now in process of changing the implementation of libc++ shared_ptr to
>> stress the API. Let's see how it goes.
>>
>> By the way, the configurability of the initialization of each array is
>> highly questionable because I haven't seen anything similar in the
>> standard. So I might need to a more traditional approach?
>> I don't know, what do you think?
>>
>> Thanks!
>> Breno G.
>>
>>
>>
>> On Sun, Mar 12, 2023 at 4:05 PM Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]>
>> wrote:
>>
>>> On Sun, Mar 12, 2023 at 2:50 PM Breno Guimarães <brenorg_at_[hidden]>
>>> wrote:
>>>
>>>> > All our use-cases so far are use-cases for the
>>>> allocating-but-not-constructing entrypoint.
>>>>
>>>> Even on std::make_shared<std::string[]>(10) ? That is a case of
>>>> allocating and constructing.
>>>>
>>>
>>> That default-constructs the 10 string objects, true. But it *doesn't*
>>> default-construct the 1 _ControlBlock object that precedes them:
>>>
>>> https://github.com/llvm/llvm-project/blob/2f887c9a760dfdffa584ce84361912fe122ad79f/libcxx/include/__memory/shared_ptr.h#L1147-L1148
>>> So an entrypoint that default-constructs all Ns... of the Ts... isn't
>>> helpful to this use-case.
>>>
>>> –Arthur
>>>
>>>>