Open TG-SAG opened 6 months ago
I might do a pull request on the change I have on my fork. But I'm not sure on how to write an ETL test for this issue because I don't want to change the configuration for the ETL unit tests (uint_least8_t). I basically want to use the existing tests and run them on the default element type AND on a bigger element type (e.g. uint32_t).
Any ideas on how I might integrate that in the ETL test suite (without duplicating sources)?
As the setting is a project compile time value, testing it with various element types is difficult. As it's a simple and easily understood change, I think we can safely ignore trying to force the unit tests to cover it. The newer bitset implementation should normally be the preferred choice.
I'm currently doing some updates to etl::bitset
for another issue (#774) so I'll just incorporate these changes at the same time.
You're correct in saying that the new bitset should be used. I will make the switch to the new version ASAP. Only because I wanted to run my test suite on the currently used implementation I did run across these problems.
Thanks for your commit!
The legacy
etl::bitset<size_t N>
uses uint_least8_t as the element type. It can be changed if you setETL_BITSET_ELEMENT_TYPE
to a different type (see https://etlcpp.com/bitset_legacy.html).This did work until the new
etl::bitset<size_t N, typename TElement>
was introduced. Since then the set/reset methods don't work anymore if the element type is bigger than 8 bit: https://gcc.godbolt.org/z/GchjbGYz3 (sorry about the weird construction of the bitset, but I needed a defined memory initialization)The error seems to be with the usage of
memset
in the mention methods. As an example this is the implementation ofreset()
:Because
memset
does copy char's (8 bit) andNumber_Of_Elements
is the count of base types that represent the bits (basicallyMaxN / Bits_Per_Element
) not all memory locations a reset to 0x00.This can be possibly fixed be multiplying
Number_Of_Elements
with the size of the element type or by usingetl::fill[_n]()
: