Skip to content

Commit

Permalink
removal of Pmr from InterfacePtr Deleter #verification #docs #sonar
Browse files Browse the repository at this point in the history
  • Loading branch information
@serges147
serges147 committed May 31, 2024
1 parent 9204f62 commit e468494
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 47 deletions.
8 changes: 5 additions & 3 deletions cetlvast/suites/docs/examples/example_07_polymorphic_alloc_deleter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class example_07_polymorphic_alloc_deleter : public testing::Test
{
protected:
template <typename Interface>
using InterfacePtr = cetl::pmr::InterfacePtr<Interface, cetl::pmr::memory_resource>;
using InterfacePtr = cetl::pmr::InterfacePtr<Interface>;

void SetUp() override
{
Expand Down Expand Up @@ -253,8 +253,10 @@ TEST_F(example_07_polymorphic_alloc_deleter, example_usage_2)
std::cout << "Obj2 desc : " << obj2->describe() << std::endl;
std::cout << "Obj2 name_a : " << obj2->name() << std::endl;

auto obj2_named = InterfacePtr<INamed>{std::move(obj2)};
std::cout << "Obj2 name_b : " << obj2_named->name() << std::endl;
// Such interface ptr upcasting currently is not supported.
//
// auto obj2_named = InterfacePtr<INamed>{std::move(obj2)};
// std::cout << "Obj2 name_b : " << obj2_named->name() << std::endl;
}

auto obj3 = cetl::pmr::InterfaceFactory::make_unique<INamed>(alloc, "obj3", 4U);
Expand Down
65 changes: 57 additions & 8 deletions cetlvast/suites/unittest/pmr/test_pmr_interface_ptr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
/// SPDX-License-Identifier: MIT

#include <cetl/pmr/interface_ptr.hpp>
#include <cetlvast/memory_resource_mock.hpp>
#include <cetlvast/tracking_memory_resource.hpp>

#include <gmock/gmock.h>
Expand All @@ -16,9 +17,20 @@
namespace
{

using testing::_;
using testing::IsNull;
using testing::Return;
using testing::IsEmpty;
using testing::NotNull;
using testing::StrictMock;

#if defined(__cpp_exceptions)

// Workaround for GCC bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
// Should be used in the tests where exceptions are expected (see `EXPECT_THROW`).
const auto sink = [](auto&&) {};

#endif

class INamed
{
Expand Down Expand Up @@ -83,9 +95,15 @@ std::uint32_t MyObjectBase::counter_ = 0;
class MyObject final : private MyObjectBase, public IIdentifiable, public IDescribable
{
public:
MyObject(std::string name)
MyObject(std::string name, bool throw_on_ctor = false)
: name_{std::move(name)}
{
if (throw_on_ctor)
{
#if defined(__cpp_exceptions)
throw std::runtime_error("ctor");
#endif
}
}

~MyObject() = default;
Expand Down Expand Up @@ -124,9 +142,6 @@ class TestPmrInterfacePtr : public testing::Test
protected:
using pmr = cetl::pmr::memory_resource;

template <typename Interface>
using InterfacePtr = cetl::pmr::InterfacePtr<Interface, pmr>;

void SetUp() override
{
MyObjectBase::counter_ = 0;
Expand Down Expand Up @@ -177,13 +192,47 @@ TEST_F(TestPmrInterfacePtr, up_cast_interface)
EXPECT_THAT(obj0->name(), "obj0");
EXPECT_THAT(obj0->describe(), "obj0 is a MyObject instance.");

cetl::pmr::InterfacePtr<INamed, cetl::pmr::memory_resource> obj0_named{std::move(obj0)};
const INamed& obj0_named = *obj0;

EXPECT_THAT(obj0, NotNull());
EXPECT_THAT(obj0_named.name(), "obj0");

obj0.reset();
}

TEST_F(TestPmrInterfacePtr, make_unique_out_of_memory)
{
StrictMock<cetlvast::MemoryResourceMock> mr_mock{};

cetl::pmr::polymorphic_allocator<MyObject> alloc{&mr_mock};

EXPECT_CALL(mr_mock, do_allocate(sizeof(MyObject), _)).WillOnce(Return(nullptr));

auto obj0 = cetl::pmr::InterfaceFactory::make_unique<IDescribable>(alloc, "obj0");
EXPECT_THAT(obj0, IsNull());
EXPECT_THAT(obj0_named, NotNull());
EXPECT_THAT(obj0_named->name(), "obj0");
}

obj0_named.reset();
TEST_F(TestPmrInterfacePtr, make_unique_myobj_ctor_throws)
{
StrictMock<cetlvast::MemoryResourceMock> mr_mock{};

cetl::pmr::polymorphic_allocator<MyObject> alloc{&mr_mock};

EXPECT_CALL(mr_mock, do_allocate(sizeof(MyObject), _))
.WillOnce(
[this](std::size_t size_bytes, std::size_t alignment) { return mr_.allocate(size_bytes, alignment); });
EXPECT_CALL(mr_mock, do_deallocate(_, sizeof(MyObject), _))
.WillOnce([this](void* p, std::size_t size_bytes, std::size_t alignment) {
mr_.deallocate(p, size_bytes, alignment);
});

#if defined(__cpp_exceptions)
EXPECT_THROW(sink(cetl::pmr::InterfaceFactory::make_unique<INamed>(alloc, "obj0", true)), std::runtime_error);
#else
auto obj0 = cetl::pmr::InterfaceFactory::make_unique<INamed>(alloc, "obj0", true);
EXPECT_THAT(obj0, NotNull());
EXPECT_THAT(obj0->name(), "obj0");
#endif
}

} // namespace
1 change: 0 additions & 1 deletion cetlvast/suites/unittest/test_unbounded_variant.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ using testing::Return;
using testing::IsNull;
using testing::IsEmpty;
using testing::NotNull;
using testing::InSequence;
using testing::StrictMock;

using namespace std::string_literals;
Expand Down
139 changes: 104 additions & 35 deletions include/cetl/pmr/interface_ptr.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,51 +16,57 @@ namespace cetl
namespace pmr
{

template <typename Interface, typename Pmr>
template <typename Interface>
class PmrInterfaceDeleter final
{
public:
template <typename PmrAllocator>
PmrInterfaceDeleter(PmrAllocator alloc, std::size_t obj_count)
: deleter_{alloc.resource(), [alloc, obj_count](Interface* ptr) mutable {
using Concrete = typename PmrAllocator::value_type;
auto* concrete_ptr = static_cast<Concrete*>(ptr);
: deleter_{[alloc, obj_count](Interface* ptr) mutable {
using Concrete = typename PmrAllocator::value_type;

concrete_ptr->~Concrete();
alloc.deallocate(concrete_ptr, obj_count);
}}
auto* concrete_ptr = static_cast<Concrete*>(ptr);
concrete_ptr->~Concrete();
alloc.deallocate(concrete_ptr, obj_count);
}}
{
}

template <typename Down, typename = std::enable_if_t<std::is_base_of<Interface, Down>::value>>
PmrInterfaceDeleter(const PmrInterfaceDeleter<Down, Pmr>& other)
: deleter_{other.get_memory_resource(), [other](Interface* ptr) {
// Delegate to the down class deleter.
other.deleter_(static_cast<Down*>(ptr));
}}
{
}

Pmr* get_memory_resource() const noexcept
{
return deleter_.get_memory_resource();
}

void operator()(Interface* ptr) noexcept
{
deleter_(ptr);
}

private:
template <typename Down, typename PmrT>
friend class PmrInterfaceDeleter;
// Below convertor constructor is only possible with enabled PMR at `function`.
// For now, we decided to comment it out, so that `InterfacePtr` always stays
// within 24-bytes small object optimization, namely without extra memory allocation
// just for the sake of "advanced" deleter (actually chain of casters down to original `Concrete` pointer).
//
// template <typename Down, typename = std::enable_if_t<std::is_base_of<Interface, Down>::value>>
// PmrInterfaceDeleter(const PmrInterfaceDeleter<Down, Pmr>& other)
// : deleter_{other.get_memory_resource(), [other](Interface* ptr) {
// // Delegate to the down class deleter.
// other.deleter_(static_cast<Down*>(ptr));
// }}
// {
// }
//
// Pmr* get_memory_resource() const noexcept
// {
// return deleter_.get_memory_resource();
// }
//
// private:
// template <typename Down, typename PmrT>
// friend class PmrInterfaceDeleter;

cetl::pmr::function<void(Interface*), 24, Pmr> deleter_;
private:
cetl::pmr::function<void(Interface*), 24> deleter_;

}; // PmrInterfaceDeleter

template <typename Interface, typename Pmr>
using InterfacePtr = std::unique_ptr<Interface, PmrInterfaceDeleter<Interface, Pmr>>;
template <typename Interface>
using InterfacePtr = std::unique_ptr<Interface, PmrInterfaceDeleter<Interface>>;

class InterfaceFactory final
{
Expand All @@ -69,20 +75,83 @@ class InterfaceFactory final
InterfaceFactory() = delete;

template <typename Interface, typename PmrAllocator, typename... Args>
static auto make_unique(PmrAllocator alloc, Args&&... args)
-> InterfacePtr<Interface, std::remove_pointer_t<decltype(alloc.resource())>>
CETL_NODISCARD static InterfacePtr<Interface> make_unique(PmrAllocator alloc, Args&&... args)
{
// Allocate memory for the concrete object.
// Then try to construct it in-place - it could potentially throw,
// so RAII will deallocate the memory BUT won't try to destroy the uninitialized object!
//
ConcreteRaii<PmrAllocator> concrete_raii{alloc};
if (auto* const concrete_ptr = concrete_raii.get())
{
concrete_raii.construct(std::forward<Args>(args)...);
}

// Everything is good, so now we can move ownership of the concrete object to the interface pointer.
//
return InterfacePtr<Interface>{concrete_raii.release(), PmrInterfaceDeleter<Interface>{alloc, 1}};
}

private:
template <typename PmrAllocator>
class ConcreteRaii final
{
using Concrete = typename PmrAllocator::value_type;
using Pmr = std::remove_pointer_t<decltype(alloc.resource())>;

InterfacePtr<Concrete, Pmr> concrete_ptr{alloc.allocate(1), PmrInterfaceDeleter<Concrete, Pmr>{alloc, 1}};
if (nullptr != concrete_ptr)
public:
ConcreteRaii(PmrAllocator& pmr_allocator)
: concrete_{pmr_allocator.allocate(1)}
, constructed_{false}
, pmr_allocator_{pmr_allocator}
{
}
~ConcreteRaii()
{
alloc.construct(concrete_ptr.get(), std::forward<Args>(args)...);
if (nullptr != concrete_)
{
if (constructed_)
{
concrete_->~Concrete();
}
pmr_allocator_.deallocate(concrete_, 1);
}
}
ConcreteRaii(const ConcreteRaii&) = delete;
ConcreteRaii(ConcreteRaii&&) noexcept = delete;
ConcreteRaii& operator=(const ConcreteRaii&) = delete;
ConcreteRaii& operator=(ConcreteRaii&&) noexcept = delete;

return concrete_ptr;
}
template <typename... Args>
void construct(Args&&... args)
{
CETL_DEBUG_ASSERT(constructed_ == false, "");

if (nullptr != concrete_)
{
pmr_allocator_.construct(concrete_, std::forward<Args>(args)...);
constructed_ = true;
}
}

Concrete* get() const noexcept
{
return concrete_;
}

Concrete* release()
{
constructed_ = false;
Concrete* result{nullptr};
std::swap(result, concrete_);
return result;
}

private:
Concrete* concrete_;
bool constructed_;
PmrAllocator& pmr_allocator_;

}; // ConcreteRaii

}; // InterfaceFactory

Expand Down

0 comments on commit e468494

Please sign in to comment.