From c6b55d4da33a2ae21ea2fda4339735d331962080 Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Wed, 22 Oct 2025 20:34:21 -0500 Subject: [PATCH] simplify the allocator current code has three different allocator functions, the generic one, one labeled "nodel" and one labeled "noassign" the `noassign` allocator just mimics the behavior of the basic one when the type in question is not copy-assignable and is only being used for types that are not copy-assignable so the default allocator will work _exactly_ the same anyway the `nodel` version is entirely unused thus, this PR removes the `nodel` and `noassign` versions and folds `try_assign` into the allocator so that all the logic is in one place also add in-code documentation --- library/DataIdentity.cpp | 2 +- library/include/DataDefs.h | 77 +++++++++++++++++++--------------- library/include/DataIdentity.h | 2 +- 3 files changed, 46 insertions(+), 35 deletions(-) diff --git a/library/DataIdentity.cpp b/library/DataIdentity.cpp index a385915d92..1bbeb64e77 100644 --- a/library/DataIdentity.cpp +++ b/library/DataIdentity.cpp @@ -19,7 +19,7 @@ namespace df { #define INTEGER_IDENTITY_TRAITS(type, name) NUMBER_IDENTITY_TRAITS(integer, type, name) #define FLOAT_IDENTITY_TRAITS(type) NUMBER_IDENTITY_TRAITS(float, type, #type) #define OPAQUE_IDENTITY_TRAITS_NAME(name, ...) \ - const opaque_identity identity_traits<__VA_ARGS__ >::identity(sizeof(__VA_ARGS__), allocator_noassign_fn<__VA_ARGS__ >, name) + const opaque_identity identity_traits<__VA_ARGS__ >::identity(sizeof(__VA_ARGS__), allocator_fn<__VA_ARGS__ >, name) #define OPAQUE_IDENTITY_TRAITS(...) OPAQUE_IDENTITY_TRAITS_NAME(#__VA_ARGS__, __VA_ARGS__ ) INTEGER_IDENTITY_TRAITS(char, "char"); diff --git a/library/include/DataDefs.h b/library/include/DataDefs.h index 9d1eb4c664..7fac3c0c5e 100644 --- a/library/include/DataDefs.h +++ b/library/include/DataDefs.h @@ -72,7 +72,7 @@ namespace DFHack PTRFLAG_HAS_BAD_POINTERS = 2, }; - typedef void *(*TAllocateFn)(void*,const void*); + using TAllocateFn = void *(*)(void*, const void*); class DFHACK_EXPORT type_identity { const size_t size; @@ -122,10 +122,10 @@ namespace DFHack constructed_identity(size_t size, const TAllocateFn alloc) : type_identity(size), allocator(alloc) {}; - virtual bool can_allocate() const { return (allocator != NULL); } - virtual void *do_allocate() const { return allocator(NULL,NULL); } + virtual bool can_allocate() const { return (allocator != nullptr); } + virtual void *do_allocate() const { return allocator(nullptr,nullptr); } virtual bool do_copy(void *tgt, const void *src) const { return allocator(tgt,src) == tgt; } - virtual bool do_destroy(void *obj) const { return allocator(NULL,obj) == obj; } + virtual bool do_destroy(void *obj) const { return allocator(nullptr,obj) == obj; } public: virtual bool isPrimitive() const { return false; } virtual bool isConstructed() const { return true; } @@ -526,42 +526,53 @@ namespace df using DFHack::DfLinkedList; using DFHack::DfOtherVectors; + /* + * + * Allocator functions are used to allocate, deallocate, and copy-assign objects + * + * When out is non-null, the object pointed to by in is copy-assigned over the object + * pointed to by out, if possible. When assignment is possible, out is returned. + * When assignment is not possible, nothing is done and nullptr is returned. + * The type must be copy-assignable for this to work; move-assignment is not + * supported. Callers can determine if the assignment succeeded by checking for the + * return value matching out (or simply being not null). + * + * When only in is non-null, the object pointed to by in is destroyed and deallocated, + * and in is returned. Note that the return value points to deallocated memory + * and should not be dereferenced. + * + * When both out and in are null, a new object is constructed and returned. + * + * Calling an allocator function with out non-null and in null is undefined behavior. + * + */ + template concept copy_assignable = std::assignable_from && std::assignable_from; template - void* allocator_try_assign(void *out, const void *in) { - if constexpr (copy_assignable) { - *(T*)out = *(const T*)in; - return out; - } - else { - // assignment is not possible; do nothing - return nullptr; + void *allocator_fn(void *out, const void *in) { + if (out) + { + if constexpr (copy_assignable) + { + *(T*)out = *(const T*)in; + return out; + } + else + { + return nullptr; + } } - } - + else if (in) + { #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor" - template - void *allocator_fn(void *out, const void *in) { - if (out) { return allocator_try_assign(out, in); } - else if (in) { delete (T*)in; return (T*)in; } - else return new T(); - } + delete (T*)in; #pragma GCC diagnostic pop - - template - void *allocator_nodel_fn(void *out, const void *in) { - if (out) { *(T*)out = *(const T*)in; return out; } - else if (in) { return NULL; } - else return new T(); - } - - template - void *allocator_noassign_fn(void *out, const void *in) { - if (out) { return NULL; } - else if (in) { delete (T*)in; return (T*)in; } - else return new T(); + return (T*)in; + } + else + return new T(); } template diff --git a/library/include/DataIdentity.h b/library/include/DataIdentity.h index e9fb5a9aa3..2c302264d9 100644 --- a/library/include/DataIdentity.h +++ b/library/include/DataIdentity.h @@ -638,7 +638,7 @@ namespace df static opaque_identity *get() { using type = std::shared_ptr; static std::string name = std::string("shared_ptr<") + typeid(T).name() + ">"; - static opaque_identity identity(sizeof(type), allocator_noassign_fn, name); + static opaque_identity identity(sizeof(type), allocator_fn, name); return &identity; } };