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

C++: mini_reflect: Add DefaultTypeTable #4614

Merged
merged 5 commits into from
Feb 15, 2018

Conversation

emaxerrno
Copy link
Contributor

Currently it's very easy to make a mistake when it comes to
instantiating the TypeTable to print a buffer because it is not type
safe.

This will allow us to write safer cpp code:

flatbuffers::FlatBufferToString(reinterpret_cast<const uint8_t *>(&t),
decltype(t)::DefaultTypeTable());

Currently it's very easy to make a mistake when it comes to
instantiating the TypeTable to print a buffer because it is not type
safe.

This will allow us to write safer cpp code:

flatbuffers::FlatBufferToString(reinterpret_cast<const uint8_t *>(&t),
                                decltype(t)::DefaultTypeTable());
@emaxerrno
Copy link
Contributor Author

@gwvo

I had a file that was like this:


ostream &
operator<<(ostream &o, const ::smf::wal::tx_put_fragment &f) {
  o << "tx_put_fragment="
    << flatbuffers::FlatBufferToString(reinterpret_cast<const uint8_t *>(&f),
                                       tx_put_fragmentTypeTable());
  return o;
}


ostream &
operator<<(ostream &o, const tx_get_request &r) {
  o << "tx_get_request="
    << flatbuffers::FlatBufferToString(reinterpret_cast<const uint8_t *>(&r),
                                       tx_get_requestTypeTable());
  return o;
}
ostream &
operator<<(ostream &o, const tx_get_reply &r) {
  o << "tx_get_reply="
    << flatbuffers::FlatBufferToString(reinterpret_cast<const uint8_t *>(&r),
                                       tx_get_replyTypeTable());
  return o;
}

...
...

I had a few core_dumps and then realize it was a typo on the name.

I can write a type safe quick wrapper with this change.

@emaxerrno
Copy link
Contributor Author

All the unit tests are passing now

test 1
Start 1: flattests

1: Test command: /home/agallego/workspace/flatbuffers/build/flattests
1: Test timeout computed to be: 9.99988e+06
1: ALL TESTS PASSED
1/1 Test #1: flattests ........................ Passed 0.06 sec

100% tests passed, 0 tests failed out of 1

Total Test time (real) = 0.06 sec

@emaxerrno
Copy link
Contributor Author

one can then create a simple helper like this

template< typename T>
struct fbs_print{
   fbs_print(const uint8_t *x):data(x){}

   inline flatbuffers::TypeTable* print() { return T::DefaultTypeTable(); }
   const uint8_t * data;
};

std::ostream operator<<(std::ostream &o, const fbs_print<Foo> &any{
   return o << any.print();
}

....

then you can it it on your code as 

char* buf = ... 

std::cout << fbs_print<MonsterType>(buf) << std::endl;

@@ -185,6 +191,9 @@ struct MonsterT : public flatbuffers::NativeTable {

struct Monster FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
typedef MonsterT NativeTableType;
static flatbuffers::TypeTable* DefaultTypeTable() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure the * goes next to the name. Also, Default doesn't sound quite right, since there are no alternatives. Just TypeTable would be too prone to clashes. Maybe.. ThisTypeTable ? GetTypeTable ? TheTypeTable ? Or go verbose and say MiniReflectTypeTable..?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, will submit fix. Thanks!

@emaxerrno
Copy link
Contributor Author

Sounds good, I fixed it, let me know if you want me to do any other edits.

@aardappel
Copy link
Collaborator

Thanks!

@aardappel aardappel merged commit 36f8564 into google:master Feb 15, 2018
zchee pushed a commit to zchee/flatbuffers that referenced this pull request Feb 14, 2019
* mini_reflect: Add DefaultTypeTable

Currently it's very easy to make a mistake when it comes to
instantiating the TypeTable to print a buffer because it is not type
safe.

This will allow us to write safer cpp code:

flatbuffers::FlatBufferToString(reinterpret_cast<const uint8_t *>(&t),
                                decltype(t)::DefaultTypeTable());

* c++: mini_reflect: update generated code

* Ensure types and names are set for mini_reflect

* c++: mini_refelct: update unit tests with new typed TypeTable

* Adding PR feedback of sylte and naming convention
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

Successfully merging this pull request may close these issues.

2 participants