Skip to content

genericize DataIdentity for container types #4591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ab9rf wants to merge 15 commits into
base: develop
Choose a base branch
from
Draft

genericize DataIdentity for container types #4591

Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
63a35c3
genericize `DataIdentity` for container types
ab9rf May 14, 2024
9701cf8
gcc-12 compliance
ab9rf May 15, 2024
d7edcce
add omitted method to `generic_container_identity`
ab9rf May 15, 2024
b6af192
use the correct type variable
ab9rf May 15, 2024
0c7dab2
make `getFullName` less verbose
ab9rf May 15, 2024
20d645c
add `insert` requirement to `isIndexedContainer`
ab9rf May 15, 2024
4aea762
make `generic_assoc_container_identity` read only
ab9rf May 15, 2024
93d9977
change "container" to "vector" in full name
ab9rf May 16, 2024
ed44b01
Revert "change "container" to "vector" in full name"
ab9rf May 16, 2024
95116d5
just "change the conditions of the test"
ab9rf May 16, 2024
2b25eb6
generic_container_identity::getFullName(): return custom names for ve…
lethosor May 17, 2024
0afd6a9
add ResizableContainer concept
ab9rf May 17, 2024
bea43f6
do resizable containers correctly
ab9rf May 17, 2024
7ce040b
Update library/include/DataIdentity.h
ab9rf May 19, 2024
6944ca6
support msvc 1940 (VS 17.10)
ab9rf May 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 142 additions & 54 deletions library/include/DataIdentity.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ distribution.
#include <variant>
#include <vector>
#include <variant>
#include <ranges>
#include <array>

#include "DataDefs.h"

Expand Down Expand Up @@ -113,10 +115,10 @@ namespace DFHack

class DFHACK_EXPORT container_identity : public constructed_identity {
type_identity *item;
enum_identity *ienum;
type_identity *ienum;
Copy link
Member

@lethosor lethosor May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this changed

Copy link
Member Author

@ab9rf ab9rf May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specifically because of associative containers, the identity key type of an associative container goes there

Copy link
Member

@lethosor lethosor May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I wonder if there is a way to enforce that this is an enum_identity for non-associative containers. Probably complicated.


public:
container_identity(size_t size, TAllocateFn alloc, type_identity *item, enum_identity *ienum = NULL)
container_identity(size_t size, TAllocateFn alloc, type_identity *item, type_identity *ienum = NULL)
: constructed_identity(size, alloc), item(item), ienum(ienum) {};

virtual identity_type type() { return IDTYPE_CONTAINER; }
Expand Down Expand Up @@ -190,6 +192,102 @@ namespace DFHack
virtual bool get_item(void *ptr, int idx) = 0;
virtual void set_item(void *ptr, int idx, bool val) = 0;
};

template <typename C>
concept isIndexedContainer = requires (C c, C::size_type sz) {
{ c[sz] } -> std::same_as<typename C::reference>;
{ c.size() } ->std::same_as<typename C::size_type>;
{ c.begin() } -> std::same_as<typename C::iterator>;
{ c.end() } -> std::same_as<typename C::iterator>;
};

template <typename C>
concept isResizableContainer = isIndexedContainer<C> && requires (C c, C::iterator i, C::size_type sz, C::value_type v) {
{ c.erase(i) };
{ c.resize(sz) };
{ c.insert(i, v) };
};

template<isIndexedContainer C>
class generic_container_identity : public container_identity {
private:
using T = C::value_type;

template<typename T> struct type_name {
static const inline std::string name = "container";
};
template<typename T> struct type_name<std::vector<T>> {
static const inline std::string name = "vector";
};
template<typename T> struct type_name<std::deque<T>> {
static const inline std::string name = "deque";
};
template<typename T, int sz> struct type_name<std::array<T, sz>> {
Copy link
Member

@lethosor lethosor May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be

Suggested change
template<typename T, int sz> struct type_name<std::array<T, sz>> {
template<typename T, size_t sz> struct type_name<std::array<T, sz>> {

to match properly.

I am debating removing the name = "container" from the base template to catch this sort of thing.

Copy link
Member

@lethosor lethosor May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I was testing with: DFHack/df-structures#757

Copy link
Member Author

@ab9rf ab9rf May 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am debating removing the name = "container" from the base template to catch this sort of thing.

i was ambivalent on that issue originally. it's not hard for us to add a new specialization but it's more of a burden for a developer of an external plugin. maybe only include the generic case in the library build? that would cause any unspecialized in core code to error at compile time, while allowing plugin developers freedom to use specialized containers for their own purposes (although i'm not sure what the scenario for that is, tbh) nah i don't think that does what i want.. maybe someone else would have a better idea

Copy link
Member

@lethosor lethosor May 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support for adding new types in plugins is limited anyway. Their identities won't be registered in the global registry tables in #4591 (comment) (although I'm a little unsure whether any container types are registered there).

Really, my intent of breaking the base template would be to catch specializations like array<T, int> that are intended to work but don't. It took me a while to figure out why a std::array was printing as "container" instead of "array".

static const inline std::string name = "array";
};

public:
generic_container_identity() : container_identity(sizeof(C), &df::allocator_fn<C>, df::identity_traits<T>::get()) {};
std::string getFullName(type_identity* item) { return type_name<C>::name + container_identity::getFullName(item); }
protected:
virtual int item_count(void* ptr, CountMode cnt) { return (int)((C*)ptr)->size(); }
virtual void* item_pointer(type_identity* item, void* ptr, int idx) { return &(*(C*)ptr)[idx]; }
};

template<isResizableContainer C>
class resizable_container_identity : public generic_container_identity<C> {
private:
using T = C::value_type;
public:
virtual bool resize(void* ptr, int size) { ((C*)ptr)->resize(size); return true; }
virtual bool erase(void* ptr, int size) { auto& ct = *(C*)ptr; ct.erase(ct.begin() + size); return true; }
virtual bool insert(void* ptr, int idx, void* item) { auto& ct = *(C*)ptr; ct.insert(ct.begin() + idx, *(T*)item); return true; }
};

template <typename C>
concept isAssocContainer = requires (C c, C::iterator i, C::key_type k) {
{ c[k] } -> std::same_as<typename C::mapped_type &>;
{ c.size() } -> std::same_as<typename C::size_type>;
{ c.begin() } -> std::same_as<typename C::iterator>;
{ c.end() } -> std::same_as<typename C::iterator>;
};

template<isAssocContainer C>
class generic_assoc_container_identity : public container_identity {
private:
using KT = C::key_type;
using T = C::mapped_type;
public:
generic_assoc_container_identity() : container_identity(sizeof(C), &df::allocator_fn<C>, df::identity_traits<T>::get(), df::identity_traits<KT>::get()) {};
std::string getFullName(type_identity* item) { return "map" + container_identity::getFullName(item); }
virtual bool is_readonly() { return true; }

protected:
virtual int item_count(void* ptr, CountMode cnt) { return (int)((C*)ptr)->size(); }
virtual void* item_pointer(type_identity* item, void* ptr, int idx) { auto iter = (((C*)ptr)->begin()); while(idx--) iter++; return (void*)&(iter->second); }
};

template <typename C>
concept isSetContainer = requires (C c, C::iterator i, C::key_type k) {
requires std::same_as<typename C::key_type, typename C::value_type>;
{ c.contains(k) } -> std::same_as<bool>;
{ c.size() } -> std::same_as<typename C::size_type>;
{ c.begin() } -> std::same_as<typename C::iterator>;
{ c.end() } -> std::same_as<typename C::iterator>;
};

template<isSetContainer C>
class generic_set_container_identity : public container_identity {
private:
using KT = C::key_type;
public:
generic_set_container_identity() : container_identity(sizeof(C), &df::allocator_fn<C>, df::identity_traits<KT>::get()) {};
std::string getFullName(type_identity* item) { return "set" + container_identity::getFullName(item); }
virtual bool is_readonly() { return true; }
protected:
virtual int item_count(void* ptr, CountMode cnt) { return (int)((C*)ptr)->size(); }
virtual void* item_pointer(type_identity* item, void* ptr, int idx) { auto iter = (((C*)ptr)->begin()); while (idx--) iter++; return (void*)&(*iter); }
};
}

namespace df
Expand All @@ -201,6 +299,15 @@ namespace df
using DFHack::container_identity;
using DFHack::ptr_container_identity;
using DFHack::bit_container_identity;
using DFHack::generic_container_identity;
using DFHack::resizable_container_identity;
using DFHack::generic_assoc_container_identity;
using DFHack::generic_set_container_identity;

using DFHack::isIndexedContainer;
using DFHack::isResizableContainer;
using DFHack::isAssocContainer;
using DFHack::isSetContainer;

class DFHACK_EXPORT number_identity_base : public primitive_identity {
const char *name;
Expand Down Expand Up @@ -577,6 +684,7 @@ namespace df
INTEGER_IDENTITY_TRAITS(unsigned long long);
FLOAT_IDENTITY_TRAITS(float);
FLOAT_IDENTITY_TRAITS(double);

OPAQUE_IDENTITY_TRAITS(std::condition_variable);
OPAQUE_IDENTITY_TRAITS(std::fstream);
OPAQUE_IDENTITY_TRAITS(std::mutex);
Expand Down Expand Up @@ -658,31 +766,13 @@ namespace df
static container_identity *get();
};

template<class T> struct identity_traits<std::vector<T> > {
static container_identity *get();
};
#endif

template<class T> struct identity_traits<std::vector<T*> > {
static stl_ptr_vector_identity *get();
};

#ifdef BUILD_DFHACK_LIB
template<class T> struct identity_traits<std::deque<T> > {
static container_identity *get();
};

template<class T> struct identity_traits<std::set<T> > {
static container_identity *get();
};

template<class KT, class T> struct identity_traits<std::map<KT, T>> {
static container_identity *get();
};

template<class KT, class T> struct identity_traits<std::unordered_map<KT, T>> {
static container_identity *get();
};

template<> struct identity_traits<BitArray<int> > {
static bit_array_identity identity;
Expand Down Expand Up @@ -712,6 +802,38 @@ namespace df
}
#endif

template<isIndexedContainer C>
struct DFHACK_EXPORT identity_traits<C> {
static type_identity* get() {
static generic_container_identity<C> identity{};
return &identity;
}
};

template<isResizableContainer C>
struct DFHACK_EXPORT identity_traits<C> {
static type_identity* get() {
static resizable_container_identity<C> identity{};
return &identity;
}
};

template<isAssocContainer C>
struct DFHACK_EXPORT identity_traits<C> {
static type_identity* get() {
static generic_assoc_container_identity<C> identity{};
return &identity;
}
};

template<isSetContainer C>
struct DFHACK_EXPORT identity_traits<C> {
static type_identity* get() {
static generic_set_container_identity<C> identity{};
return &identity;
}
};

template<class T>
inline pointer_identity *identity_traits<T *>::get() {
static pointer_identity identity(identity_traits<T>::get());
Expand All @@ -725,12 +847,6 @@ namespace df
return &identity;
}

template<class T>
inline container_identity *identity_traits<std::vector<T> >::get() {
typedef std::vector<T> container;
static stl_container_identity<container> identity("vector", identity_traits<T>::get());
return &identity;
}
#endif

template<class T>
Expand All @@ -740,34 +856,6 @@ namespace df
}

#ifdef BUILD_DFHACK_LIB
template<class T>
inline container_identity *identity_traits<std::deque<T> >::get() {
typedef std::deque<T> container;
static stl_container_identity<container> identity("deque", identity_traits<T>::get());
return &identity;
}

template<class T>
inline container_identity *identity_traits<std::set<T> >::get() {
typedef std::set<T> container;
static ro_stl_container_identity<container> identity("set", identity_traits<T>::get());
return &identity;
}

template<class KT, class T>
inline container_identity *identity_traits<std::map<KT, T>>::get() {
typedef std::map<KT, T> container;
static ro_stl_assoc_container_identity<container> identity("map", identity_traits<KT>::get(), identity_traits<T>::get());
return &identity;
}

template<class KT, class T>
inline container_identity *identity_traits<std::unordered_map<KT, T>>::get() {
typedef std::unordered_map<KT, T> container;
static ro_stl_assoc_container_identity<container> identity("unordered_map", identity_traits<KT>::get(), identity_traits<T>::get());
return &identity;
}

template<class T>
inline bit_container_identity *identity_traits<BitArray<T> >::get() {
static bit_array_identity identity(identity_traits<T>::get());
Expand Down