DFHack / df-structures

Dwarf Fortress data structure descriptions
https://github.com/DFHack/dfhack
114 stars 80 forks source link

Support stl-array #757

Closed lethosor closed 1 month ago

lethosor commented 1 month ago

Companion to https://github.com/DFHack/dfhack/pull/4591

Not necessary to merge until we need it.

ab9rf commented 1 month ago

we have been generally been rewriting std::array uses in bay12 headers as <static-array> because the implementation of std::array<T,sz> in both gcc and msvc is binary-equivalent to that of T[sz], so std::array isn't strictly necessary for the purposes of structures, but at the same time it allows for a more transparent respresentation of bay12 source documents. at the same time it's not likely to be harmful and i don't think it adds much to the maintainability burden, and it could be useful if we ever write something to directly parse bay12 header files....

lethosor commented 1 month ago

This was mostly intended to test the std::array specialization of generic_container_identity. I'm fine delaying this until a point when we do want std::array support.

One benefit of std::array is that bounds checking is available in C++, with .at(). static-array only provides checking to Lua.

ab9rf commented 1 month ago

One benefit of std::array is that bounds checking is available in C++, with .at(). static-array only provides checking to Lua.

I'd actually be fine with using std::array for all static arrays in the DF structures, in part because it provides bounds checking.

lethosor commented 1 month ago

To be clear - operator[] does not provide bounds checking, similar to std::vector. Only .at() does, and most of our code doesn't use .at().

chdoc commented 1 month ago

I don't know about MSVC, but for GCC, one can choose to compile operator[] with bounds checking:

Macros

_GLIBCXX_ASSERTIONS Undefined by default. When defined, enables extra error checking in the form of precondition assertions, such as bounds checking in strings and null pointer checks when dereferencing smart pointers.

(even though it isn't mentioned, this includes array)

myk002 commented 1 month ago

This was mostly intended to test the std::array specialization of generic_container_identity. I'm fine delaying this until a point when we do want std::array support.

I'd prefer to merge it if it makes sense. A forever-open PR is just bound to get out of sync with the main branch