ndsev / zserio

zero sugar, zero fat, zero serialization overhead
https://zserio.org/
BSD 3-Clause "New" or "Revised" License
108 stars 26 forks source link

Offsets using parameters should not be allowed #647

Open mikir opened 2 months ago

mikir commented 2 months ago

Consider the following schema:

struct OffsetHolder
{
    uint32 roomOffset;
};

struct Room(OffsetHolder offsetHolder)
{
offsetHolder.roomOffset:
    uint16 roomId;
};

struct School
{
    uint16 schoolId;
    OffsetHolder offsetHolder;
    Room(offsetHolder) room;
};

Such schema is completely valid but it prevents to pass parameters using const references (parameter offsetHolder in Room cannot be passed by const reference). This brings significant complexity for implementation of generators.

Because we cannot see any practical usage of this feature, it would be probable the easiest solution just to disable it. Perhaps, it will be enough to define offsets only where there are used:

struct Room
{
    OffsetHolder offsetHolder;
offsetHolder.roomOffset:
    uint16 roomId;
};

struct School
{
    uint16 schoolId;
    Room room;
};
mikir commented 1 month ago

This is a little bit controversial because it really could break the backward compatibility for some user schemas. We will have to rethink it twice if we can afford to do it in this milestone.

mikir commented 1 month ago

We will implement it only if this is absolutely necessary for new C++17 generator.

It has been moved to the backlog in the meantime.

Mi-La commented 3 weeks ago

If parameters will remain allowed in offset expression, we should create a new issue for checking duplicated use of offset fields - see #645 where only simple cases were covered.

Consider following schema:

struct Holder
{
    uint32 offset;
};

struct Parameterized(Holder holder)
{
    uint32 array1[];
holder.offset:
    string array2[];
};

struct Container
{
   Holder holder1;
   Holder holder2;
   Parameterized(holder1) param1; 
   Parameterized(holder2) param2; // this is ok since another instance of holder is used
   Parameterized(holder1) param3; // !!! here holder1.offset will be used again !!!
};

struct OtherContainer
{
    Holder holder;
holder.offset:
    Parameterized(holder) param; // !!! here holder.offset will be used again !!!
};

The problem is that we don't track instances of compound fields and thus we cannot check whether the same offset field is used multiple times or not.