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

How to copy separate tables into table vector? #49

Open
AndrewJDR opened this issue Jun 15, 2017 · 34 comments
Open

How to copy separate tables into table vector? #49

AndrewJDR opened this issue Jun 15, 2017 · 34 comments

Comments

@AndrewJDR
Copy link
Contributor

AndrewJDR commented Jun 15, 2017

Let's say I have:

table SomeTable {
  id:uint;
  someStrings:[string];
  name:string;
  name1:string;
  name2:string;
  name3:string;
  name5:string;
  
  // Etc... Pretend this is a very large table with many, many members.
}

I have a whole bunch of these SomeTable flatbuffers saved out into separate binary files somewhere on my filesystem as root objects.

Now let's say that in my code, I want to create a new table comprised of a vector of those SomeTables flatbuffers:

table SomeTables {
  tables:[Sometable];
}

How can I build this SomeTables buffer as concisely as possible? That is, I'd very much prefer to avoid code like this (pseudocode):

ns(SomeTables_start_as_root(B));
ns(SomeTables_tables_start(B));
ns(SomeTables_tables_push_start(B));
for(thisTableFromMyFilesystem from someTablesOnMyFilesystem)
{
  ns(SomeTable_table_t) thisTable = ns(SomeTable_as_root(thisTableFromMyFilesystem));
  ns(SomeTables_tables_push_create(&b, flatbuffers_string_create_str(B, ns(SomeTable_name1(thisTable), ns(SomeTable_name2(thisTable), ...)); // <-- I have to update this line when the SomeTable schema changes.
}
ns(SomeTables_tables_push_end(B));
ns(SomeTables_tables_end(B));
etc...

In such an arrangement, I have to update the SomeTables_tables_push_create line when the SomeTable schema changes. I'd prefer a "Take this SomeTable_table_t, copy its contents into the flatbuffer, and push it onto this table vector" function.
Does such a thing exist? e.g. SomeTables_tables_push_create_from_table_t in this example?

ns(SomeTables_start_as_root(B));
ns(SomeTables_tables_start(B));
ns(SomeTables_tables_push_start(B));
for(thisFlatBufferFromMyFilesystem from someTablesOnMyFilesystem)
{
  ns(SomeTable_table_t) thisTable = ns(SomeTable_as_root(thisTableFromMyFilesystem));
  ns(SomeTables_tables_push_create_from_table_t(B, thisTable)); // With this, I don't have to worry about a long push_create() line where I pass in all the values...
}
ns(SomeTables_tables_push_end(B));
ns(SomeTables_tables_end(B));
etc...
@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 15, 2017

Hi Andrew, thanks for asking.

Unfortunately there is no such function although it would be good to have.

It does not exist because it requires code for recursively iterating over tables - a copied table can have a vector of other tables etc. and there could be two references to the same table during copy - if this is not handled, data could explode in size.

So it is non-trival to implement, but definitely possible - it would approximately follow the same logic as the generated verifier code and I am open to contributions if anyone feels up to the task.

@AndrewJDR
Copy link
Contributor Author

@mikkelfj Thanks for the quick reply! Taking on such a task is a bit more than I can manage at the moment, so I'll just rethink my overall approach and try avoid this problem altogether for now.

@AndrewJDR AndrewJDR changed the title How to copy separate tables into table vector? [question] How to copy separate tables into table vector? Jun 15, 2017
@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 15, 2017

I would suggest you implement your own type specific table clone function. It would return a reference to the table and you can push that reference onto a vector or store it as a member of another table. In monster_test you will find examples of how to juggle table creation with references in different ways.

You still need to adapt the function, but you only have to do it in one place per schema change. You can also avoid copying all fields, if some are irrelevant and not flagged as required.

If you then feel like you need a function as you suggest, you could easily create a non-changing push_clone on top of that.

This is what I had in mind when leaving out the table clone feature in the early design.

@AndrewJDR
Copy link
Contributor Author

Thanks. As a side note, is this even something that the flatbuffers C++ project supports to your knowledge? Just curious.

@mikkelfj
Copy link
Contributor

I'm not really sure - I believe the C++ version does have some advanced reflection logic that allows you to modify a buffer in temporary storage and rewrite it, and possibly also to copy from an external buffer - but it is probably not the fastest way to go about things.

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 15, 2017

You could also write your own purpose specific clone function generator in C using reflection without supporting all sorts of special cases. It could be based on https://github.com/dvidelabs/flatcc/tree/master/samples/reflection

@AndrewJDR
Copy link
Contributor Author

Another quick, slightly related question. It feels like it should be possible to clone an existing flatbuffers_string_vec_t from another buffer and add it to something, e.g. something like:

// let's say a flatbuffers_string_vec_t is in strVecT
ns(SomeTable_someStrings_add(flatbuffers_string_vec_clone(strVecT)));

But I've tried all different combinations of things and can't seem to get anything to work. Is this possible?

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 15, 2017

It appears not - though it could be implemented with a simple loop since the string_vec has a push_clone method.

If you look at flatbuffers_common_builder.h you'll find there is a vec_clone but no string_vec_clone:

#define __flatbuffers_build_vector(NS, N, T, S, A)
...
static inline N ## _vec_ref_t N ## _vec_clone(NS ## builder_t *B, N ##_vec_t vec)
...

#define __flatbuffers_build_string_vector_ops(NS, N)
...

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 15, 2017

Note: strictly speaking a string_vec_clone and string_vec_slice should handle multiple references to the same string, so a robust implementation is not that trivial to do. It would require a hash table, and probably an option to not check for duplicates for performance and memory concerns.

@AndrewJDR
Copy link
Contributor Author

AndrewJDR commented Jun 17, 2017

Thanks again for your pointers. I'd like to move back to the original question a bit...

Let's say I want to embed a bunch of separately saved flatbuffers each containing a SomeTable_table_t into a brand new flatbuffer that I'm building (literally, I just memcpy them in there, end-to-end), retrieve references to those tables each time I memcpy them in, then populate the SomeTables_tables list with references to each of the SomeTables that i memcpy'd in earlier.

Given each flatbuffer containing the SomeTable_table_t is self contained and will by definition contain everything else required by that SomeTable_table_t, why is what I describe not possible? Basically, I don't entirely understand the need for the table recursion as you described it, why can't I just blindly copy in these flatbuffers as "sub flatbuffers", then refer to them from later in the flatbuffer? Is this some limitation of the flatbuffer format that it doesn't allow references of this kind?

Thanks!

@AndrewJDR
Copy link
Contributor Author

As a side note, I actually tried to do something like what I just described using flatcc_builder_embed_buffer, but I didn't get far with that. I believe I'm misunderstanding something fundamental about the flatbuffer format, here.

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 17, 2017

This is complex question, but I'll try:

The short answer you probably seek is that if a table does not contain any offset types, you can copy the table into a new buffer iff you make sure to also copy the vtable. A string, a vector, and a table are all offset types meaning that these exists it different memory blocks in the existing buffer. You would then have to chase each offset and copy the data, then copy the table referencing those data and update the tables references to the copies. Note that these references are known as uoffset_t types and are NOT the reference type the builder works with. If the offset types do not contain further offset types, the recursion stops there, meaning a table does contain subtables or vectors of tables. But even here, you need to consider the possibility of the same string being referenced in two places and decide if you want to copy the string once, or just duplicate it naively. Updating offsets and vtable references and ensuring proper alignment in the resulting buffer is non-trivial - this is what the builder does for you.

If you want to get creative, you can use the untyped low-level flatcc_builder.h interface to build you own data with proper alignment iff you respect the schema the buffer is expected to have.

You also need to understand that the ref type in the builder is an relative offset to specific location in the new buffer being built. Even if the buffer does not (yet) physically exist as a single coherent block, the reference will be translated to a relative distance in the final resulting buffer. This final distance is an offset type. You (or the builder) cannot know the offset before you both know the location of the source location and destination location in the new buffer. References exists to work around this temporarily. When a vector is being built piecemeal, the references are pushed, and when the vector closes, all references are converted. Anything added to an open vector must already have been added to the buffer. (Edit: unlike C++ you can add things, such as a subtable or a string, while the vector is open, and then push the reference, but you can't have the reference to push before the object has been created. C++ requires you to create the object even before opening the vector, and then remember all references before creating the vector. The same applies to Go and other languages).

You cannot get cannot get a reference into another existing buffer, because it makes no sense as a concept. You also cannot easily access an offset from an existing buffer because it is not exposed - it is always converted into a pointer for easy access. Read only bavigation works internally by adding offsets to pointers (roughly).

So, you could theoretically copy a very simple table type, but then it is almost the same as struct type, except for schema evolution. So you might consider using structs instead, and structs can be cloned by the existing builder API.

None of this prevents copying tables from one buffer to another, there is just a lot of detail to manage.

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 17, 2017

Note on shared strings - which also apply to vectors and tables:

If a string is referenced in two different places in the same buffer, it will be stored as two different uoffset_t values that happen to reach the same location.

When building a new buffer, a string will get a single string_ref_t value. This reference can be used in several places and will later by converted to offsets, where the offset value depends on the location from which the string is being referenced.

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 17, 2017

As a side note, I actually tried to do something like what I just described using flatcc_builder_embed_buffer, but I didn't get far with that. I believe I'm misunderstanding something fundamental about the flatbuffer format, here.

I don't think that I suggested adding source tables as nested buffers, but if you wish to do that, that is a possibility. But then you must add the entire buffer, not a specific table. Also the generated API should have type specific support for this - you don't have to use the low-level builder API for that.

However, I don't think this is what you want to do.

You can read and copy a table from a source buffer, as follows:
For all fields of interest you can use the _is_present method on all scalar and struct types, and test for null on all offset types. If the this suggests the field is present, you add the fields value to the new buffer.

There are two ways to do this, either bottom up: first create all offset types and store the reference returned for each, then open the table and add / clone all scalar and struct fields, and also add the references previously stored.

You can also open the table first, then copy or create each field top-down using the old tables field as a source to the new field. A source must be a string pointer, a struct pointer, or a scalar value. You cannot use a pointer to a table or vector, except for scalar/struct vectors which you can use as source because a scalar/struct vector is a self-contained memory block. As discussed, other vectors could theoretically be cloned automatically as well, but there is no support for it.

@AndrewJDR
Copy link
Contributor Author

@mikkelfj Thanks for all your detailed help in this thread! I'm still absorbing/using flatbuffers and how I can best use them. I may end up implementing this at some point after all -- I'm not sure yet.

@mikkelfj
Copy link
Contributor

You are welcome - as I said, it is a complex topic you picked :)

@mikkelfj
Copy link
Contributor

mikkelfj commented Jul 1, 2017

For reference, the limitation is/was actually documented here: https://github.com/dvidelabs/flatcc/blob/master/doc/builder.md#limitations

@AndrewJDR
Copy link
Contributor Author

Just for the record, I did end up using nested flatbuffers for this situation and it is doing the job. Using the _nest() call as well as the inline call method of creating nested flatbuffers in flatcc is quite convenient to use.

@mikkelfj
Copy link
Contributor

mikkelfj commented Jul 15, 2017

Also for the record:

Copying tables is (even) more difficult than I originally thought:

A table start is only aligned to a uoffset size (4 bytes) while individual members can be aligned to a higher value. Thus copying a table might skew alignment of a member field. To handle this each field must be checked to see if they are aligned properly in the target location or the table must be rewritten. For most common cases there are 3 situations:

  • no field aligned to more than 4 bytes, AOK.
  • some fields are 8 byte aligned and the target location matches alignment
  • some fields are 8 byte aligned and the target location must be adjusted 4 bytes to avoid rewriting

And two abnormal situation: the field has members forced to a higher alignment and

  • none of them are present and the above applies, or
  • some are present and the table must be rewritten or placed according to the highest aligned field.

In praxis this would be a get_actual_alignment function that returns the tables alignment requirements, and one source address that has this alignment. This can be used to compute how to place the target table in a new buffer. The get_actual_alignment function would be generated and must often just return 4 and table start.

@mikkelfj
Copy link
Contributor

@AndrewJDR Cloning of tables, vectors, unions and union vectors is now supported on master branch.
There is also pick operation that clones a single field from on table to another of same type. There is a new refmap object to deal with DAG identities.

@AndrewJDR
Copy link
Contributor Author

AndrewJDR commented Apr 9, 2018

@mikkelfj This is great. Thanks!

I had one question. If I'm using a table clone operation such as:

ns(Envelope_msg_Payload_clone(&builder, ns(someOtherPayload)));

Does that do the equivalent of calling *_Payload_start() / *_Payload_end() on the newly cloned table, or is the table left open for me to add some fields that perhaps were not added by the clone operation (for lack of those fields being set in the table being cloned from)? In other words, would it ever make sense to do this?

ns(Envelope_msg_Payload_clone(&builder, ns(someOtherPayload)));
ns(Payload_anotherField_create(&builder, 42)); // We happen to know that someOtherPayload did not set the anotherField field, so we'll set it ourselves here.
ns(Envelope_msg_Payload_end(&builder));

My current assumption is that this will not work because the cloned table is already closed by the _clone operation. So I do something like this instead:

ns(Envelope_msg_Payload_start(&builder));
ns(Payload_fieldA_pick(&builder, ns(someOtherPayload)));
ns(Payload_fieldB_pick(&builder, ns(someOtherPayload)));
ns(Payload_fieldC_pick(&builder, ns(someOtherPayload)));
ns(Payload_anotherField_create(&builder, 42));
ns(Envelope_msg_Payload_end(&builder));

Reason I'm asking:
Obviously the first form is less code, and is also more adaptable to future fields added to the Payload table schema, since those will just get copied and I don't have to add a new _pick() call for them.

But this leads me to a side question... if you set a field twice, what happens? I.e.

ns(Envelope_msg_Payload_start(&builder));
ns(Payload_anotherField_create(&builder, 42));
ns(Payload_anotherField_create(&builder, 43));
ns(Envelope_msg_Payload_end(&builder));

Does 43 overwrite 42 here?

And so with this situation (assuming it was even possible -- i'm going to postfix the call with 'start_clone' just for clarity):

ns(Envelope_msg_Payload_start_clone(&builder, ns(someOtherPayload))); // anotherField is set to "41" in someOtherPayload
ns(Payload_anotherField_create(&builder, 42)); // Would this overwrite 41?
ns(Envelope_msg_Payload_end(&builder));

(Questions inline with the code above)

@AndrewJDR
Copy link
Contributor Author

(Just updated my last post with more questions/details)

@AndrewJDR
Copy link
Contributor Author

AndrewJDR commented Apr 9, 2018

It basically boils down to:
is the current table clone function equivalent to _start_and_end_clone() or just _start_clone()?
If it's equivalent to _start_and_end_clone(), should there be _start_clone()-like table clone feature to be able to do things like what I described above?

@mikkelfj
Copy link
Contributor

mikkelfj commented Apr 9, 2018

Hi Andrew, these are good questions.

I don't think the operation you request is currently avaiable. I chose to focus on pick and clone which starts and ends the target table.

I did think a fair bit about the leaving table open, but did not prioritize it thinking you can easily pick subfields manually if you need to manipulate the table anyway, and also that you could easily copy/paste from the generated clone source - have a look if you haven't already - only the refmap thing is a bit intriguing. I also needed to cut down on the work effort to implement this - there are many small intertwined macros.

However, it should be fairly straightforward to do now that the ground work has been done, and the names can followed in the source, so if you are brave, you could attempt a PR on this.

On the overwriting fields: this is another reason I chose ot not address this problem. The behavior is the that the low-level flatcc_builder.h interface will reject the conflict on purpose. It is quite possible to avoid this, but you a) need to design an interface for this, and b) accept that you might be dumping garbage into the buffer that you are building.

But, if you accept that conflicts are errors, then it is not difficult to manage.

What would be a good name for such an operation?

@mikkelfj
Copy link
Contributor

mikkelfj commented Apr 9, 2018

If such a new operation were to be added, it would probably not include a start. When you reject conficts it doesn't matter, but if want to add if not already present, you want to do it on an open table.

This kind of conflicting handling would actually be relatively easy to handle - it can test before picking and avoid leaving garbage. flatcc_builder.h has a test to check if a field was already set while a table is still open.

@mikkelfj
Copy link
Contributor

mikkelfj commented Apr 9, 2018

On you conflict example with create calls: note that it first creates, then adds. This means garbage in the buffer before the high level operation returns an error (either non zero int, or a null ref - I don't recall). It is safe to do if you ignore the error and accept garbage in the buffer - but debug builds will likely assert if you do not flag out those assertions (some tests do that).

@AndrewJDR
Copy link
Contributor Author

AndrewJDR commented Apr 10, 2018

I may give it a shot if I'm feeling brave some day :)

Another way of thinking about the syntax:

ns(Msg_start_as_root(&builder));
ns(Msg_pick_all(&builder, ns(otherMsg)));
ns(Msg_theAnswer_create(&builder, 42));
ns(Msg_end_as_root(&builder));

This maybe fits in better with the current _clone and _pick concepts you have in place, but I'm not sure. Thoughts?

On the overwriting fields: this is another reason I chose ot not address this problem. The behavior is the that the low-level flatcc_builder.h interface will reject the conflict on purpose. It is quite possible to avoid this, but you a) need to design an interface for this, and b) accept that you might be dumping garbage into the buffer that you are building.

For my use-case, I'm not interested in overwriting existing fields... only defining fields that were not already defined. For example:

ns(Msg_start_as_root(&builder));
ns(Msg_pick_all(&builder, ns(otherMsg)));
ns(Msg_theAnswer_create(&builder, 42)); // I know for certain (because of the way my code is written) that  theAnswer was not defined in otherMsg.
ns(Msg_end_as_root(&builder));

In fact, for my purposes, I would probably prefer something like:

ns(Msg_start_as_root(&builder));
ns(Msg_pick_all(&builder, ns(otherMsg)));
MY_ASSERT( !ns(Msg_theAnswer_exists() ); // Checks if theAnswer is a defined field in the currently open table
ns(Msg_theAnswer_create(&builder, 42)); // I know for certain (because of the way my code is written) that  theAnswer was not defined in otherMsg.
ns(Msg_end_as_root(&builder));

Because I consider otherMsg defining the theAnswer field to be an error condition, and would like to know about that if it happens... does anything like Msg_theAnswer_exists() exist in flatcc currently?

@AndrewJDR
Copy link
Contributor Author

In summary, I'm mostly looking to define fields that aren't already defined on the open table. Ability to actually overwrite defined fields on the open table is maybe interesting, but goes above and beyond what I'm really looking for here, so unless it's easy to implement I probably wouldn't go that far with it. It also feels like it goes against the design of flatbuffers/flatcc, which is that you're working with the output buffer in a more-or-less in-place fashion. With a field ovewriting feature, we've either got to rewind and deal with the previously written field in some way (probably by rewriting the buffer which I imagine gets messy), or be okay with garbage in the buffer. Eh, I'd rather not fight that part of the design.

@mikkelfj
Copy link
Contributor

Just focusing on this one question here:

does anything like Msg_theAnswer_exists() exist in flatcc currently?

In the generated _common_reader.h there is an _is_present() method, but at the typed (generated) level there is no such thing for the builder. You generally do want to use the untyped builder interface for adding new features. If you go ahead with a PR you could also add an _is_present for the builder. (Maybe exists is better, but history ...).

I wrote earlier that the low-level builder has a method for this, but I couldn't actually find it - it is in a private branch for an unpublished feature

...
void *flatcc_builder_table_add(flatcc_builder_t *B, int id, size_t size, uint16_t align);

/*
 * Returns 1 if a table field was already added for the given id
 * and 0 otherwise. Only valid to call while the table is open,
 * and the id must be within the valid range.
 */
int flatcc_builder_table_is_present(flatcc_builder_t *B, int id);

/**
 * Returns a pointer to the buffer holding the last field added. The
 * size argument must match the field size added. May, for example, be
 * used to perform endian conversion after initially updating field
 * as a native struct. Must be called before the table is ended.
 */
void *flatcc_builder_table_edit(flatcc_builder_t *B, size_t size);
...

Implemented as

int flatcc_builder_table_is_present(flatcc_builder_t *B, int id)
{
    check(frame(type) == flatcc_builder_table, "expected table frame");
    check(id >= 0 && id <= (int)FLATBUFFERS_ID_MAX, "table id out of range");

    return (B->vs[id] != 0);
}

it is quite similar to flatcc_builder_check_required_field but semantics are different and this might affect future logging and asertions.

Also note the builder flag that hardcodes to ignore duplicate added fields - it is likely not the solution but worth knowing about: FLATCC_BUILDER_ALLOW_REPEAT_TABLE_ADD.

@mikkelfj
Copy link
Contributor

In summary, I'm mostly looking to define fields that aren't already defined on the open table. Ability to actually overwrite defined fields on the open table is maybe interesting, but goes above and beyond what I'm really looking for here, so unless it's easy to implement I probably wouldn't go that far with it. It also feels like it goes against the design of flatbuffers/flatcc, which is that you're working with the output buffer in a more-or-less in-place fashion. With a field ovewriting feature, we've either got to rewind and deal with the previously written field in some way (probably by rewriting the buffer which I imagine gets messy), or be okay with garbage in the buffer. Eh, I'd rather not fight that part of the design.

A agree with this. As to being able to overwrite a field, this is extremely easy a the the flacc_builder_ api level, you just copy the table_add method and enable the flagged out FLATCC_BUILDER_ALLOW_REPEAT_TABLE_ADD
which results in even less code than before.

But then you have to deal with buffer garbage - cleanup is absolute not possible becuase the content might be cross-atlantic by the time we get to that point - the emitter is streaming. Post cleanup is easy with the new clone facility. You just clone the entire buffer - and if you don't care about identity, you can the refmap to have at most one copy.

Worse, the user facing api needs to duplicate add methods - which is quite messy - or alternatively set a temporary runtime flag - which is costly to check and easy to get wrong. Actually, the refmap used with cloning does something similar - you can decide if you want to reuse a clone or not - but this is also marketed as an advanced don't do this at home feature.

@mikkelfj
Copy link
Contributor

Now, to address you main questions:

It would be greate if would consider a PR - I believe we are on the same page.
There are two features of concern: builder version of _is_present and _pick_all aka _clone_into aka some_other_name.

quoting

ns(Msg_start_as_root(&builder));
ns(Msg_pick_all(&builder, ns(otherMsg)));
ns(Msg_theAnswer_create(&builder, 42));
ns(Msg_end_as_root(&builder));

This maybe fits in better with the current _clone and _pick concepts you have in place, but I'm not sure. Thoughts?

Yes, this is what I in mind, except for the name pick_all. The problem with that name is that pick takes a table type T and returns another table of type T, so in a sense it already picks all, but you using pick at the wrong semantic level. You can't a member method like Msg_aComment_pick_all(b, t), but you can have Msg_aComment_pick(b, t).

I'm not sure I really have the overview to give the right answer here right now - but the point is that you need to think about the different ways the operation can be used - as a member in a table or standalone (clone can be standalone returning a ref, but your pick_all likely can't).

clone_into might be a more appropriate name, but this needs more thinking.

The actual behavior of the method, OTOH, is quite clear:

do a clone into an open table similar to be repeatedly picking fields that are known to not be present in the target table and do not close the table subsequently.

You also need to consider what happens at the root level - e.g. you can clone a buffer with table, but does clone_into make any sense here? Likely not. What about nested buffers - likely not.

What about structs - there is no presence check - should it just overwrite any null fields? (This problem was also a reason to leave out this feature). Structs already have assign methods and these somewhat map to clone - but it depends on whether the struct is standalone is in roots and unions, or inlined as in tables and vector elements.

There is also the separate operation of start_as_clone or start_with which I do not suggest implementing - but if done, it needs to address all of the above concerns, but avoids presence checks.

As to your suggested assertion:

ns(Msg_start_as_root(&builder));
ns(Msg_pick_all(&builder, ns(otherMsg)));
MY_ASSERT( !ns(Msg_theAnswer_exists() ); // Checks if theAnswer is a defined field in the currently open table
ns(Msg_theAnswer_create(&builder, 42)); // I know for certain (because of the way my code is written) that  theAnswer was not defined in otherMsg.
ns(Msg_end_as_root(&builder));

The builder call flatcc_builder_table_add already has a check similar to MY_ASSERT. The check can be flagged in and out. See src/runtimer/builder.c - maybe it should have been a check_error call instead.

@mikkelfj
Copy link
Contributor

You can't a member method like Msg_aComment_pick_all(b, t), but you can have Msg_aComment_pick(b, t).

Actually, you have such an operation assuming you had called Msg_aComment_start() first, but the name would still be confusing.

@mikkelfj
Copy link
Contributor

Reopening issue.

@mikkelfj mikkelfj reopened this Apr 10, 2018
@AndrewJDR
Copy link
Contributor Author

do a clone into an open table similar to be repeatedly picking fields that are known to not be present in the target table and do not close the table subsequently.

This "known to not be present in the target table" is a good point. This way, if one wants to override a value that might be present in the source table, one need only define that field prior to the clone_into operation (rather than afterward).

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

No branches or pull requests

2 participants