From bd2e949869cd238d5d94812007953cf6fe6f4d84 Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Tue, 29 May 2018 00:08:47 +0000 Subject: [PATCH] LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()" Patch from Arthur O'Dwyer. In the TS, `uses_allocator` construction for `pair` tried to use an allocator type of `memory_resource*`, which is incorrect because `memory_resource*` is not an allocator type. LWG 2969 fixed it to use `polymorphic_allocator` as the allocator type instead. https://wg21.link/lwg2969 (D47090 included this in ``; at Eric's request, I've split this out into its own patch applied to the existing `` instead.) Reviewed as https://reviews.llvm.org/D47109 llvm-svn: 333384 --- libcxx/include/experimental/memory_resource | 22 +-- .../construct_piecewise_pair.pass.cpp | 12 +- .../construct_pair_const_lvalue_pair.pass.cpp | 30 +++- .../construct_pair_rvalue.pass.cpp | 29 +++- .../construct_pair_values.pass.cpp | 33 +++- .../construct_piecewise_pair.pass.cpp | 43 +++++- .../construct_piecewise_pair_evil.pass.cpp | 141 ++++++++++++++++++ .../construct_types.pass.cpp | 41 ++++- libcxx/test/support/test_memory_resource.hpp | 2 +- 9 files changed, 318 insertions(+), 35 deletions(-) create mode 100644 libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair_evil.pass.cpp diff --git a/libcxx/include/experimental/memory_resource b/libcxx/include/experimental/memory_resource index d101f3e08113..748e2e787578 100644 --- a/libcxx/include/experimental/memory_resource +++ b/libcxx/include/experimental/memory_resource @@ -206,7 +206,7 @@ public: void construct(_Tp* __p, _Ts &&... __args) { _VSTD_LFTS::__lfts_user_alloc_construct( - __p, resource(), _VSTD::forward<_Ts>(__args)... + __p, *this, _VSTD::forward<_Ts>(__args)... ); } @@ -218,14 +218,14 @@ public: ::new ((void*)__p) pair<_T1, _T2>(piecewise_construct , __transform_tuple( typename __lfts_uses_alloc_ctor< - _T1, memory_resource*, _Args1... + _T1, polymorphic_allocator&, _Args1... >::type() , _VSTD::move(__x) , typename __make_tuple_indices::type{} ) , __transform_tuple( typename __lfts_uses_alloc_ctor< - _T2, memory_resource*, _Args2... + _T2, polymorphic_allocator&, _Args2... >::type() , _VSTD::move(__y) , typename __make_tuple_indices::type{} @@ -289,23 +289,23 @@ private: template _LIBCPP_INLINE_VISIBILITY - tuple + tuple __transform_tuple(integral_constant, tuple<_Args...> && __t, - __tuple_indices<_Idx...>) const + __tuple_indices<_Idx...>) { - using _Tup = tuple; - return _Tup(allocator_arg, resource(), + using _Tup = tuple; + return _Tup(allocator_arg, *this, _VSTD::get<_Idx>(_VSTD::move(__t))...); } template _LIBCPP_INLINE_VISIBILITY - tuple<_Args&&..., memory_resource*> + tuple<_Args&&..., polymorphic_allocator&> __transform_tuple(integral_constant, tuple<_Args...> && __t, - __tuple_indices<_Idx...>) const + __tuple_indices<_Idx...>) { - using _Tup = tuple<_Args&&..., memory_resource*>; - return _Tup(_VSTD::get<_Idx>(_VSTD::move(__t))..., resource()); + using _Tup = tuple<_Args&&..., polymorphic_allocator&>; + return _Tup(_VSTD::get<_Idx>(_VSTD::move(__t))..., *this); } _LIBCPP_INLINE_VISIBILITY diff --git a/libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp b/libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp index 83b304196371..40bacbcbad59 100644 --- a/libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp +++ b/libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp @@ -79,12 +79,12 @@ struct CountCopies { }; struct CountCopiesAllocV1 { - typedef ex::memory_resource* allocator_type; - allocator_type alloc; + typedef ex::polymorphic_allocator allocator_type; + ex::memory_resource *alloc; int count; CountCopiesAllocV1() : alloc(nullptr), count(0) {} CountCopiesAllocV1(std::allocator_arg_t, allocator_type const& a, - CountCopiesAllocV1 const& o) : alloc(a), count(o.count + 1) + CountCopiesAllocV1 const& o) : alloc(a.resource()), count(o.count + 1) {} CountCopiesAllocV1(CountCopiesAllocV1 const& o) : count(o.count + 1) {} @@ -92,12 +92,12 @@ struct CountCopiesAllocV1 { struct CountCopiesAllocV2 { - typedef ex::memory_resource* allocator_type; - allocator_type alloc; + typedef ex::polymorphic_allocator allocator_type; + ex::memory_resource *alloc; int count; CountCopiesAllocV2() : alloc(nullptr), count(0) {} CountCopiesAllocV2(CountCopiesAllocV2 const& o, allocator_type const& a) - : alloc(a), count(o.count + 1) + : alloc(a.resource()), count(o.count + 1) { } CountCopiesAllocV2(CountCopiesAllocV2 const& o) : count(o.count + 1) {} diff --git a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp index acc42d39f60c..666d8f7807d4 100644 --- a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp +++ b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp @@ -90,6 +90,32 @@ void test_pmr_uses_allocator(std::pair const& p) assert((doTest(UA_AllocArg, UA_None, p))); } } + +template +void test_pmr_not_uses_allocator(std::pair const& p) +{ + { + using T = NotUsesAllocator; + using U = NotUsesAllocator; + assert((doTest(UA_None, UA_None, p))); + } + { + using T = UsesAllocatorV1; + using U = UsesAllocatorV2; + assert((doTest(UA_None, UA_None, p))); + } + { + using T = UsesAllocatorV2; + using U = UsesAllocatorV3; + assert((doTest(UA_None, UA_None, p))); + } + { + using T = UsesAllocatorV3; + using U = NotUsesAllocator; + assert((doTest(UA_None, UA_None, p))); + } +} + template struct Print; @@ -103,7 +129,7 @@ int main() int y = 42; const std::pair p(x, y); test_pmr_uses_allocator(p); - test_pmr_uses_allocator(p); + test_pmr_not_uses_allocator(p); test_pmr_uses_allocator(p); } { @@ -111,7 +137,7 @@ int main() int y = 42; const std::pair p(x, std::move(y)); test_pmr_uses_allocator(p); - test_pmr_uses_allocator(p); + test_pmr_not_uses_allocator(p); test_pmr_uses_allocator(p); } } diff --git a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_rvalue.pass.cpp b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_rvalue.pass.cpp index 05cf82cfbd2b..9e316991c022 100644 --- a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_rvalue.pass.cpp +++ b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_rvalue.pass.cpp @@ -90,6 +90,31 @@ void test_pmr_uses_allocator(std::pair&& p) } } +template +void test_pmr_not_uses_allocator(std::pair&& p) +{ + { + using T = NotUsesAllocator; + using U = NotUsesAllocator; + assert((doTest(UA_None, UA_None, std::move(p)))); + } + { + using T = UsesAllocatorV1; + using U = UsesAllocatorV2; + assert((doTest(UA_None, UA_None, std::move(p)))); + } + { + using T = UsesAllocatorV2; + using U = UsesAllocatorV3; + assert((doTest(UA_None, UA_None, std::move(p)))); + } + { + using T = UsesAllocatorV3; + using U = NotUsesAllocator; + assert((doTest(UA_None, UA_None, std::move(p)))); + } +} + int main() { using ERT = std::experimental::erased_type; @@ -100,7 +125,7 @@ int main() int y = 42; std::pair p(x, std::move(y)); test_pmr_uses_allocator(std::move(p)); - test_pmr_uses_allocator(std::move(p)); + test_pmr_not_uses_allocator(std::move(p)); test_pmr_uses_allocator(std::move(p)); } { @@ -108,7 +133,7 @@ int main() int y = 42; std::pair p(std::move(x), y); test_pmr_uses_allocator(std::move(p)); - test_pmr_uses_allocator(std::move(p)); + test_pmr_not_uses_allocator(std::move(p)); test_pmr_uses_allocator(std::move(p)); } } diff --git a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_values.pass.cpp b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_values.pass.cpp index 1a76072661d6..f2f7712a4467 100644 --- a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_values.pass.cpp +++ b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_values.pass.cpp @@ -93,6 +93,35 @@ void test_pmr_uses_allocator(TT&& t, UU&& u) } } +template +void test_pmr_not_uses_allocator(TT&& t, UU&& u) +{ + { + using T = NotUsesAllocator; + using U = NotUsesAllocator; + assert((doTest(UA_None, UA_None, + std::forward(t), std::forward(u)))); + } + { + using T = UsesAllocatorV1; + using U = UsesAllocatorV2; + assert((doTest(UA_None, UA_None, + std::forward(t), std::forward(u)))); + } + { + using T = UsesAllocatorV2; + using U = UsesAllocatorV3; + assert((doTest(UA_None, UA_None, + std::forward(t), std::forward(u)))); + } + { + using T = UsesAllocatorV3; + using U = NotUsesAllocator; + assert((doTest(UA_None, UA_None, + std::forward(t), std::forward(u)))); + } +} + int main() { using ERT = std::experimental::erased_type; @@ -102,14 +131,14 @@ int main() int x = 42; int y = 42; test_pmr_uses_allocator(x, std::move(y)); - test_pmr_uses_allocator(x, std::move(y)); + test_pmr_not_uses_allocator(x, std::move(y)); test_pmr_uses_allocator(x, std::move(y)); } { int x = 42; const int y = 42; test_pmr_uses_allocator(std::move(x), y); - test_pmr_uses_allocator(std::move(x), y); + test_pmr_not_uses_allocator(std::move(x), y); test_pmr_uses_allocator(std::move(x), y); } } diff --git a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp index 8f78521995d8..27807c1f4ca8 100644 --- a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp +++ b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp @@ -83,6 +83,35 @@ void test_pmr_uses_allocator(std::tuple ttuple, std::tuple } } +template +void test_pmr_not_uses_allocator(std::tuple ttuple, std::tuple utuple) +{ + { + using T = NotUsesAllocator; + using U = NotUsesAllocator; + assert((doTest(UA_None, UA_None, + std::move(ttuple), std::move(utuple)))); + } + { + using T = UsesAllocatorV1; + using U = UsesAllocatorV2; + assert((doTest(UA_None, UA_None, + std::move(ttuple), std::move(utuple)))); + } + { + using T = UsesAllocatorV2; + using U = UsesAllocatorV3; + assert((doTest(UA_None, UA_None, + std::move(ttuple), std::move(utuple)))); + } + { + using T = UsesAllocatorV3; + using U = NotUsesAllocator; + assert((doTest(UA_None, UA_None, + std::move(ttuple), std::move(utuple)))); + } +} + int main() { using ERT = std::experimental::erased_type; @@ -91,7 +120,7 @@ int main() { std::tuple<> t1; test_pmr_uses_allocator(t1, t1); - test_pmr_uses_allocator(t1, t1); + test_pmr_not_uses_allocator(t1, t1); test_pmr_uses_allocator(t1, t1); } { @@ -99,8 +128,8 @@ int main() std::tuple<> t2; test_pmr_uses_allocator(t1, t2); test_pmr_uses_allocator(t2, t1); - test_pmr_uses_allocator(t1, t2); - test_pmr_uses_allocator(t2, t1); + test_pmr_not_uses_allocator(t1, t2); + test_pmr_not_uses_allocator(t2, t1); test_pmr_uses_allocator(t1, t2); test_pmr_uses_allocator(t2, t1); } @@ -111,8 +140,8 @@ int main() std::tuple t2(x, std::move(dx)); test_pmr_uses_allocator( t1, std::move(t2)); test_pmr_uses_allocator(std::move(t2), t1); - test_pmr_uses_allocator( t1, std::move(t2)); - test_pmr_uses_allocator(std::move(t2), t1); + test_pmr_not_uses_allocator( t1, std::move(t2)); + test_pmr_not_uses_allocator(std::move(t2), t1); test_pmr_uses_allocator( t1, std::move(t2)); test_pmr_uses_allocator(std::move(t2), t1); } @@ -126,8 +155,8 @@ int main() std::tuple t2(x, std::move(dx), s); test_pmr_uses_allocator( t1, std::move(t2)); test_pmr_uses_allocator(std::move(t2), t1); - test_pmr_uses_allocator( t1, std::move(t2)); - test_pmr_uses_allocator(std::move(t2), t1); + test_pmr_not_uses_allocator( t1, std::move(t2)); + test_pmr_not_uses_allocator(std::move(t2), t1); test_pmr_uses_allocator( t1, std::move(t2)); test_pmr_uses_allocator(std::move(t2), t1); } diff --git a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair_evil.pass.cpp b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair_evil.pass.cpp new file mode 100644 index 000000000000..f087ec23ca2b --- /dev/null +++ b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair_evil.pass.cpp @@ -0,0 +1,141 @@ +//===----------------------------------------------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: c++98, c++03 + +// + +// template class polymorphic_allocator + +// template +// void polymorphic_allocator::construct(pair*, piecewise_construct_t +// tuple, tuple) + +#include +#include +#include +#include +#include +#include + +#include "test_macros.h" + +namespace ex = std::experimental::pmr; + +template +struct EvilAlloc { + explicit EvilAlloc() : inner_(ex::null_memory_resource()) {} + + EvilAlloc(ex::polymorphic_allocator & a) : inner_(a) {} + EvilAlloc(ex::polymorphic_allocator && a) : inner_(a) {} + EvilAlloc(ex::polymorphic_allocator const & a) = delete; + EvilAlloc(ex::polymorphic_allocator const && a) = delete; + + using value_type = T; + template EvilAlloc(EvilAlloc const & rhs) : inner_(rhs.inner_) {} + + ex::polymorphic_allocator inner_; +}; + +struct WidgetV0 { + WidgetV0(int v) : value_(v) {} + + bool holds(int v, const ex::polymorphic_allocator&) const { + return value_ == v; + } +private: + int value_; +}; + +struct WidgetV1 { + using allocator_type = EvilAlloc; + + WidgetV1(int v) : value_(v), alloc_() {} + WidgetV1(std::allocator_arg_t, EvilAlloc a, int v) : value_(v), alloc_(a) {} + + bool holds(int v, const ex::polymorphic_allocator& a) const { + return value_ == v && alloc_.inner_ == a; + } +private: + int value_; + EvilAlloc alloc_; +}; + +struct WidgetV2 { + using allocator_type = EvilAlloc; + + WidgetV2(int v) : value_(v), alloc_() {} + WidgetV2(int v, EvilAlloc a) : value_(v), alloc_(a) {} + + bool holds(int v, ex::polymorphic_allocator a) const { + return value_ == v && alloc_.inner_ == a; + } +private: + int value_; + EvilAlloc alloc_; +}; + +struct WidgetV3 { + using allocator_type = EvilAlloc; + + WidgetV3(int v) : value_(v), alloc_() {} + WidgetV3(std::allocator_arg_t, EvilAlloc a, int v) : value_(v), alloc_(a) {} + WidgetV3(int v, EvilAlloc a) : value_(v), alloc_(a) {} + + bool holds(int v, ex::polymorphic_allocator a) const { + return value_ == v && alloc_.inner_ == a; + } +private: + int value_; + EvilAlloc alloc_; +}; + +static_assert(std::uses_allocator>::value, ""); +static_assert(std::uses_allocator>::value, ""); +static_assert(std::uses_allocator>::value, ""); +static_assert(std::uses_allocator>::value, ""); +static_assert(std::uses_allocator>::value, ""); +static_assert(std::uses_allocator>::value, ""); + +template +void test_evil() +{ + using PMA = ex::polymorphic_allocator; + PMA pma(ex::new_delete_resource()); + { + using Pair = std::pair; + void *where = std::malloc(sizeof (Pair)); + Pair *p = (Pair *)where; + pma.construct(p, std::piecewise_construct, std::make_tuple(42), std::make_tuple(42)); + assert(p->first.holds(42, pma)); + assert(p->second.holds(42, pma)); + pma.destroy(p); + std::free(where); + } +} + +int main() +{ + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); + test_evil(); +} diff --git a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_types.pass.cpp b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_types.pass.cpp index 3e83173555be..a619194fec17 100644 --- a/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_types.pass.cpp +++ b/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_types.pass.cpp @@ -126,6 +126,39 @@ void test_pmr_uses_alloc(Args&&... args) } } +template +void test_pmr_not_uses_alloc(Args&&... args) +{ + TestResource R(12435); + ex::memory_resource* M = &R; + { + // NotUsesAllocator provides valid signatures for each uses-allocator + // construction but does not supply the required allocator_type typedef. + // Test that we can call these constructors manually without + // polymorphic_allocator interfering. + using T = NotUsesAllocator; + assert(doTestUsesAllocV0(std::forward(args)...)); + assert((doTestUsesAllocV1(M, std::forward(args)...))); + assert((doTestUsesAllocV2(M, std::forward(args)...))); + } + { + // Test T(std::allocator_arg_t, Alloc const&, Args...) construction + using T = UsesAllocatorV1; + assert((doTest(UA_None, std::forward(args)...))); + } + { + // Test T(Args..., Alloc const&) construction + using T = UsesAllocatorV2; + assert((doTest(UA_None, std::forward(args)...))); + } + { + // Test that T(std::allocator_arg_t, Alloc const&, Args...) construction + // is preferred when T(Args..., Alloc const&) is also available. + using T = UsesAllocatorV3; + assert((doTest(UA_None, std::forward(args)...))); + } +} + // Test that polymorphic_allocator does not prevent us from manually // doing non-pmr uses-allocator construction. template @@ -167,16 +200,16 @@ int main() const int cvalue = 43; { test_pmr_uses_alloc(); - test_pmr_uses_alloc(); + test_pmr_not_uses_alloc(); test_pmr_uses_alloc(); test_pmr_uses_alloc(value); - test_pmr_uses_alloc(value); + test_pmr_not_uses_alloc(value); test_pmr_uses_alloc(value); test_pmr_uses_alloc(cvalue); - test_pmr_uses_alloc(cvalue); + test_pmr_not_uses_alloc(cvalue); test_pmr_uses_alloc(cvalue); test_pmr_uses_alloc(cvalue, std::move(value)); - test_pmr_uses_alloc(cvalue, std::move(value)); + test_pmr_not_uses_alloc(cvalue, std::move(value)); test_pmr_uses_alloc(cvalue, std::move(value)); } { diff --git a/libcxx/test/support/test_memory_resource.hpp b/libcxx/test/support/test_memory_resource.hpp index b3472c8b6105..4b8167ba0393 100644 --- a/libcxx/test/support/test_memory_resource.hpp +++ b/libcxx/test/support/test_memory_resource.hpp @@ -28,7 +28,7 @@ // because it can't include template <> struct TransformErasedTypeAlloc { - using type = std::experimental::pmr::memory_resource*; + using type = std::experimental::pmr::polymorphic_allocator; }; template