zserio icon indicating copy to clipboard operation
zserio copied to clipboard

Define a fundamental type for array sizes

Open reinco opened this issue 2 years ago • 2 comments

Currently there is no fundamental type for array sizes. This leads to a problem with the generated c++ api. In the code example below, the numPositions will get type 'uint32_t'. However, the field is used to express the size of array 'positions', 'positions' becomes an std::vector in the c++ emitted code. An std::vector has a function std::vector::size() that returns a size_t.
We use the following to set the positions field of struct Data:

Data::setPositions(static_cast<uint32_t>(positions.size());

In this the static_cast seems unnatural and could lead to subtle errors.

Current definition in zserio script:

struct Data
{
   varsize numPositions;
   packed Position positions[numPositions];
};

Instead I propose a new fundamental type, arraysize. It should have the same size on the bitstream as varsize, but the type of getNumPositions() and setNumPositions() in the generated code should be size_t:

struct Data
{
   arraysize numPositions;
   packed Position positions[numPositions];
};

reinco avatar Oct 03 '22 10:10 reinco

Maybe it's enough just to map varsize directly to size_t? (and continue to check VARSIZE_MAX limit during write...)

Mi-La avatar Oct 03 '22 10:10 Mi-La

We emit uint32_t instead of a size_t for varsize just because of saving memory for 64-bit OS. The memory is probably not a big issue now, so we might think to change this mapping to solve the casting issue as you correctly pointed out.

mikir avatar Oct 07 '22 10:10 mikir