Skip to content

Commit

Permalink
Better protection against inherited name collisions
Browse files Browse the repository at this point in the history
in dynarray::_memOwner and dynarray::_scopedPtr.
Also passing Alloc by value for constructors
  • Loading branch information
OleErikPeistorpet committed Sep 29, 2023
1 parent c91858b commit 3d9670b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 47 deletions.
85 changes: 42 additions & 43 deletions dynarray.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,19 @@ class dynarray
using const_reverse_iterator = std::reverse_iterator<const_iterator>;


constexpr dynarray() noexcept(noexcept(Alloc{})) : _m(Alloc{}) {}
constexpr explicit dynarray(const Alloc & a) noexcept : _m(a) {}
constexpr dynarray() noexcept(noexcept(Alloc{})) : dynarray(Alloc{}) {}
constexpr explicit dynarray(Alloc a) noexcept : _m(a) {}

//! Construct empty dynarray with space reserved for exactly capacity elements
dynarray(reserve_tag, size_type capacity, const Alloc & a = Alloc{}) : _m(a) { _initReserve(capacity); }
dynarray(reserve_tag, size_type capacity, Alloc a = Alloc{}) : _m(a) { _initReserve(capacity); }

/** @brief Default-initializes elements, can be significantly faster if T is scalar or has trivial default constructor
*
* @copydetails resize_for_overwrite(size_type) */
dynarray(size_type size, for_overwrite_t, const Alloc & a = Alloc{});
dynarray(size_type size, for_overwrite_t, Alloc a = Alloc{});
//! (Value-initializes elements, same as std::vector)
explicit dynarray(size_type size, const Alloc & a = Alloc{});
dynarray(size_type size, const T & val, const Alloc & a = Alloc{}) : _m(a) { append(size, val); }
explicit dynarray(size_type size, Alloc a = Alloc{});
dynarray(size_type size, const T & val, Alloc a = Alloc{}) : _m(a) { append(size, val); }

/** @brief Equivalent to `std::vector(begin(r), end(r), a)`, where `end(r)` is not needed if `r.size()` exists
*
Expand All @@ -117,15 +117,15 @@ class dynarray
@endcode */
template< typename InputRange,
typename /*EnableIfRange*/ = iterator_t<InputRange> >
explicit dynarray(InputRange && r, const Alloc & a = Alloc{}) : _m(a) { append(r); }
explicit dynarray(InputRange && r, Alloc a = Alloc{}) : _m(a) { append(r); }

dynarray(std::initializer_list<T> il, const Alloc & a = Alloc{}) : _m(a) { append(il); }
dynarray(std::initializer_list<T> il, Alloc a = Alloc{}) : _m(a) { append(il); }

dynarray(dynarray && other) noexcept : _m(std::move(other._m)) {}
dynarray(dynarray && other, const Alloc & a);
dynarray(const dynarray & other) : dynarray(other,
_alloTrait::select_on_container_copy_construction(other._m)) {}
dynarray(const dynarray & other, const Alloc & a) : _m(a) { append(other); }
dynarray(dynarray && other) noexcept : _m(std::move(other._m)) {}
dynarray(dynarray && other, Alloc a);
dynarray(const dynarray & other) : dynarray(other,
_alloTrait::select_on_container_copy_construction(other._m)) {}
dynarray(const dynarray & other, Alloc a) : _m(a) { append(other); }

~dynarray() noexcept;

Expand Down Expand Up @@ -294,56 +294,53 @@ class dynarray

private:
using _allocateWrap = _detail::DebugAllocateWrapper<Alloc, pointer>;
using _internBase = _detail::DynarrBase<pointer>;
using _internBase = _detail::DynarrBase<pointer>;
using _debugSizeUpdater = _detail::DebugSizeInHeaderUpdater<_internBase>;
using _alloc_7KQWe = Alloc; // guarding against name collision due to inheritance (MSVC)

template< typename > // template to allow constexpr constructor when Alloc copy constructor is not constexpr
struct _memOwner : public _internBase, public allocator_type
struct _memOwner : public _internBase, public _alloc_7KQWe
{
using _internBase::data; // Owning pointer to beginning of data buffer
using _internBase::end; // Pointer to one past the back object
using _internBase::reservEnd; // Pointer to end of allocated memory
using ::oel::_detail::DynarrBase<pointer>::data; // owner
using ::oel::_detail::DynarrBase<pointer>::end;
using ::oel::_detail::DynarrBase<pointer>::reservEnd;

constexpr _memOwner(const allocator_type & a)
: _internBase(), allocator_type(a) {
constexpr _memOwner(_alloc_7KQWe & a) noexcept
: ::oel::_detail::DynarrBase<pointer>{}, _alloc_7KQWe{std::move(a)} {
}

_memOwner(_memOwner && other) noexcept
: _internBase(other), allocator_type(std::move(other))
: ::oel::_detail::DynarrBase<pointer>{other}, _alloc_7KQWe{std::move(other)}
{
other.reservEnd = other.end = other.data = nullptr;
}

~_memOwner()
{
if (data)
_allocateWrap::dealloc(*this, data, reservEnd - data);
::oel::_detail::DebugAllocateWrapper<_alloc_7KQWe, pointer>::dealloc(*this, data, reservEnd - data);
}
};
_memOwner<Alloc> _m; // the only non-static data member
}
_m; // the only non-static data member

// Should be very careful with potential name collisions because of inheriting from allocator
struct _scopedPtr : private allocator_type
struct _scopedPtr : private _alloc_7KQWe
{
pointer data; // owner
pointer bufEnd;

_scopedPtr(const allocator_type & a, size_type const capPrechecked)
: allocator_type(a),
data{_allocateWrap::allocate(*this, capPrechecked)},
bufEnd{data + capPrechecked} {
_scopedPtr(const _alloc_7KQWe & a, pointer buf, size_type capacity)
: _alloc_7KQWe{a}, data{buf}, bufEnd{buf + capacity} {
}

_scopedPtr(_scopedPtr &&) = delete;

~_scopedPtr()
{
if (data)
_allocateWrap::dealloc(*this, data, bufEnd - data);
::oel::_detail::DebugAllocateWrapper<_alloc_7KQWe, pointer>::dealloc(*this, data, bufEnd - data);
}
};

using _uninitFill = _detail::UninitFill<decltype(_m)>;
using _uninitFill = _detail::UninitFill<_memOwner>;

void _resetData(T *const newData)
{
Expand Down Expand Up @@ -576,7 +573,7 @@ class dynarray
InputIter _append(InputIter src, size_type const n)
{
return _appendImpl(
[src_ = std::move(src)](T * dest, size_type n_, decltype(_m) & alloc) mutable
[src_ = std::move(src)](T * dest, size_type n_, _memOwner & alloc) mutable
{
return _detail::UninitCopy(std::move(src_), dest, dest + n_, alloc);
},
Expand All @@ -600,7 +597,8 @@ class dynarray
template< typename InsertHelper, typename... Args >
T * _insertRealloc(T *const pos, Args... args)
{
_scopedPtr newBuf{_m, InsertHelper::calcCap(*this, args...)};
auto const newCap = InsertHelper::calcCap(*this, args...);
_scopedPtr newBuf{_m, _allocateWrap::allocate(_m, newCap), newCap};

size_type const nBefore = pos - data();
T *const newPos = newBuf.data + nBefore;
Expand All @@ -624,7 +622,7 @@ class dynarray
}

template< typename... Args >
static T * construct(decltype(_m) & alloc, T *const newPos, Args... args)
static T * construct(_memOwner & alloc, T *const newPos, Args... args)
{
_alloTrait::construct(alloc, newPos, static_cast<Args &&>(args)...);
return newPos + 1;
Expand All @@ -640,7 +638,7 @@ class dynarray
}

template< typename InputIter, typename >
static T * construct(decltype(_m) & alloc, T *const newPos, InputIter first, size_type count)
static T * construct(_memOwner & alloc, T *const newPos, InputIter first, size_type count)
{
T *const dLast = newPos + count;
_detail::UninitCopy(std::move(first), newPos, dLast, alloc);
Expand Down Expand Up @@ -757,10 +755,11 @@ inline T & dynarray<T, Alloc>::emplace_back(Args &&... args) &


template< typename T, typename Alloc >
dynarray<T, Alloc>::dynarray(dynarray && other, const Alloc & a)
: _m(a)
dynarray<T, Alloc>::dynarray(dynarray && other, Alloc a)
: _m(a) // moves from a
{
OEL_CONST_COND if (!_alloTrait::is_always_equal::value and a != other._m)
Alloc & myA = _m;
OEL_CONST_COND if (!_alloTrait::is_always_equal::value and myA != other._m)
append(other | view::move);
else
_moveInternBase(other._m);
Expand Down Expand Up @@ -790,7 +789,7 @@ dynarray<T, Alloc> & dynarray<T, Alloc>::operator =(dynarray && other) &
}

template< typename T, typename Alloc >
dynarray<T, Alloc>::dynarray(size_type n, for_overwrite_t, const Alloc & a)
dynarray<T, Alloc>::dynarray(size_type n, for_overwrite_t, Alloc a)
: _m(a)
{
_debugSizeUpdater guard{_m};
Expand All @@ -801,7 +800,7 @@ dynarray<T, Alloc>::dynarray(size_type n, for_overwrite_t, const Alloc & a)
}

template< typename T, typename Alloc >
dynarray<T, Alloc>::dynarray(size_type n, const Alloc & a)
dynarray<T, Alloc>::dynarray(size_type n, Alloc a)
: _m(a)
{
_debugSizeUpdater guard{_m};
Expand Down Expand Up @@ -851,7 +850,7 @@ inline void dynarray<T, Alloc>::append(size_type n, const T & val)
{
_detail::ForwardT<const T &> val_;

auto operator()(T * dest, size_type n_, decltype(_m) & alloc) const
auto operator()(T * dest, size_type n_, _memOwner & alloc) const
{
_uninitFill::call(dest, dest + n_, alloc, val_);
return nullptr; // returned by _appendImpl
Expand Down
10 changes: 9 additions & 1 deletion unit_test/dynarray_construct_assignop_swap_gtest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,17 @@ struct NonConstexprAlloc : oel::allocator<int>
};
}

#if __cpp_constinit
void testConstInitCompile()
{
constinit static dynarray<int> d;
}
#endif

void testNonConstexprCompile()
{
dynarray<int, NonConstexprAlloc> d;
static dynarray<int, NonConstexprAlloc> d;
[[maybe_unused]] auto d2 = dynarray<int, NonConstexprAlloc>(NonConstexprAlloc{});
}

TEST_F(dynarrayConstructTest, emptyBracesArg)
Expand Down
13 changes: 10 additions & 3 deletions unit_test/test_classes.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,22 @@ struct TrackingAllocator : TrackingAllocatorBase<T>
new(raw) T(std::forward<Args>(args)...);;
}

// Testing collision with internal names in dynarray
using allocator_type = TrackingAllocatorBase<T>;
using Alloc = void;
struct oel {};
struct _detail {};
struct _internBase {};
struct _allocateWrap
{
void dealloc(TrackingAllocator, void *, std::size_t);
};
};

template<typename T, bool PropagateOnMoveAssign = false, bool UseConstruct = true>
struct StatefulAllocator : std::conditional_t< UseConstruct, TrackingAllocator<T>, TrackingAllocatorBase<T> >
{
using propagate_on_container_move_assignment = oel::bool_constant<PropagateOnMoveAssign>;
using propagate_on_container_move_assignment = std::bool_constant<PropagateOnMoveAssign>;

int id;

Expand All @@ -272,8 +281,6 @@ struct StatefulAllocator : std::conditional_t< UseConstruct, TrackingAllocator<T
friend bool operator==(StatefulAllocator a, StatefulAllocator b) { return a.id == b.id; }

friend bool operator!=(StatefulAllocator a, StatefulAllocator b) { return !(a == b); }

using allocator_type = StatefulAllocator;
};


Expand Down

0 comments on commit 3d9670b

Please sign in to comment.