Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<memory>: Add different control block types for allocate_shared_for_overwrite #4274

Merged
merged 6 commits into from
Jan 30, 2024

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Dec 17, 2023

allocate_shared_for_overwrite creates allocated non-array objects by default-initialization, so these objects should be destroyed by plain destructor calls, not Ator::destroy. Currently the standard wording is unclear on make_shared_for_overwrite and allocate_shared_for_overwrite, so I submitted LWG-4024.

It's a bit unfortunate that the existing control block types can't be reused because the _Destroy functions can't recognize _For_overwrite_tag.

Unblocks one libcxx test:

  • std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared_for_overwrite.pass.cpp

Drive-by changes:

  • completing visualizers of control blocks for array types.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner December 17, 2023 16:34
@frederick-vs-ja frederick-vs-ja changed the title <memory>: Add different control block types for allocator_shared_for_overwrite <memory>: Add different control block types for allocate_shared_for_overwrite Dec 17, 2023
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Dec 18, 2023
@StephanTLavavej StephanTLavavej self-assigned this Dec 18, 2023
@StephanTLavavej
Copy link
Member

Do these control blocks need new visualizers?

@frederick-vs-ja
Copy link
Contributor Author

frederick-vs-ja commented Dec 18, 2023

Do these control blocks need new visualizers?

I'm now trying to add visualizer for _Ref_count_obj_alloc_for_overwrite.

But visualization of control block types for array types still seems missing and I have no idea how to handle them. Previous PRs #309, #1315, and the previous issue #2787 didn't mention these stuffs.

Edit: Completed these visualizers. However, it's unfortunate that the control blocks for std::make_shared<TrivialType[]> can't be completely visualized since the size of array is not stored.

Test program

#include <memory>
#include <memory_resource>
#include <string>

int main() {
    // make_shared for arrays of trivial types
    {
        auto p = std::make_shared<int[8]>();
        p[1]   = 42;
    }
    {
        auto p = std::make_shared<int[]>(8);
        p[1]   = 42;
    }
    {
        auto p = std::make_shared_for_overwrite<int[8]>();
        p[1]   = 42;
    }
    {
        auto p = std::make_shared_for_overwrite<int[]>(8);
        p[1]   = 42;
    }
    // make_shared for arrays of non-trivial types
    {
        auto p = std::make_shared<std::string[8]>("s");
        p[1]   = "42";
    }
    {
        auto p = std::make_shared<std::string[]>(8, "s");
        p[1]   = "42";
    }
    {
        auto p = std::make_shared_for_overwrite<std::string[8]>();
        p[1]   = "42";
    }
    {
        auto p = std::make_shared_for_overwrite<std::string[]>(8);
        p[1]   = "42";
    }
    // allocate_shared(_for_overwrite)
    {
        auto p = std::allocate_shared<int[8]>(std::allocator<int>{});
        p[1]   = 42;

        auto q = std::allocate_shared<int[8]>(std::pmr::polymorphic_allocator<int>{});
        q[2]   = -42;
    }
    {
        auto p = std::allocate_shared<int[]>(std::allocator<int>{}, 8);
        p[1]   = 42;

        auto q = std::allocate_shared<int[]>(std::pmr::polymorphic_allocator<int>{}, 8);
        q[2]   = -42;
    }
    {
        auto p = std::allocate_shared_for_overwrite<int>(std::allocator<int>{});
        *p     = 42;

        auto q = std::allocate_shared_for_overwrite<int>(std::pmr::polymorphic_allocator<int>{});
        *q     = -42;
    }
    {
        auto p = std::allocate_shared_for_overwrite<int[8]>(std::allocator<int>{});
        p[1]   = 42;

        auto q = std::allocate_shared_for_overwrite<int[8]>(std::pmr::polymorphic_allocator<int>{});
        q[2]   = -42;
    }
    {
        auto p = std::allocate_shared_for_overwrite<int[]>(std::allocator<int>{}, 8);
        p[1]   = 42;

        auto q = std::allocate_shared_for_overwrite<int[]>(std::pmr::polymorphic_allocator<int>{}, 8);
        q[2]   = -42;
    }
}

stl/inc/memory Show resolved Hide resolved
stl/inc/memory Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

Thanks! 😻 This investigation, fix, and visualizers must have been a lot of work, I really appreciate it. I pushed a couple of tiny commits and I think this is ready to go.

@StephanTLavavej StephanTLavavej removed their assignment Jan 25, 2024
@StephanTLavavej StephanTLavavej self-assigned this Jan 30, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit c0da2af into microsoft:main Jan 30, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for destroying this bug! 🐞 💥 😹

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants