C++ Logo

std-proposals

Advanced search

Re: [std-proposals] Making contiguous objects

From: Breno Guimarães <brenorg_at_[hidden]>
Date: Fri, 17 Mar 2023 20:23:50 -0300
Ping in case anyone wants to take a look.
I think the last email is really the star of the proposal, showing how much
simpler the library implementation of std::shared_ptr<T[]> could have been.

But I also added more content to
https://github.com/brenoguim/make_contiguous_objects/blob/main/README.md
Including a discussion on RAII, and alternative APIs.

Thanks,
Breno G.




On Mon, Mar 13, 2023 at 8:07 PM Breno Guimarães <brenorg_at_[hidden]> wrote:

> 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
>>>>
>>>>>

Received on 2023-03-17 23:24:02