I also like the idea of marking some fields as required, also erring on the side of conservatism.
I can test on the Rust implementation (although IPC support is still incomplete) if we go ahead with this. On Fri, 17 Jan 2020 at 08:29, Micah Kornfield <emkornfi...@gmail.com> wrote: > I too, couldn't find anything that says this would break backwards > compatibility for the binary format. But it probably pays to open an issue > with the flatbuffer team just to be safe. > > Two points: > 1. I'd like to make sure we are conservative in choosing "definitely > required" > 2. Before committing to the change, it would be good to get a sense of how > much this affects other language bindings (e.g. scope of work). > 3. If we decide to this it seems like it should be a 1.0.0 blocker? > > On Thu, Jan 16, 2020 at 1:47 PM Wes McKinney <wesmck...@gmail.com> wrote: > > > If using "required" does not alter the Flatbuffers binary format (it > > doesn't seem that it does, it adds non-null assertions on the write > > path and additional checks in the read verifiers, is that accurate?), > > then it may be worthwhile to set it on "definitely required" fields so > > spare clients from having to implement their own null checks. Thoughts > > from others? > > > > - Wes > > > > On Thu, Jan 16, 2020 at 8:13 AM Antoine Pitrou <anto...@python.org> > wrote: > > > > > > > > > Hello, > > > > > > In Flatbuffers, all fields are optional by default. It means that the > > > reader can get NULL (in C++) for a missing field. In turn, this means > > > that message validation (at least in C++) should check all child table > > > fields for non-NULL. Not only is this burdensome, but it's easy to > miss > > > some checks. Currently, we don't seem to do any of them. > > > > > > Instead, it seems we could mark most child fields *required*. This > > > would allow the generated verifier to check that those fields are > indeed > > > non-NULL when reading. It would also potentially break compatibility, > > > though I'm not sure why (do people rely on the fields being missing > > > sometimes?). What do you think? > > > > > > > > > To quote the Flatbuffers documentation: > > > """ > > > By default, all fields are optional, i.e. may be left out. This is > > > desirable, as it helps with forwards/backwards compatibility, and > > > flexibility of data structures. It is also a burden on the reading > code, > > > since for non-scalar fields it requires you to check against NULL and > > > take appropriate action. By specifying this field, you force code that > > > constructs FlatBuffers to ensure this field is initialized, so the > > > reading code may access it directly, without checking for NULL. If the > > > constructing code does not initialize this field, they will get an > > > assert, and also the verifier will fail on buffers that have missing > > > required fields. > > > """ > > > https://google.github.io/flatbuffers/md__schemas.html > > > > > > Regards > > > > > > Antoine. > > >
