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 `<memory_resource>`; at Eric's request, I've split
this out into its own patch applied to the existing
`<experimental/memory_resource>` instead.)

Reviewed as https://reviews.llvm.org/D47109

llvm-svn: 333384
This commit is contained in:
Eric Fiselier 2018-05-29 00:08:47 +00:00
parent dcfcfdb0d1
commit bd2e949869
9 changed files with 318 additions and 35 deletions

View File

@ -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<sizeof...(_Args1)>::type{}
)
, __transform_tuple(
typename __lfts_uses_alloc_ctor<
_T2, memory_resource*, _Args2...
_T2, polymorphic_allocator&, _Args2...
>::type()
, _VSTD::move(__y)
, typename __make_tuple_indices<sizeof...(_Args2)>::type{}
@ -289,23 +289,23 @@ private:
template <class ..._Args, size_t ..._Idx>
_LIBCPP_INLINE_VISIBILITY
tuple<allocator_arg_t const&, memory_resource*, _Args&&...>
tuple<allocator_arg_t const&, polymorphic_allocator&, _Args&&...>
__transform_tuple(integral_constant<int, 1>, tuple<_Args...> && __t,
__tuple_indices<_Idx...>) const
__tuple_indices<_Idx...>)
{
using _Tup = tuple<allocator_arg_t const&, memory_resource*, _Args&&...>;
return _Tup(allocator_arg, resource(),
using _Tup = tuple<allocator_arg_t const&, polymorphic_allocator&, _Args&&...>;
return _Tup(allocator_arg, *this,
_VSTD::get<_Idx>(_VSTD::move(__t))...);
}
template <class ..._Args, size_t ..._Idx>
_LIBCPP_INLINE_VISIBILITY
tuple<_Args&&..., memory_resource*>
tuple<_Args&&..., polymorphic_allocator&>
__transform_tuple(integral_constant<int, 2>, 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

View File

@ -79,12 +79,12 @@ struct CountCopies {
};
struct CountCopiesAllocV1 {
typedef ex::memory_resource* allocator_type;
allocator_type alloc;
typedef ex::polymorphic_allocator<char> 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<char> 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) {}

View File

@ -90,6 +90,32 @@ void test_pmr_uses_allocator(std::pair<TT, UU> const& p)
assert((doTest<T, U>(UA_AllocArg, UA_None, p)));
}
}
template <class Alloc, class TT, class UU>
void test_pmr_not_uses_allocator(std::pair<TT, UU> const& p)
{
{
using T = NotUsesAllocator<Alloc, 1>;
using U = NotUsesAllocator<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None, p)));
}
{
using T = UsesAllocatorV1<Alloc, 1>;
using U = UsesAllocatorV2<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None, p)));
}
{
using T = UsesAllocatorV2<Alloc, 1>;
using U = UsesAllocatorV3<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None, p)));
}
{
using T = UsesAllocatorV3<Alloc, 1>;
using U = NotUsesAllocator<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None, p)));
}
}
template <class Tp>
struct Print;
@ -103,7 +129,7 @@ int main()
int y = 42;
const std::pair<int, int&> p(x, y);
test_pmr_uses_allocator<ERT>(p);
test_pmr_uses_allocator<PMR>(p);
test_pmr_not_uses_allocator<PMR>(p);
test_pmr_uses_allocator<PMA>(p);
}
{
@ -111,7 +137,7 @@ int main()
int y = 42;
const std::pair<int&, int&&> p(x, std::move(y));
test_pmr_uses_allocator<ERT>(p);
test_pmr_uses_allocator<PMR>(p);
test_pmr_not_uses_allocator<PMR>(p);
test_pmr_uses_allocator<PMA>(p);
}
}

View File

@ -90,6 +90,31 @@ void test_pmr_uses_allocator(std::pair<TT, UU>&& p)
}
}
template <class Alloc, class TT, class UU>
void test_pmr_not_uses_allocator(std::pair<TT, UU>&& p)
{
{
using T = NotUsesAllocator<Alloc, 1>;
using U = NotUsesAllocator<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None, std::move(p))));
}
{
using T = UsesAllocatorV1<Alloc, 1>;
using U = UsesAllocatorV2<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None, std::move(p))));
}
{
using T = UsesAllocatorV2<Alloc, 1>;
using U = UsesAllocatorV3<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None, std::move(p))));
}
{
using T = UsesAllocatorV3<Alloc, 1>;
using U = NotUsesAllocator<Alloc, 1>;
assert((doTest<T, U>(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<int&, int&&> p(x, std::move(y));
test_pmr_uses_allocator<ERT>(std::move(p));
test_pmr_uses_allocator<PMR>(std::move(p));
test_pmr_not_uses_allocator<PMR>(std::move(p));
test_pmr_uses_allocator<PMA>(std::move(p));
}
{
@ -108,7 +133,7 @@ int main()
int y = 42;
std::pair<int&&, int&> p(std::move(x), y);
test_pmr_uses_allocator<ERT>(std::move(p));
test_pmr_uses_allocator<PMR>(std::move(p));
test_pmr_not_uses_allocator<PMR>(std::move(p));
test_pmr_uses_allocator<PMA>(std::move(p));
}
}

View File

@ -93,6 +93,35 @@ void test_pmr_uses_allocator(TT&& t, UU&& u)
}
}
template <class Alloc, class TT, class UU>
void test_pmr_not_uses_allocator(TT&& t, UU&& u)
{
{
using T = NotUsesAllocator<Alloc, 1>;
using U = NotUsesAllocator<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None,
std::forward<TT>(t), std::forward<UU>(u))));
}
{
using T = UsesAllocatorV1<Alloc, 1>;
using U = UsesAllocatorV2<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None,
std::forward<TT>(t), std::forward<UU>(u))));
}
{
using T = UsesAllocatorV2<Alloc, 1>;
using U = UsesAllocatorV3<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None,
std::forward<TT>(t), std::forward<UU>(u))));
}
{
using T = UsesAllocatorV3<Alloc, 1>;
using U = NotUsesAllocator<Alloc, 1>;
assert((doTest<T, U>(UA_None, UA_None,
std::forward<TT>(t), std::forward<UU>(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<ERT>(x, std::move(y));
test_pmr_uses_allocator<PMR>(x, std::move(y));
test_pmr_not_uses_allocator<PMR>(x, std::move(y));
test_pmr_uses_allocator<PMA>(x, std::move(y));
}
{
int x = 42;
const int y = 42;
test_pmr_uses_allocator<ERT>(std::move(x), y);
test_pmr_uses_allocator<PMR>(std::move(x), y);
test_pmr_not_uses_allocator<PMR>(std::move(x), y);
test_pmr_uses_allocator<PMA>(std::move(x), y);
}
}

View File

@ -83,6 +83,35 @@ void test_pmr_uses_allocator(std::tuple<TTypes...> ttuple, std::tuple<UTypes...>
}
}
template <class Alloc, class ...TTypes, class ...UTypes>
void test_pmr_not_uses_allocator(std::tuple<TTypes...> ttuple, std::tuple<UTypes...> utuple)
{
{
using T = NotUsesAllocator<Alloc, sizeof...(TTypes)>;
using U = NotUsesAllocator<Alloc, sizeof...(UTypes)>;
assert((doTest<T, U>(UA_None, UA_None,
std::move(ttuple), std::move(utuple))));
}
{
using T = UsesAllocatorV1<Alloc, sizeof...(TTypes)>;
using U = UsesAllocatorV2<Alloc, sizeof...(UTypes)>;
assert((doTest<T, U>(UA_None, UA_None,
std::move(ttuple), std::move(utuple))));
}
{
using T = UsesAllocatorV2<Alloc, sizeof...(TTypes)>;
using U = UsesAllocatorV3<Alloc, sizeof...(UTypes)>;
assert((doTest<T, U>(UA_None, UA_None,
std::move(ttuple), std::move(utuple))));
}
{
using T = UsesAllocatorV3<Alloc, sizeof...(TTypes)>;
using U = NotUsesAllocator<Alloc, sizeof...(UTypes)>;
assert((doTest<T, U>(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<ERT>(t1, t1);
test_pmr_uses_allocator<PMR>(t1, t1);
test_pmr_not_uses_allocator<PMR>(t1, t1);
test_pmr_uses_allocator<PMA>(t1, t1);
}
{
@ -99,8 +128,8 @@ int main()
std::tuple<> t2;
test_pmr_uses_allocator<ERT>(t1, t2);
test_pmr_uses_allocator<ERT>(t2, t1);
test_pmr_uses_allocator<PMR>(t1, t2);
test_pmr_uses_allocator<PMR>(t2, t1);
test_pmr_not_uses_allocator<PMR>(t1, t2);
test_pmr_not_uses_allocator<PMR>(t2, t1);
test_pmr_uses_allocator<PMA>(t1, t2);
test_pmr_uses_allocator<PMA>(t2, t1);
}
@ -111,8 +140,8 @@ int main()
std::tuple<int&, double&&> t2(x, std::move(dx));
test_pmr_uses_allocator<ERT>( t1, std::move(t2));
test_pmr_uses_allocator<ERT>(std::move(t2), t1);
test_pmr_uses_allocator<PMR>( t1, std::move(t2));
test_pmr_uses_allocator<PMR>(std::move(t2), t1);
test_pmr_not_uses_allocator<PMR>( t1, std::move(t2));
test_pmr_not_uses_allocator<PMR>(std::move(t2), t1);
test_pmr_uses_allocator<PMA>( t1, std::move(t2));
test_pmr_uses_allocator<PMA>(std::move(t2), t1);
}
@ -126,8 +155,8 @@ int main()
std::tuple<int&, double&&, const char*> t2(x, std::move(dx), s);
test_pmr_uses_allocator<ERT>( t1, std::move(t2));
test_pmr_uses_allocator<ERT>(std::move(t2), t1);
test_pmr_uses_allocator<PMR>( t1, std::move(t2));
test_pmr_uses_allocator<PMR>(std::move(t2), t1);
test_pmr_not_uses_allocator<PMR>( t1, std::move(t2));
test_pmr_not_uses_allocator<PMR>(std::move(t2), t1);
test_pmr_uses_allocator<PMA>( t1, std::move(t2));
test_pmr_uses_allocator<PMA>(std::move(t2), t1);
}

View File

@ -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
// <memory_resource>
// template <class T> class polymorphic_allocator
// template <class U1, class U2, class ...Args1, class ...Args2>
// void polymorphic_allocator<T>::construct(pair<U1, U2>*, piecewise_construct_t
// tuple<Args1...>, tuple<Args2...>)
#include <experimental/memory_resource>
#include <tuple>
#include <type_traits>
#include <utility>
#include <cassert>
#include <cstdlib>
#include "test_macros.h"
namespace ex = std::experimental::pmr;
template <class T>
struct EvilAlloc {
explicit EvilAlloc() : inner_(ex::null_memory_resource()) {}
EvilAlloc(ex::polymorphic_allocator<T> & a) : inner_(a) {}
EvilAlloc(ex::polymorphic_allocator<T> && a) : inner_(a) {}
EvilAlloc(ex::polymorphic_allocator<T> const & a) = delete;
EvilAlloc(ex::polymorphic_allocator<T> const && a) = delete;
using value_type = T;
template <class U> EvilAlloc(EvilAlloc<U> const & rhs) : inner_(rhs.inner_) {}
ex::polymorphic_allocator<T> inner_;
};
struct WidgetV0 {
WidgetV0(int v) : value_(v) {}
bool holds(int v, const ex::polymorphic_allocator<char>&) const {
return value_ == v;
}
private:
int value_;
};
struct WidgetV1 {
using allocator_type = EvilAlloc<char>;
WidgetV1(int v) : value_(v), alloc_() {}
WidgetV1(std::allocator_arg_t, EvilAlloc<char> a, int v) : value_(v), alloc_(a) {}
bool holds(int v, const ex::polymorphic_allocator<char>& a) const {
return value_ == v && alloc_.inner_ == a;
}
private:
int value_;
EvilAlloc<char> alloc_;
};
struct WidgetV2 {
using allocator_type = EvilAlloc<char>;
WidgetV2(int v) : value_(v), alloc_() {}
WidgetV2(int v, EvilAlloc<char> a) : value_(v), alloc_(a) {}
bool holds(int v, ex::polymorphic_allocator<char> a) const {
return value_ == v && alloc_.inner_ == a;
}
private:
int value_;
EvilAlloc<char> alloc_;
};
struct WidgetV3 {
using allocator_type = EvilAlloc<char>;
WidgetV3(int v) : value_(v), alloc_() {}
WidgetV3(std::allocator_arg_t, EvilAlloc<char> a, int v) : value_(v), alloc_(a) {}
WidgetV3(int v, EvilAlloc<char> a) : value_(v), alloc_(a) {}
bool holds(int v, ex::polymorphic_allocator<char> a) const {
return value_ == v && alloc_.inner_ == a;
}
private:
int value_;
EvilAlloc<char> alloc_;
};
static_assert(std::uses_allocator<WidgetV1, EvilAlloc<char>>::value, "");
static_assert(std::uses_allocator<WidgetV2, EvilAlloc<char>>::value, "");
static_assert(std::uses_allocator<WidgetV3, EvilAlloc<char>>::value, "");
static_assert(std::uses_allocator<WidgetV1, ex::polymorphic_allocator<char>>::value, "");
static_assert(std::uses_allocator<WidgetV2, ex::polymorphic_allocator<char>>::value, "");
static_assert(std::uses_allocator<WidgetV3, ex::polymorphic_allocator<char>>::value, "");
template<class W1, class W2>
void test_evil()
{
using PMA = ex::polymorphic_allocator<char>;
PMA pma(ex::new_delete_resource());
{
using Pair = std::pair<W1, W2>;
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<WidgetV0, WidgetV0>();
test_evil<WidgetV0, WidgetV1>();
test_evil<WidgetV0, WidgetV2>();
test_evil<WidgetV0, WidgetV3>();
test_evil<WidgetV1, WidgetV0>();
test_evil<WidgetV1, WidgetV1>();
test_evil<WidgetV1, WidgetV2>();
test_evil<WidgetV1, WidgetV3>();
test_evil<WidgetV2, WidgetV0>();
test_evil<WidgetV2, WidgetV1>();
test_evil<WidgetV2, WidgetV2>();
test_evil<WidgetV2, WidgetV3>();
test_evil<WidgetV3, WidgetV0>();
test_evil<WidgetV3, WidgetV1>();
test_evil<WidgetV3, WidgetV2>();
test_evil<WidgetV3, WidgetV3>();
}

View File

@ -126,6 +126,39 @@ void test_pmr_uses_alloc(Args&&... args)
}
}
template <class Alloc, class ...Args>
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<Alloc, sizeof...(Args)>;
assert(doTestUsesAllocV0<T>(std::forward<Args>(args)...));
assert((doTestUsesAllocV1<T>(M, std::forward<Args>(args)...)));
assert((doTestUsesAllocV2<T>(M, std::forward<Args>(args)...)));
}
{
// Test T(std::allocator_arg_t, Alloc const&, Args...) construction
using T = UsesAllocatorV1<Alloc, sizeof...(Args)>;
assert((doTest<T>(UA_None, std::forward<Args>(args)...)));
}
{
// Test T(Args..., Alloc const&) construction
using T = UsesAllocatorV2<Alloc, sizeof...(Args)>;
assert((doTest<T>(UA_None, std::forward<Args>(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<Alloc, sizeof...(Args)>;
assert((doTest<T>(UA_None, std::forward<Args>(args)...)));
}
}
// Test that polymorphic_allocator does not prevent us from manually
// doing non-pmr uses-allocator construction.
template <class Alloc, class AllocObj, class ...Args>
@ -167,16 +200,16 @@ int main()
const int cvalue = 43;
{
test_pmr_uses_alloc<ET>();
test_pmr_uses_alloc<PMR>();
test_pmr_not_uses_alloc<PMR>();
test_pmr_uses_alloc<PMA>();
test_pmr_uses_alloc<ET>(value);
test_pmr_uses_alloc<PMR>(value);
test_pmr_not_uses_alloc<PMR>(value);
test_pmr_uses_alloc<PMA>(value);
test_pmr_uses_alloc<ET>(cvalue);
test_pmr_uses_alloc<PMR>(cvalue);
test_pmr_not_uses_alloc<PMR>(cvalue);
test_pmr_uses_alloc<PMA>(cvalue);
test_pmr_uses_alloc<ET>(cvalue, std::move(value));
test_pmr_uses_alloc<PMR>(cvalue, std::move(value));
test_pmr_not_uses_alloc<PMR>(cvalue, std::move(value));
test_pmr_uses_alloc<PMA>(cvalue, std::move(value));
}
{

View File

@ -28,7 +28,7 @@
// because it can't include <experimental/memory_resource>
template <>
struct TransformErasedTypeAlloc<std::experimental::erased_type> {
using type = std::experimental::pmr::memory_resource*;
using type = std::experimental::pmr::polymorphic_allocator<int>;
};
template <class ProviderT, int = 0>