<memory>: Add different control block types for allocate_shared_for_overwrite by frederick-vs-ja · Pull Request #4274 · microsoft/STL (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits6 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

frederick-vs-ja

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:

Drive-by changes:

@frederick-vs-ja

@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

Do these control blocks need new visualizers?

@frederick-vs-ja

@frederick-vs-ja

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

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_sharedstd::string[8]("s"); p[1] = "42"; } { auto p = std::make_sharedstd::string[](8, "s"); p[1] = "42"; } { auto p = std::make_shared_for_overwritestd::string[8](); p[1] = "42"; } { auto p = std::make_shared_for_overwritestd::string[](8); p[1] = "42"; } // allocate_shared(_for_overwrite) { auto p = std::allocate_shared<int[8]>(std::allocator{}); 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;
}

}

@frederick-vs-ja

@frederick-vs-ja

@StephanTLavavej

@StephanTLavavej

StephanTLavavej

StephanTLavavej

@StephanTLavavej

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

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

@StephanTLavavej

Thanks for destroying this bug! 🐞 💥 😹

Labels

bug

Something isn't working

2 participants

@frederick-vs-ja @StephanTLavavej