A cast is all that's need to make this work? Wow.

But yeah, that could break badly at some point.. Maybe we can throw a static_assert in there to check if both types are the same, and/or if certain members we are accessing are at the same offset? :)

@pavanbadugu @joseprupi @thomas-t1 @yaoshengzhe @aashray @inobelar @MarceColl from the other thread.

Do we not have to update the cpp_generator.cc file from the grpc release version source code to the flatbuffers repo?
That will automatically pull in the grpc++ --> grpcpp header changes as well as take care of any other incompatibilities.

It looks like though there will need to some some changes related to not using grpc::string in favor of std::string and a couple of minor fixes.
In the past that is how this compatibility has been maintained, I believe.
I have it somewhere in one of my local workspaces that I've been meaning to upload a PR for after clean up, but happy to help out on this PR too.

This has resolved the gRPC 1.35 integration issues in our systems! Thanks!

Travis CI failed because tests/monster_test.grpc.fb.cc was not updated in this PR, can you add that?

And the Bazel build fails with:

In file included from grpc/tests/message_builder_test.cpp:1:0:
  | bazel-out/k8-fastbuild/bin/_virtual_includes/flatbuffers/flatbuffers/grpc.h: In static member function 'static grpc::Status grpc::SerializationTraits<flatbuffers::grpc::Message<T> >::Deserialize(grpc::ByteBuffer*, flatbuffers::grpc::Message<T>*)':
  | bazel-out/k8-fastbuild/bin/_virtual_includes/flatbuffers/flatbuffers/grpc.h:294:36: error: request for member 'c_buffer' in 'buf', which is of pointer type 'grpc::ByteBuffer*' (maybe you meant to use '->' ?)
  | grpc_byte_buffer *buffer = buf.c_buffer();  // *reinterpret_cast<grpc_byte_buffer **>(buf);

@youknowone what is the status on this PR? I would love to have it be part of the following redesign #6443

@mustiikhalil This is not an actual PR. So maybe the status is invalid. This PR shows how to (unsafely) solve the problem. As described in the issue text, I wish someone actually can solve the problem better way in future.

I just wanted to share our experience in case someone is interested; We attempted to use gRPC with flatbuffers as a solution for RPC on resource restricted platforms/embedded, but it ultimately failed for a couple of reasons; You could argue that none of them has to do with flatbuffers, but in part has to do with the gRPC integration, so I'll write it up anyway;

• gRPC and flatbuffers simply aren't compatible (anymore), even though it is being claimed, and the gRPC team does not seem to care, so a safe fix seems unlikely in the short term
• gRPC and its channel concept is not very customizable and has a lot of high level constructs hard-coded (e.g. async processing/threading)

So in the end we decided to roll our own RPC mechanism with flatbuffers as binary serialization format, but inspired by the payload format of gRPC, so in addition to client/server code generation, it allowed us to build a gRPC to/from our-internal-RPC proxy that does not require knowledge of the schema/service definition.

@youknowone
Well how about the following, lets finalize the following PR. Since it's should be working with the internal components of grpc. I know its not the safest option, but rather a hot fix for now. My current view is that we should continue supporting this method until grpc breaks it. However, at the same time we also need to work on a solution that works with the current state of grpc (API design than zero-copy.), unfortunately that's where the library is heading towards and unless we want to engineer our own networking library, we have our hands tied.

so whenever a user is generating a cpp grpc file they would have the following options --cpp --grpc --zero-copy (Current method that might be deprecated by grpc) and --cpp --grpc full support to the current grpc api design.

@aardappel what do you think?

On a side note, I don't think we would be able to support the current gRPC apis unless someone is willing to work on it, if we want to have it before the flatbuffers 2.0 release. If not I can take a look at it but that might be in later this year

@mustiikhalil I am fine supporting this "hack" until grpc break it. My worry would be how we will found out if grpc break it, since it is just a cast, so it could be silent breakage that just corrupts data.

Also not sure if we need a flag, since who will still be using an old grpc that doesn't have this problem?

@aardappel what we can do is actually implement tests for this "hacky" solution for now.

The flag would be for the new cpp generated code that adheres to the their APIs. so what i mean is the following:

• --cpp --grpc would generate the new grpc APIs
• -cpp --grpc --zero-copy would generate the current hacky solution until it completely breaks
  but if this is a hassle we can simply just support the hacky solution until it breaks then move to the new apis

@aardappel Tagged you on grpc/grpc#20594 which I've been following up on regarding similar concerns.

Ok, looks like someone from gRPC will be looking at this in Q2.

Still suggest to move forward with workarounds in the meantime. Once there is a better path we can adopt it.

@mustiikhalil should we just merge this "hack" for now, and then you add your proposed flags after, or do you want to work on those first / in one PR?

On a side note, I don't think we would be able to support the current gRPC apis (new code generator) unless someone is willing to work on it, if we want to have it before the flatbuffers 2.0 release. If not I can take a look at it but that might be in later this year

The flag would be for the new cpp generated code that adheres to the their APIs. so what i mean is the following:
--cpp --grpc would generate the new grpc APIs
-cpp --grpc --zero-copy would generate the current hacky solution until it completely breaks
But if this is a hassle we can simply just support the hacky solution until it breaks then move to the new apis

The flags were supposed to be whenever we support the new API generation for grpc, so that won't be anytime soon. So for now, we have to settle with the hack.

Ok, looks like someone from gRPC will be looking at this in Q2.

And if there is someone who would be looking into it, I believe we should wait before generating the new grpc apis. since this "hacky" solution might be a permanent one

The PR is still failing, and it needs:

• Regenerating the code
• Formatting the code
• And not pointing to a method

In file included from tests/monster_test.grpc.fb.h:8:0,
--
  | from tests/monster_test.grpc.fb.cc:6:

pointer type 'grpc::ByteBuffer*' (maybe you meant to use '->' ?)
--
  | grpc_byte_buffer *buffer = buf.c_buffer();  // *reinterpret_cast<grpc_byte_buffer **>(buf);

if the author is not willing to continue to work on it, I can just recreate it and finalize the hacky solution @aardappel

ping @youknowone

Oh, I wasn't expecting this will be going to be merged as it is. If it will be, I will clean up and fix broken parts.

If possible can you rebase, so we can have the code generator github action and the new layout for the example code.

I followed up the thread and realized lots things happend recently. I'd better to look in this earlier. Sorry for a week absence.

@mustiikhalil Do you want me just rebase this PR? Unfortunately, I only used flatbuffers via vcpkg. I just tried to run build and test from flatbuffers project first time, but it doesn't look trivial for now. If you want a proper patch, I will look into it. But if you want just rebase, I think I can skip the step. (edited: nevermind, i misread the comment)

About the broken code, I made an accidental force-push. So first 2 comments are based on the other version.

The first version

--- a/include/flatbuffers/grpc.h
+++ b/include/flatbuffers/grpc.h
@@ -287,8 +287,9 @@ template<class T> class SerializationTraits<flatbuffers::grpc::Message<T>> {
   }
 
   // Deserialize by pulling the
-  static grpc::Status Deserialize(grpc_byte_buffer *buffer,
+  static grpc::Status Deserialize(ByteBuffer *buf,
                                   flatbuffers::grpc::Message<T> *msg) {
+    grpc_byte_buffer *buffer = *reinterpret_cast<grpc_byte_buffer**>(buf);
     if (!buffer) {
       return ::grpc::Status(::grpc::StatusCode::INTERNAL, "No payload");
     }

And I was on the way to make second version because this would be a bit better to detect future break change if it works. But never finished and never tested. Still I think this version also will eventually work, because they are intending the same thing.

--- a/include/flatbuffers/grpc.h
+++ b/include/flatbuffers/grpc.h
@@ -20,7 +20,9 @@
 // Helper functionality to glue FlatBuffers and GRPC.
 
 #include "flatbuffers/flatbuffers.h"
+#define private public
 #include "grpcpp/support/byte_buffer.h"
+#undef private
 #include "grpc/byte_buffer_reader.h"
 
 namespace flatbuffers {
@@ -287,8 +289,9 @@ template<class T> class SerializationTraits<flatbuffers::grpc::Message<T>> {
   }
 
   // Deserialize by pulling the
-  static grpc::Status Deserialize(grpc_byte_buffer *buffer,
+  static grpc::Status Deserialize(ByteBuffer *buf,
                                   flatbuffers::grpc::Message<T> *msg) {
+    grpc_byte_buffer *buffer = buf->c_buffer();
     if (!buffer) {
       return ::grpc::Status(::grpc::StatusCode::INTERNAL, "No payload");
     }

I think force-push happened before the major discussion, so most of comments are about the second version. But one of the participant @aardappel maybe is thinking about the first version. So to make clear, which one is preferred?

For the second version, I think it need some more compiler directive works to not to generate warning for the macro private.

The broken CI is mostly about upgrading grpc rather than the PR itself. For now, cmake+linux (finally! thanks to @Perlmint) passes but cmake+macos does not. Bazel build also does not. Unfortunately, I don't know well about either cmake or bazel, so it is not an easy challenge to me. Could anyone take this over for further work for build systems?

Finally, All checks are passed. I applied some workarounds for it.
Fixing boringssl build on macOS will be resolved appropriately later by updating gRPC with the newer version.
I used the messy way to update the cmake on macOS. It was the only working way I found. Updating cmake with brew is failed with the timeout, and Installing cmake via pip on the system is failed with a permission error. It was my best effort. So, If there is anyone who knows the clear way, Fix it, please.

@aardappel this PR seems to be done now, not sure about how the patch for the CI is done though

Ok, I will merge this for now, so we can move forward.

There are clearly some unfortunate things in here, but it be better to improve those in follow-ups.

Thanks so much @youknowone and @mustiikhalil for helping moving this situation forward!

@aardappel what type of cleanup/improvements that would be required for this?

@mustiikhalil maybe the 2 things I mention above.. and eventually, a more solid solution for the cast. But that can probably wait until we've had someone from the gRPC team look at this.

Thank you everyone opened to choose this option. Merging this one is totally unexpected event when I first made this PR.

I am happy to wait new flatbuffers work with latest gRPC again.