From 41fb809557abd29a57151b6e1aaeabd05f9437e1 Mon Sep 17 00:00:00 2001 From: Vaxry <43317083+vaxerski@users.noreply.github.com> Date: Thu, 2 Jul 2026 16:30:34 +0200 Subject: [PATCH] memory: fix virtual inheritance casts (#115) --- include/hyprutils/memory/Atomic.hpp | 12 +- include/hyprutils/memory/ImplBase.hpp | 5 + include/hyprutils/memory/SharedPtr.hpp | 23 ++-- include/hyprutils/memory/UniquePtr.hpp | 2 +- include/hyprutils/memory/WeakPtr.hpp | 31 +++-- tests/memory/Memory.cpp | 159 +++++++++++++++++++++++++ 6 files changed, 206 insertions(+), 26 deletions(-) diff --git a/include/hyprutils/memory/Atomic.hpp b/include/hyprutils/memory/Atomic.hpp index c43475b..e4a5d17 100644 --- a/include/hyprutils/memory/Atomic.hpp +++ b/include/hyprutils/memory/Atomic.hpp @@ -54,7 +54,7 @@ namespace Hyprutils::Memory { using validHierarchy = std::enable_if_t&, X>, CAtomicSharedPointer&>; public: - explicit CAtomicSharedPointer(T* object) noexcept : m_ptr(new Atomic_::impl(sc(object), _delete), sc(object)) { + explicit CAtomicSharedPointer(T* object) noexcept : m_ptr(new Atomic_::impl(Impl_::dataPointer(object), _delete), Impl_::dataPointer(object)) { ; } @@ -426,26 +426,26 @@ namespace Hyprutils::Memory { template CAtomicSharedPointer reinterpretPointerCast(const CAtomicSharedPointer& ref) { - return CAtomicSharedPointer(ref.impl(), ref.m_data); + return CAtomicSharedPointer(ref.impl(), Impl_::dataPointer(ref.get())); } template CAtomicSharedPointer dynamicPointerCast(const CAtomicSharedPointer& ref) { if (!ref) return nullptr; - T* newPtr = dynamic_cast(sc(ref.impl()->getData())); + T* newPtr = dynamic_cast(ref.get()); if (!newPtr) return nullptr; - return CAtomicSharedPointer(ref.impl(), newPtr); + return CAtomicSharedPointer(ref.impl(), Impl_::dataPointer(newPtr)); } template CAtomicWeakPointer dynamicPointerCast(const CAtomicWeakPointer& ref) { if (!ref) return nullptr; - T* newPtr = dynamic_cast(sc(ref.impl()->getData())); + T* newPtr = dynamic_cast(ref.get()); if (!newPtr) return nullptr; - return CAtomicWeakPointer(ref.impl(), newPtr); + return CAtomicWeakPointer(ref.impl(), Impl_::dataPointer(newPtr)); } } diff --git a/include/hyprutils/memory/ImplBase.hpp b/include/hyprutils/memory/ImplBase.hpp index 7f988e4..2deb097 100644 --- a/include/hyprutils/memory/ImplBase.hpp +++ b/include/hyprutils/memory/ImplBase.hpp @@ -6,6 +6,11 @@ namespace Hyprutils { namespace Memory { namespace Impl_ { + template + constexpr void* dataPointer(T* ptr) noexcept { + return const_cast(static_cast(ptr)); + } + class impl_base { public: using DeleteFn = void (*)(void*); diff --git a/include/hyprutils/memory/SharedPtr.hpp b/include/hyprutils/memory/SharedPtr.hpp index 86b2abe..11ae211 100644 --- a/include/hyprutils/memory/SharedPtr.hpp +++ b/include/hyprutils/memory/SharedPtr.hpp @@ -28,13 +28,13 @@ namespace Hyprutils { /* creates a new shared pointer managing a resource avoid calling. Could duplicate ownership. Prefer makeShared */ - explicit CSharedPointer(T* object) noexcept : impl_(new Impl_::impl_base(sc(object), _delete)), m_data(sc(object)) { + explicit CSharedPointer(T* object) noexcept : impl_(new Impl_::impl_base(Impl_::dataPointer(object), _delete)), m_data(Impl_::dataPointer(object)) { increment(); } /* creates a shared pointer from a reference */ template > - CSharedPointer(const CSharedPointer& ref) noexcept : impl_(ref.impl_), m_data(ref.m_data) { + CSharedPointer(const CSharedPointer& ref) noexcept : impl_(ref.impl_), m_data(Impl_::dataPointer(sc(ref.get()))) { increment(); } @@ -44,8 +44,10 @@ namespace Hyprutils { template > CSharedPointer(CSharedPointer&& ref) noexcept { - std::swap(impl_, ref.impl_); - std::swap(m_data, ref.m_data); + impl_ = ref.impl_; + m_data = Impl_::dataPointer(sc(ref.get())); + ref.impl_ = nullptr; + ref.m_data = nullptr; } CSharedPointer(CSharedPointer&& ref) noexcept { @@ -72,12 +74,14 @@ namespace Hyprutils { template validHierarchy&> operator=(const CSharedPointer& rhs) { - if (impl_ == rhs.impl_) + if (impl_ == rhs.impl_) { + m_data = Impl_::dataPointer(sc(rhs.get())); return *this; + } decrement(impl_); impl_ = rhs.impl_; - m_data = rhs.m_data; + m_data = Impl_::dataPointer(sc(rhs.get())); increment(); return *this; } @@ -95,8 +99,11 @@ namespace Hyprutils { template validHierarchy&> operator=(CSharedPointer&& rhs) { + auto* rhsData = rhs.get(); + std::swap(impl_, rhs.impl_); std::swap(m_data, rhs.m_data); + m_data = Impl_::dataPointer(sc(rhsData)); return *this; } @@ -206,10 +213,10 @@ namespace Hyprutils { CSharedPointer dynamicPointerCast(const CSharedPointer& ref) { if (!ref) return nullptr; - T* newPtr = dynamic_cast(sc(ref.impl_->getData())); + T* newPtr = dynamic_cast(ref.get()); if (!newPtr) return nullptr; - return CSharedPointer(ref.impl_, newPtr); + return CSharedPointer(ref.impl_, Impl_::dataPointer(newPtr)); } } } diff --git a/include/hyprutils/memory/UniquePtr.hpp b/include/hyprutils/memory/UniquePtr.hpp index 6db13d0..3e1a975 100644 --- a/include/hyprutils/memory/UniquePtr.hpp +++ b/include/hyprutils/memory/UniquePtr.hpp @@ -22,7 +22,7 @@ namespace Hyprutils { /* creates a new unique pointer managing a resource avoid calling. Could duplicate ownership. Prefer makeUnique */ - explicit CUniquePointer(T* object) noexcept : impl_(new Impl_::impl_base(sc(object), [](void* p) { std::default_delete{}(sc(p)); }, false)) { + explicit CUniquePointer(T* object) noexcept : impl_(new Impl_::impl_base(Impl_::dataPointer(object), [](void* p) { std::default_delete{}(sc(p)); }, false)) { increment(); } diff --git a/include/hyprutils/memory/WeakPtr.hpp b/include/hyprutils/memory/WeakPtr.hpp index 9372669..a670d1f 100644 --- a/include/hyprutils/memory/WeakPtr.hpp +++ b/include/hyprutils/memory/WeakPtr.hpp @@ -27,7 +27,7 @@ namespace Hyprutils { return; impl_ = ref.impl_; - m_data = ref.m_data; + m_data = Impl_::dataPointer(sc(ref.get())); incrementWeak(); } @@ -38,7 +38,7 @@ namespace Hyprutils { return; impl_ = ref.impl_; - m_data = ref.impl_->getData(); + m_data = Impl_::dataPointer(sc(ref.get())); incrementWeak(); } @@ -49,7 +49,7 @@ namespace Hyprutils { return; impl_ = ref.impl_; - m_data = ref.m_data; + m_data = Impl_::dataPointer(sc(ref.get())); incrementWeak(); } @@ -72,8 +72,10 @@ namespace Hyprutils { template > CWeakPointer(CWeakPointer&& ref) noexcept { - std::swap(impl_, ref.impl_); - std::swap(m_data, ref.m_data); + impl_ = ref.impl_; + m_data = Impl_::dataPointer(sc(ref.get())); + ref.impl_ = nullptr; + ref.m_data = nullptr; } CWeakPointer(CWeakPointer&& ref) noexcept { @@ -84,12 +86,14 @@ namespace Hyprutils { /* create a weak ptr from another weak ptr with assignment */ template validHierarchy&> operator=(const CWeakPointer& rhs) { - if (impl_ == rhs.impl_) + if (impl_ == rhs.impl_) { + m_data = Impl_::dataPointer(sc(rhs.get())); return *this; + } decrementWeak(); impl_ = rhs.impl_; - m_data = rhs.m_data; + m_data = Impl_::dataPointer(sc(rhs.get())); incrementWeak(); return *this; } @@ -107,8 +111,11 @@ namespace Hyprutils { template validHierarchy&> operator=(CWeakPointer&& rhs) noexcept { + auto* rhsData = rhs.get(); + std::swap(impl_, rhs.impl_); std::swap(m_data, rhs.m_data); + m_data = Impl_::dataPointer(sc(rhsData)); return *this; } @@ -125,12 +132,14 @@ namespace Hyprutils { /* create a weak ptr from a shared ptr with assignment */ template validHierarchy&> operator=(const CSharedPointer& rhs) { - if (rc(impl_) == rc(rhs.impl_)) + if (rc(impl_) == rc(rhs.impl_)) { + m_data = Impl_::dataPointer(sc(rhs.get())); return *this; + } decrementWeak(); impl_ = rhs.impl_; - m_data = rhs.m_data; + m_data = Impl_::dataPointer(sc(rhs.get())); incrementWeak(); return *this; } @@ -250,10 +259,10 @@ namespace Hyprutils { CWeakPointer dynamicPointerCast(const CWeakPointer& ref) { if (!ref) return nullptr; - T* newPtr = dynamic_cast(sc(ref.impl_->getData())); + T* newPtr = dynamic_cast(ref.get()); if (!newPtr) return nullptr; - return CWeakPointer(ref.impl_, newPtr); + return CWeakPointer(ref.impl_, Impl_::dataPointer(newPtr)); } } } diff --git a/tests/memory/Memory.cpp b/tests/memory/Memory.cpp index fc37aed..86a9cf7 100644 --- a/tests/memory/Memory.cpp +++ b/tests/memory/Memory.cpp @@ -162,6 +162,35 @@ class CChildA : public InterfaceA { int m_childAInt = 4; }; +class IVirtualRoot { + public: + virtual ~IVirtualRoot() = default; + int m_rootInt = 5; +}; + +class IVirtualLeft : public virtual IVirtualRoot { + public: + virtual ~IVirtualLeft() = default; + int m_leftInt = 6; +}; + +class IVirtualRight : public virtual IVirtualRoot { + public: + virtual ~IVirtualRight() = default; + int m_rightInt = 7; +}; + +class CVirtualChild : public IVirtualLeft, public IVirtualRight { + public: + virtual ~CVirtualChild() = default; + int m_virtualChildInt = 8; +}; + +class CConstObject { + public: + int m_value = 9; +}; + static void testHierarchy() { // Same test for atomic and non-atomic { @@ -257,6 +286,132 @@ static void testHierarchy() { } } +static void testVirtualHierarchy() { + { + SP child = makeShared(); + + SP root = child; + SP right = dynamicPointerCast(root); + SP recastChild = dynamicPointerCast(root); + + EXPECT_TRUE(root); + EXPECT_TRUE(right); + EXPECT_TRUE(recastChild); + EXPECT_EQ(root->m_rootInt, 5); + EXPECT_EQ(right->m_rightInt, 7); + EXPECT_EQ(recastChild->m_virtualChildInt, 8); + + SP rootFromCast = dynamicPointerCast(child); + EXPECT_TRUE(rootFromCast); + EXPECT_EQ(rootFromCast->m_rootInt, 5); + + WP rootWeak = child; + WP rightWeak = dynamicPointerCast(rootWeak); + + EXPECT_TRUE(rootWeak); + EXPECT_TRUE(rightWeak); + EXPECT_EQ(rootWeak->m_rootInt, 5); + EXPECT_EQ(rightWeak->m_rightInt, 7); + EXPECT_EQ(rightWeak.lock()->m_rightInt, 7); + } + + { + ASP child = makeAtomicShared(); + + ASP root = child; + ASP right = dynamicPointerCast(root); + ASP recastChild = dynamicPointerCast(root); + + EXPECT_TRUE(root); + EXPECT_TRUE(right); + EXPECT_TRUE(recastChild); + EXPECT_EQ(root->m_rootInt, 5); + EXPECT_EQ(right->m_rightInt, 7); + EXPECT_EQ(recastChild->m_virtualChildInt, 8); + + ASP rootFromCast = dynamicPointerCast(child); + EXPECT_TRUE(rootFromCast); + EXPECT_EQ(rootFromCast->m_rootInt, 5); + + AWP rootWeak = root; + AWP rightWeak = dynamicPointerCast(rootWeak); + + EXPECT_TRUE(rootWeak); + EXPECT_TRUE(rightWeak); + EXPECT_EQ(rootWeak->m_rootInt, 5); + EXPECT_EQ(rightWeak->m_rightInt, 7); + EXPECT_EQ(rightWeak.lock()->m_rightInt, 7); + } +} + +static void testConstPointers() { + { + SP shared = makeShared(); + WP weak = shared; + + EXPECT_TRUE(shared); + EXPECT_TRUE(weak); + EXPECT_EQ(shared->m_value, 9); + EXPECT_EQ(weak->m_value, 9); + } + + { + UP unique = makeUnique(); + WP weak = unique; + + EXPECT_TRUE(unique); + EXPECT_TRUE(weak); + EXPECT_EQ(unique->m_value, 9); + EXPECT_EQ(weak->m_value, 9); + } + + { + SP child = makeShared(); + + SP root = child; + SP right = dynamicPointerCast(root); + SP recastChild = dynamicPointerCast(root); + + EXPECT_TRUE(root); + EXPECT_TRUE(right); + EXPECT_TRUE(recastChild); + EXPECT_EQ(root->m_rootInt, 5); + EXPECT_EQ(right->m_rightInt, 7); + EXPECT_EQ(recastChild->m_virtualChildInt, 8); + + WP rootWeak = child; + WP rightWeak = dynamicPointerCast(rootWeak); + + EXPECT_TRUE(rootWeak); + EXPECT_TRUE(rightWeak); + EXPECT_EQ(rootWeak->m_rootInt, 5); + EXPECT_EQ(rightWeak->m_rightInt, 7); + } + + { + ASP child = makeAtomicShared(); + + ASP root = child; + ASP right = dynamicPointerCast(root); + ASP recastChild = dynamicPointerCast(root); + + EXPECT_TRUE(root); + EXPECT_TRUE(right); + EXPECT_TRUE(recastChild); + EXPECT_EQ(root->m_rootInt, 5); + EXPECT_EQ(right->m_rightInt, 7); + EXPECT_EQ(recastChild->m_virtualChildInt, 8); + + AWP rootWeak = root; + AWP rightWeak = dynamicPointerCast(rootWeak); + + EXPECT_TRUE(rootWeak); + EXPECT_TRUE(rightWeak); + EXPECT_EQ(rootWeak->m_rootInt, 5); + EXPECT_EQ(rightWeak->m_rightInt, 7); + } +} + class CSelfDestruct { public: SP self; @@ -334,5 +489,9 @@ TEST(Memory, memory) { testHierarchy(); + testVirtualHierarchy(); + + testConstPointers(); + testSelfDestruct(); }