legacy etl::bitset set/reset does not work if the element type is greater than 8 bit

[TG-SAG]

The legacy etl::bitset<size_t N> uses uint_least8_t as the element type. It can be changed if you set ETL_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 of reset():

     ibitset& reset() {
         ::memset(pdata, 0x00, Number_Of_Elements);
         return *this;
     }

Because memset does copy char's (8 bit) and Number_Of_Elements is the count of base types that represent the bits (basically MaxN / 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 using etl::fill[_n]():

diff --git a/include/etl/private/bitset_legacy.h b/include/etl/private/bitset_legacy.h
index 24052e41..0d974c40 100644
--- a/include/etl/private/bitset_legacy.h
+++ b/include/etl/private/bitset_legacy.h
@@ -311,7 +311,7 @@ namespace etl
     //*************************************************************************
     ibitset& set()
     {
-      ::memset(pdata, 0xFF, Number_Of_Elements);
+      etl::fill_n(pdata, Number_Of_Elements, -1);
       pdata[Number_Of_Elements - 1U] = Top_Mask;
 
       return *this;
@@ -512,7 +512,7 @@ namespace etl
     //*************************************************************************
     ibitset& reset()
     {
-      ::memset(pdata, 0x00, Number_Of_Elements);
+      etl::fill_n(pdata, Number_Of_Elements, 0);
 
       return *this;
     }

[TG-SAG]

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)?

[jwellbelove]

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.

[TG-SAG]

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!