native_type for tables in c++ object based api

[buster3]

Hi everybody, I like to use custom types for tables in the object based API. The behavior should be equal to the "native_type" attribute for structs. I hope to get some feedback on the idea and possible pack and unpack APIs.

Example:

table MatrixXd (native_type:"Eigen::MatrixXd") {
  rows:uint32;
  data:[double];
}

table Signal {
  mat:MatrixXd;
}

I like to get an object API similar to:

struct SignalT {
  Eigen::MatrixXd mat; // instead of std::unique_ptr<MatrixXdT>mat;
}

For structs the user has to specify the following functions:

namespace flatbuffers { 
native_type UnPack(const FlatbufferStruct& obj); 
FlatbufferStruct Pack(const native_type& obj); 
}

I think this is no suitable interface for tables, as "Pack" should directly map to a FlatBufferBuilder.

UnPack default is { auto _e = mat(); if (_e) _o->mat = std::unique_ptr<MatrixXdT>(_e->UnPack(_resolver)); }; The actual unpack functionality is implemented in "UnPackTo". Both UnPack and UnPackTo are members of the generated code and thus can not be user defined. I would suggest:

native_type UnPack(const FlatbufferTable& obj, const flatbuffers::resolver_function_t *_resolver = nullptr); or alternatively
void UnPackTo(native_type *_o, const FlatbufferTable& obj, const flatbuffers::resolver_function_t *_resolver = nullptr); I would prefer the first version. Return value optimization should do it's job.

Pack There is a "Pack" method generated for each Table. On the other hand there are two overloaded CreateType functions which do the actual work. I think the interface is similar to what we like to have.

Suggestion: inline flatbuffers::Offset<Signal> CreateSignal(flatbuffers::FlatBufferBuilder &_fbb, const native_type &_o, const flatbuffers::rehasher_function_t *_rehasher);

I think it is possible to clean up the various generated code pack and unpack functions. For compatibility reasons we should not do so? I am looking forward to some feedback.

Best regards Thomas

[aardappel]

I can see the usefulness, but this is definitely more complex than with structs, where it is a mere memory mapping.

Changing the signature of these functions at this point doesn't sound like a great idea, we'd break potentially a lot of code.

I understand you're not happy having UnPack return a Eigen::MatrixXd *, but that is the only thing that would be compatible with the current API. To declare it just Eigen::MatrixXd we'd also have to specify native_inline which is currently not supported for tables. So maybe we should look at that first.

[buster3]

First of all, yes, breaking existing code is not a good idea. Thanks for the hint with native_inline. I had a deeper look on the code and just tried to understand the intended behavior.

Not encoded scalar types are replaced by their default values on read. For the user it is an an implementation detail that default values of scalar types (int, enum) are not encoded and thus not transmitted. I think this is a good strategy. It is a bit weird to me that on the other hand, structs or tables have a state "not transmitted". Why not applying the default value strategy again? Is there any reason or is it simply the way it is?

Maybe I found a bug in the current implementation?

struct Foo (native_type:"MyFoo") {
  x:float;
}

table FooBar {
  f:Foo (native_inline);
  ff:Foo;
}

results in

struct FooBarT : public flatbuffers::NativeTable {
  MyFoo f;
  std::unique_ptr<MyFoo> ff;
}

inline void FooBar::UnPackTo(FooBarT *_o, const flatbuffers::resolver_function_t *_resolver) const {
  { auto _e = f(); if (_e) _o->f = flatbuffers::UnPack(*_e); };
  { auto _e = ff(); if (_e) _o->ff = flatbuffers::UnPack(*_e); };

The current struct unpack is not suitable for std::unique_ptr<MyFoo> case. I would appreciate if there is a solution without a heap allocation of new MyFoo in unpack function.

[buster3]

It seems to me that the native_type feature is completely broken.

I cannot define a Pack function without a fully defined type FlatbufferStruct.

FlatbufferStruct Pack(const native_type& obj);

It is not possible to include the generated header in the pack and unpack functions.

Can anybody show me an example code which uses the native_type feature?

[aardappel]

Default values for tables are more difficult since they are possibly large and recursive. Tables have a default value.. it is just null rather than empty object. It seemed to make sense at the time.

"The current struct unpack is not suitable for std::unique_ptr case" how so?

As for the Pack function, does this not work?

#include "my_pack_prototypes.h"
#include "generated.h"
// My pack implementations follows here.

[buster3]

• Ok, got it. Table values can be null.
• You are right. If i predefine prototypes Pack works. Additionally, I have to make a forward declaration for the generated classes. All in all it looks quite messy. Example:

#pragma once
#include <complex>

// forward declaration of generated classes
struct Complex;

namespace flatbuffers {
    Complex Pack(const std::complex<double>& obj);
    std::complex<double> UnPack(const Complex& obj);
}

#include "test_generated.h"

namespace flatbuffers {
    inline Complex Pack(const std::complex<double>& obj)
    {
        return Complex(obj.real(), obj.imag());
    }

    inline std::complex<double> UnPack(const Complex& obj)
    {
        return std::complex<double>(obj.i(), obj.q());
    }
}

It might have been a good idea to split the object base API in a separate header.

• I think the following definition does not produce compilable code. std::complex<double> can not be converted to std::unique_ptr<std::complex<double>>.

struct Complex (native_type:"std::complex<double>") {
  i:double;
  q:double;
}

table FooConfig {
  native_inline:Complex (native_inline);
  default:Complex;
}

Native_type for tables I make the following suggestion:

• Automatically native_inline all structs/tables with a native_type. Users who wants to have a std::unique_ptr<User_t> can define (native_type:"std::unique_ptr<User_t>").
• inline flatbuffers::Offset<Signal> Pack(flatbuffers::FlatBufferBuilder &_fbb, const native_type &_o, const flatbuffers::rehasher_function_t *_rehasher);
• native_type UnPack(const FlatbufferTable& obj, const flatbuffers::resolver_function_t *_resolver = nullptr);

[aardappel]

Yes, that is a bit clumsy. This feature hasn't seen a lot of testing I think. Glad that at least it can work.

I'm not sure about the generated code up into multiple headers, which would be a bit of invasive change at this point. Same with defaulting to native_inline. We'll need a solution that incrementally improves what is currently there.

[buster3]

I do not understand which code would be broken if we default native_inline. Currently, the signature of UnPack is defined as native_type UnPack(const FlatbufferStruct& obj);. This will not work without native_inline. UnPack is not defined to return 'std::unique_ptr<native_type>'.

I would work on a native_type for tables feature, but currently I have no idea which kind of Pack/Unpack interfaces to define.

[aardappel]

Yes, it may well be that so far all users using this functionality have always used both attributes, so defaulting to native_inline can't possibly break them. If that is indeed the case we can change this.

Of course saying native_inline + native_type:"std::unique_ptr<User_t>" is a bit weird, but ok.. hopefully it is not needed. I guess the whole point of a native type is using memory-compatible structs which you always want inline.

Do you want to make a PR for this?

[buster3]

Thanks for the feedback. I will try to work on this the next weeks

[iceboy233]

For native_inline for tables, we can use std::optional (port as flatbuffers::optional) if we want to keep the optional semantics.

[aardappel]

@iceb0y yes, that would be a nice option to have (since now the type can be inline), though we'd have to provide our own implementation which I am not a fan of.

[iceboy233]

There are two other approaches:

• Abseil convergence: Does flatbuffers minimum requirement met abseil's requirement? if so we can depend on abseil.
• Unpack null as empty in object API and pack empty to null. We have actually done this for string, so they are string and not optional<string>.

I met this problem because currently we are generating unique_ptr for nested tables, which makes the object non-copyable.

Also when we drop VC2010 support (see #4646), we can support this:

table Foo {
    a: string;
}

table Bar {
    b: int;
    c: Foo;
}

In C++:

BarT bar{1, {"a"}};

For longer term we may also want to add view-based object API, so that we can perform zero-copy deserialization and one-copy serialization in object API.

[aardappel]

We have zero dependencies so far, and I don't think we're about to take one on to include Abseil.

And an empty table often makes a lot less sense than an empty string. I think we'd want to keep the optionality explicit there, by default, and probably don't want to break the existing API.

[aardappel]

How would such a view based API be different from the base API we already have? To me, the biggest gain of the object API is to enable mutation with standard C++ types.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity for 1 year. It will be automatically closed if no further activity occurs. To keep it open, simply post a new comment. Maintainers will re-open on new activity. Thank you for your contributions.

[sketch34]

table RowData {
  id: uint16;
  pmask: uint32;
  payload: [uint8];
}

struct RowUpdate {
  rows: [RowData];
}

I don't want std::vector<std::unique_ptr<px::protocol::RowDataT>> rows{};, just a plain std::vector<px::protocol::RowDataT>.

But:

Adding (native_inline) --> flatc error: native_inline can only be defined on structs Making RowUpdate a struct --> flatc error: structs may contain only scalar or struct fields

Will using native_type allow me to have a regular std::vector<MyRowData>?

I think the "could be null" semantics justifying the use of std::unique_ptr elsewhere doesn't make so much sense for a vector of tables, since the vector itself can already express this semantic by simply not containing the element. At least I think this would hold true in the most common usages.

[aardappel]

@sketch34 yes, if you see the posts above, it was suggested this attribute could be made to work on tables, it just hasn't been implemented yet.. should be a fairly simple addition for anyone to take on.

[lp35]

Badly need this. Does anyone tried to implement it?

[CasperN]

I don't think so. You're welcome to contribute if you need it!

[meniku]

I would be interested in this as well