Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support fixed length arrays within structs? #63

Closed
vgough opened this issue Sep 3, 2014 · 13 comments
Closed

support fixed length arrays within structs? #63

vgough opened this issue Sep 3, 2014 · 13 comments

Comments

@vgough
Copy link

vgough commented Sep 3, 2014

Feature request: fixed length arrays, especially allowed within structs.

If I want to store a 256-bit hash in a struct, I'd either need to convert to a table in order to use [ubyte], or else store 4 ulongs (which is ugly). Would be convenient if there was a way to store a fixed-length array in a struct.

@ghost
Copy link

ghost commented Sep 3, 2014

I agree, this would be nice to have, and a logical fit for structs. It's a bit of a bigger feature though, since it lacks forwards compatibility on the schema side.

@shaxbee
Copy link
Contributor

shaxbee commented Sep 24, 2014

+1 very useful for stuff like transformation matrices etc.

@orientalpers
Copy link

Yeah, i vote for this feature too.

@falconair
Copy link

Any word on this feature? I have a large definition file where my definitions have strings of pre-specified length (my target is mainly c++). I would love to be able to use just structs, rather than tables.

@ahundt
Copy link

ahundt commented Sep 6, 2015

sounds very useful!

@ghost
Copy link

ghost commented Sep 9, 2015

Noone has attempted this yet. As I said it is a larger feature at this point, even if initially only implemented for C++, since it be an entirely new datatype supported by FlatBuffers.

For simplicity, we could allow it only in structs. We could also allow it in tables (inline, much like a struct), but this can also already be achieved by simply wrapping this array in a struct and then sticking that in a table, which would give identical results.

It would not break compatibility, since you already can't change structs once defined, so a struct with such an array by definition is a new struct, and by definition is referenced by a new field in a table. This means that old code simply won't access this new field, so it is "forwards & backwards compatible" on a binary level, in that sense.

It of course breaks forwards compatibility in schemas, and it breaks all those schemas with the other language implementations until they support the feature. In fact, if we implemented it in C++ only, all the other code generators will at least need to be aware of it and error-out. We'll need separate tests just for this feature.

Then we just need implementation for all functionality, specific to this datatype, which is quite a lot of places.

Quite a bit of work, but if any of you are familiar enough with the C++ implementation, it should not be that crazy. If not, I'll eventually get to it, but I can't promise when.

@falconair
Copy link

I almost created a new ticket for this, but it is probably enough just to add a comment here. It would be nice to be able to define a 'static' string attribute in a schema. For example:
table TestEvent{
event_id :string = "test_event" ;
}

This seems to be the kind of thing that should be added whenever static length strings/vectors are implemented (just like numbers can be defaulted).

@falconair
Copy link

@gwvo could you start to provide some guidance on how this feature might be implemented. Will there need to be custom fixed size vector and string classes? How will the schema definitions change, etc.? Perhaps if you add more information on how expect this feature to be implemented, some C++ dev will take a crack (unfortunately I'm not a c++ guy).

@ghost
Copy link

ghost commented Dec 14, 2015

@falconair some of that is already above. To be more specific:

A new type in the schema language. Before we had field : [float] as a way to declare arbitrary size vectors (allowed in tables only), now we'd have field : [float:4] to declare a fixed array (allowed inside structs only).

On a binary level, this represents exactly the same data as if you had written fieldN : float 4 times. It does not have a runtime size field, though it may have a generated code function/constant/enum to indicate the fixed size.

Rather than reusing the BASE_TYPE_VECTOR for this, I'd introduce a new BASE_TYPE_ARRAY in the C++ code. Then comes the fun work of ensuring this is taken into account in all places that deal with schema types, which is a lot.

kakikubo pushed a commit to kakikubo/flatbuffers that referenced this issue Apr 19, 2016
…_distribution

support LOCATION CHARACTER UI INCLUDE in copy_resource.py
@idoroshev
Copy link

idoroshev commented Jul 14, 2016

Where can we store the length of such array after type parsing?
We could create a new field in Type struct or create an inheriting struct with this field, but I doubt, these solutions are great.

@ghost
Copy link

ghost commented Jul 14, 2016

I think a new fixed_length inside Type is ok. If you make it a short and place it after the element field, the Type object won't even grow in size :)
short is ok, because a struct is meant to be used inside a table, meaning it needs to be (way) smaller than 16bit anyway for the vtable offsets to fit.

@letmaik
Copy link

letmaik commented Jul 4, 2019

How about using a new built-in "size" attribute for this and, for structs, allow using vectors but only if they have the size attribute. E.g. hash:[ubyte] (size: 32); For structs, there is some implementation work, but for tables I could imagine that the existing format is kept as-is and the size is simply checked during buffer validation.
In my case I'm only interested in tables. Would a PR be accepted that adds support for tables only as a first step?

@aardappel
Copy link
Collaborator

@letmaik Support for fixed length arrays was recently merged: #5313

I think we can close this issue now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants