- Reject Initialization of polymorphic_allocator from nullptr per [mem.poly.allocator.ctor]/2.
- Use new char[n] instead of malloc(n) to avoid warnings about dereferencing potentially null pointers.
- MSVCSTL doesn't implement LWG-3120 (cursed ABI); XFAIL monotonic_buffer_resource::reset test.
- Don't expect to portably be able to allocate 0 bytes from a potentially-exhausted block in monotonic_buffer_resource. Zero-byte allocations may still consume block space to provide that guarantee that a return value cannot repeat absent deallocation/reset. LWG-3637 "pmr::memory_resource::do_allocate needs clarification" is pertinent here.
- MSVCSTL's pool resources use a vector internally, so they perform an additional allocation when iterator debugging. This requires adjustment to the expected number of allocations for some tests, and completely disabling the case that sets the default resource to null_memory_resource.
Details
- Reviewers
philnik - Group Reviewers
Restricted Project
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp | ||
---|---|---|
41 | Does this change alignment guarantees? | |
libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/resource.pass.cpp | ||
36 | I think this should be removed and we should add a _LIBCPP_ASSERT(__r != nullptr, "memory_resource pointer has to be non-null"); in the constructor instead. | |
libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/select_on_container_copy_construction.pass.cpp | ||
29 | Missing <cstdlib> include. | |
60 | Why the special-casing for libc++? | |
libcxx/test/std/utilities/utility/mem.res/mem.res.monotonic.buffer/mem.res.monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp | ||
46 | Couldn't you just test this downstream and just have #ifndef _MSVC_STL_VERSION around the assert? | |
libcxx/test/std/utilities/utility/mem.res/mem.res.monotonic.buffer/mem.res.monotonic.buffer.mem/allocate_with_initial_size.pass.cpp | ||
38 | What's the problem here? | |
libcxx/test/std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/ctor_does_not_allocate.pass.cpp | ||
23–26 | AFAICT this is non-conforming and should therefore not be tested here. Instead, this test should be XFAILed for the problematic case, since it's all that is tested. Same for the cases below. | |
libcxx/test/std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/sync_with_default_resource.pass.cpp | ||
23 | What is the problem here? Same question below. |
libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp | ||
---|---|---|
41 | It does not. malloc (C 7.24.3) returns a pointer aligned for "any type of object with a fundamental alignment requirement and size less than or equal to the size requested", whereas new char[n] gets storage from operator new[] ([basic.stc.dynamic.allocation]/3.2) which is "aligned for any object that does not have new-extended alignment and is no larger than the request size" (at least as strict a requirement as "fundamental alignment") which it might offset by a multiple of the fundamental alignment requirement ([expr.new]/16). | |
libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/select_on_container_copy_construction.pass.cpp | ||
60 | Just to preserve the nullptr behavior. Since you've suggested above that the constructor should refuse nullptr - which I agree is a good idea - I'll remove the special-case here. | |
libcxx/test/std/utilities/utility/mem.res/mem.res.monotonic.buffer/mem.res.monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp | ||
46 | I want the test to break loudly when we implement LWG-3120, which honestly I could do as well by XFAILing it in our out-of-band control structure. On further thought I believe that's preferable: the cases that do pass here pass for the wrong reasons, so this test would be incredibly fragile for us. I'll revert this part of the change (leaving the comment fix and the change from assert to ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS). | |
libcxx/test/std/utilities/utility/mem.res/mem.res.monotonic.buffer/mem.res.monotonic.buffer.mem/allocate_with_initial_size.pass.cpp | ||
38 | LWG-3637 will possibly make it a requirement that allocate may not return the same pointer twice without intervening deallocation. That requirement can't be satisfied by an implementation that allocates 0 bytes from an exhausted block - as test(1) expects - since repeated calls would presumably return the same address. Making this case libc++-only allows implementations like MSVCSTL to pass as well. | |
libcxx/test/std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/ctor_does_not_allocate.pass.cpp | ||
23–26 | I believe it is conforming; the Standard gives a lot of leeway with "It is unspecified if, or under what conditions, this constructor calls upstream->allocate()" ([mem.res.pool.ctor]/3). | |
libcxx/test/std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/sync_with_default_resource.pass.cpp | ||
23 | Same as the previous test: our pool resources allocate on construction when iterator debugging is active, so line 26 and 27 fail horribly. I believe this part of the test is non-portable. Maybe it would be better to make it libc++-specific? (Same response below.) |
libcxx/test/std/utilities/utility/mem.res/mem.poly.allocator.class/mem.poly.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp | ||
---|---|---|
41 | Thanks for the info! | |
libcxx/test/std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/ctor_does_not_allocate.pass.cpp | ||
23–26 | I guess you are right. IMO this should be a libc++-specific test then, since it tests implementation details which are unspecified in the standard. | |
libcxx/test/std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/sync_with_default_resource.pass.cpp | ||
23 | Yes, I think this should be libc++ specific then. |
Abandoning this for now since I haven't had time to complete it. I'll bring this back on GitHub when I find the time.
Does this change alignment guarantees?