feat: EBO‑optimized UniquePtr<T, Deleter> #40

brodheadw · 2025-05-14T00:11:57Z

Closes #1: implement std‑like UniquePtr with EBO, converting moves, nullptr‑assign, etc.

…n requirement to Development Setup section of CONTRIBUTING.md

…d version requirement to Development Setup section of CONTRIBUTING.md" This reverts commit 2c4c34f.

…n requirement to Development Setup section of CONTRIBUTING.md

- Create radiant/UniquePtr.h with the class template declaration, default and pointer‐taking constructors, destructor and move‐ops. - Add placeholder comments for the missing member functions.

Change UniquePtr to inherit from an empty DefaultDelete deleter so sizeof(UniquePtr<T>) == sizeof(T*) Fix ctor to initialize base deleter, not mistakenly pass pointer Remove stored function-pointer field, rely on empty-base optimization Update UniquePtrDefault alias to use the new DefaultDelete Add test/test_UniquePtr.cpp covering basic lifetime, release/reset, move, swap and custom deleter size guarantee test: revise tests for EBO-optimized DefaultDelete (drop old alias, move D and its count to namespace scope, simplify static_assert, update CustomDeleter test)

…sive tests Introduce DefaultDelete<T> as an empty deleter type to enable empty-base-optimization Refactor UniquePtr<T, Deleter> to inherit from Deleter, removing extra storage and ensuring sizeof(UniquePtr) == sizeof(T*) Add move-ctor, move-assign, release, reset, swap, and nullptr conversion APIs Provide ADL-friendly swap overload Update alias UniquePtrDefault<T> to use default deleter Add test/test_UniquePtr.cpp with GoogleTest suite covering basic lifetime, sizeof check, release/reset, move semantics, swap semantics, and custom deleter

…d of std traits add converting constructor to DefaultDelete to allow DefaultDelete<Base> from DefaultDelete<Derived> remove <type_traits> dependency and replace std::is_convertible_v with rad::IsConv keep existing move and converting-move overloads unchanged; tests continue to pass

…/Utility.h

github-actions · 2025-05-14T00:12:12Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

brodheadw · 2025-05-14T00:18:37Z

I have read the CLA Document and I hereby sign the CLA

ben-craig-cs

Thanks for showing interest in this project! We're definitely in need of a UniquePtr class.

Have you considered also adding UniquePtr<T[], D>? https://eel.is/c++draft/unique.ptr.runtime

ben-craig-cs · 2025-05-14T12:46:38Z

radiant/UniquePtr.h

+
+/// @brief Default deleter using Radiant's StdAllocator (EBO if empty).
+template <typename T>
+struct DefaultDelete : private StdAllocator {


Do we need to couple DefaultDelete to an allocator? Do we need to make DefaultDelete a template?

I will suggest that we instead have a non-templated DefaultDelete, where the op() is similar to the following:

template <typename T> constexpr void operator()(const T *t) const { // require complete types static_assert(sizeof(T)!=0); delete t; }

https://godbolt.org/z/fPv9b8oPh

I don't think we need any of the allocator related methods, at least not for the default deleter.

ben-craig-cs · 2025-05-14T12:52:58Z

radiant/UniquePtr.h

+/// @brief A unique ownership smart pointer with customizable deleter.
+/// If the deleter is empty (via EBO), sizeof(UniquePtr)==sizeof(T*).
+template <typename T, typename Deleter = DefaultDelete<T>>
+class UniquePtr : private Deleter {


The goal here is correct: have a UniquePtr with sizeof(T*). We shouldn't get there by having UniquePtr inherit from Deleter though. If Deleter happened to have virtual methods, then that would make UniquePtr polymorphic as well.

Instead, put the Deleter in an EmptyOptimizedPair. That still does EBO, but it's on an internal type that won't leak subtle Deleter details into the UniquePtr.

If you're feeling really adventurous, you can also figure out the compatibility and support matrix, and then use [[no_unique_address]] and [[msvc::no_unique_address]] instead of EmptyOptimizedPair instead. I don't think we're ready for that yet though.

ben-craig-cs · 2025-05-14T12:59:01Z

radiant/UniquePtr.h

+public:
+    RAD_NOT_COPYABLE(UniquePtr);
+
+    using pointer      = T*;


Look at Vector.h for how we do these kinds of typedefs.
s/pointer/PointerType
s/deleter_type/DeleterType

ben-craig-cs · 2025-05-14T13:08:53Z

radiant/UniquePtr.h

+    constexpr UniquePtr() noexcept = default;
+
+    /// @brief Constructs owning p, with optional custom deleter d.
+    explicit constexpr UniquePtr(pointer p, Deleter d = Deleter()) noexcept


I'm going to suggest mirroring the standard's unique_ptr overload set as best as possible, and then justify where you diverge.
https://eel.is/c++draft/unique.ptr

For this constructor, I think you should avoid default arguments. I think the default argument here forces the deleter to be movable, where if this were a single pointer ctor, then very careful UniquePtr usage could let a deleter get away with being immovable.

I'll also recommend that you add a type_identity_t analog, and use that in the UniquePtr constructors. That will suppress constructor template argument deduction (CTAD). We don't want CTAD here, because pointers to single elements and pointers to arrays look the same, and we don't want to make it unnecessarily easy to bind a scalar UniquePtr to an array allocation.

You're right, I've now split the pointer + deleter ctor into two overloads so the single-arg ctor only ever default-constructs the deleter so an immovable custom deleter will still work. Two-arg ctor now only participates when you explicitly supply a deleter.

Also wrapped the pointer parameter in TypeIdentityT alias to suppress CTAD and avoid accidentally binding a new T[...] array to a scalar.

Also I am now making PRs to main on my own fork.

ben-craig-cs · 2025-05-14T13:14:26Z

radiant/UniquePtr.h

+    }
+
+    /// @brief Implicit bool conversion: true if non-null.
+    constexpr operator bool() const noexcept { return m_ptr != nullptr; }


Make this explicit

ben-craig-cs · 2025-05-14T13:16:28Z

radiant/UniquePtr.h

+    constexpr operator bool() const noexcept { return m_ptr != nullptr; }
+
+    /// @brief Observers
+    constexpr pointer get() const noexcept       { return m_ptr; }


Rename to Get(). Using STL naming conventions is ok when it enables substantial generic interoperability (notably, things like begin and end). I don't think get crosses that threshold, as the operator interface is the generic interface.

Similar comments throughout the file, like with release, reset, and swap

ben-craig-cs · 2025-05-14T13:18:23Z

radiant/UniquePtr.h

+
+/// @brief ADL swap overload.
+template <typename T, typename D>
+void swap(UniquePtr<T,D>& a, UniquePtr<T,D>& b) noexcept {


Radiant's Swap is in Algorithm.h

ben-craig-cs · 2025-05-14T13:25:45Z

radiant/UniquePtr.h

+
+/// @brief Allocator-aware make_unique: allocates via AllocTraits, constructs, and returns UniquePtr.
+template <typename T, typename... Args>
+UniquePtr<T> make_unique(Args&&... args) {


As written, this doesn't do much with regards to allocators, as it forces StdAllocator. I think there's potential here though.

I'll suggest adding an AllocatorDeleter<Alloc>, and then changing make_unique to something like the following:

template <typename T, typename Alloc, typename... Args> UniquePtr<AllocatorDeleter<Alloc>> MakeUnique(Alloc alloc, Args&&... args) { /*...*/ }

ben-craig-cs · 2025-05-14T13:27:09Z

radiant/UniquePtr.h

+
+    A alloc;
+    T* raw = Traits::template Alloc<T>(alloc, 1);
+    Traits::Construct(alloc, raw, rad::Forward<Args>(args)...);


need to check raw for null before constructing. Return an empty UniquePtr if that happens.

brodheadw added 12 commits May 8, 2025 15:45

docs: Fixed typo in SECURITY.md ('Raditn' -> 'Radiant'), added versio…

2c4c34f

…n requirement to Development Setup section of CONTRIBUTING.md

Revert "docs: Fixed typo in SECURITY.md ('Raditn' -> 'Radiant'), adde…

20fb702

…d version requirement to Development Setup section of CONTRIBUTING.md" This reverts commit 2c4c34f.

docs: Fixed typo in SECURITY.md ('Raditn' -> 'Radiant'), added versio…

294483d

…n requirement to Development Setup section of CONTRIBUTING.md

Updated SECURITY.md again.

343dd2d

feat: add rad::UniquePtr skeleton

7f7b29e

- Create radiant/UniquePtr.h with the class template declaration, default and pointer‐taking constructors, destructor and move‐ops. - Add placeholder comments for the missing member functions.

feat(unique_ptr): functional UniquePtr implementation

5b76d1b

feat(unique_ptr): Changed std::move to rad::Move after import radiant…

cd2352e

…/Utility.h

feat(unique_ptr): Changed all std:: to rad::

1bc834f

feat(memory): rework UniquePtr to use Radiant allocator machinery

3b5a589

brodheadw closed this May 14, 2025

ben-craig-cs suggested changes May 14, 2025

View reviewed changes

brodheadw deleted the feat/unique_ptr branch May 14, 2025 14:08

brodheadw restored the feat/unique_ptr branch May 14, 2025 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: EBO‑optimized UniquePtr<T, Deleter> #40

feat: EBO‑optimized UniquePtr<T, Deleter> #40

Uh oh!

brodheadw commented May 14, 2025

Uh oh!

github-actions bot commented May 14, 2025 •

edited

Loading

Uh oh!

brodheadw commented May 14, 2025

Uh oh!

ben-craig-cs left a comment

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

brodheadw May 15, 2025

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

ben-craig-cs May 14, 2025

Uh oh!

Uh oh!

feat: EBO‑optimized UniquePtr<T, Deleter> #40

feat: EBO‑optimized UniquePtr<T, Deleter> #40

Uh oh!

Conversation

brodheadw commented May 14, 2025

Uh oh!

github-actions bot commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brodheadw commented May 14, 2025

Uh oh!

ben-craig-cs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

feat: EBO‑optimized UniquePtr<T, Deleter> #40

feat: EBO‑optimized UniquePtr<T, Deleter> #40

github-actions bot commented May 14, 2025 •

edited

Loading