Vector of tables broken in 1.7.0

[greenrobot]

4355 seems to break vector of tables.

I have something like that:

table A {...}
table B {
    aVec: [A];
}

Using 1.7.0 (6f94fb51b14a1f9cc8b4fcc14744a6c334e644cb), aVec contained bad data (e.g. nullptr strings, 0 ints). The commit before optimizing vectors (f52f848b951f2e162b62b09e24e6e7cc5c82b0ba) works just fine.

[aardappel]

That would be serious. We have testing code that constructs a vector of tables though, that works fine (on 3 different compilers and platforms), Can you share the code that constructs aVec ? What compiler are you using?

[greenrobot]

@aardappel Can you point me to those tests? So you are verifying you get the data out you put in?

I saw this happening using clang++-3.8 on a 64 bit Linux machine.

Will try to collect some more data.

[aardappel]

Are you by chance serializing a vector<offset_t> as opposed to vector<Offset<..>> ?

This is an example of serializing a vector of tables (ignore the sorting, it still calls the regular CreateVector underneath): https://github.com/google/flatbuffers/blob/master/tests/test.cpp#L138 That then gets read and verified here: https://github.com/google/flatbuffers/blob/master/tests/test.cpp#L277

[aardappel]

You can test by just building the FlatBuffers tests. If that fails, we have a possible issue with the compiler. If it succeeds, there's something different about your code compared to our tests.

We test with clang, but on OS X.

[greenrobot]

That's about what my code is doing:

flatbuffers::Offset<Thing> singleOffset = thing->makeFlat(fbb);
auto vectorOffset = fbb.CreateVector<flatbuffers::Offset<Thing>>(&singleOffset, 1);
HasThingsBuilder builder(fbb); // the generated builder
builder.add_things(vectorOffset);

Is there something odd?

[greenrobot]

Also checked running your test, passes fine.

[aardappel]

Can you try sticking it in a vector<Offset<Thing>> and see if that fixes it? I wonder why it is picking the wrong template overload..

[aardappel]

Ok, reproduced it. If you change your call from fbb.CreateVector<flatbuffers::Offset<Thing>>(&singleOffset, 1) to fbb.CreateVector(&singleOffset, 1) the problem goes away.

The way I can explain this is that both overloads have the same template parameter, T, but are overloaded on the first argument. So without the template parameter, C++ picks the correct overload. But with the template parameter, you're saying, "I want T to set to Offset<Thing>", which means the first overload now matches also.

Now the big question is how common it is for people to explicitly specify the template parameter, since those people will get themselves into trouble with this overload.

@cberner who wrote this. We may need to a 1.7.1 bug-fix release.

[aardappel]

Not even sure how to change it such that this problem can't happen, apart from creating a differently named function.

[aardappel]

Ok, for now, added this assert to make sure that people that have code similar to yours get a compile time error: https://github.com/google/flatbuffers/commit/25a15950f5a24d7217689739ed8f6dac64912d62

[aardappel]

Made a 1.7.1 bug fix release just in-case: https://github.com/google/flatbuffers/releases