Add move semantics to MessageBuilder, FlatBufferBuilder, SliceAllocator, and…

Add move semantics to MessageBuilder, FlatBufferBuilder, SliceAllocator, and vector_downward (#4893)

Unit tests
Update flatbuffers + gRPC build instructions
Update CMakeLists.txt with cmake variables for grpc and protobuf install paths
Update tests for travis build

CMakeLists.txt

@@ -101,6 +101,7 @@ set(FlatBuffers_GRPCTest_SRCS
   tests/monster_test.grpc.fb.h
   tests/monster_test.grpc.fb.cc
   grpc/tests/grpctest.cpp
+  grpc/tests/message_builder_test.cpp
   # file generated by running compiler on samples/monster.fbs
   ${CMAKE_CURRENT_BINARY_DIR}/samples/monster_generated.h
 )
@@ -257,6 +258,15 @@ if(FLATBUFFERS_BUILD_GRPCTEST)
   if(CMAKE_COMPILER_IS_GNUCXX)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-parameter -Wno-shadow")
   endif()
+  if(NOT GRPC_INSTALL_PATH)
+    message(SEND_ERROR "GRPC_INSTALL_PATH variable is not defined. See grpc/README.md")
+  endif()
+  if(NOT PROTOBUF_DOWNLOAD_PATH)
+    message(SEND_ERROR "PROTOBUF_DOWNLOAD_PATH variable is not defined. See grpc/README.md")
+  endif()
+  INCLUDE_DIRECTORIES(${GRPC_INSTALL_PATH}/include)
+  INCLUDE_DIRECTORIES(${PROTOBUF_DOWNLOAD_PATH}/src)
+  LINK_DIRECTORIES(${GRPC_INSTALL_PATH}/lib)
   add_executable(grpctest ${FlatBuffers_GRPCTest_SRCS})
   target_link_libraries(grpctest grpc++_unsecure grpc_unsecure gpr pthread dl)
 endif()
@@ -346,6 +356,9 @@ if(FLATBUFFERS_BUILD_TESTS)
   file(COPY "${CMAKE_CURRENT_SOURCE_DIR}/tests" DESTINATION
        "${CMAKE_CURRENT_BINARY_DIR}")
   add_test(NAME flattests COMMAND flattests)
+  if(FLATBUFFERS_BUILD_GRPCTEST)
+    add_test(NAME grpctest COMMAND grpctest)
+  endif()
 endif()
 
 include(CMake/BuildFlatBuffers.cmake)

grpc/README.md

@@ -9,3 +9,23 @@ from GRPC, and work with both the Protobuf and FlatBuffers code generator.
 the GRPC libraries for this to compile. This test will build using the
 `FLATBUFFERS_BUILD_GRPCTEST` option to the main FlatBuffers CMake project.
 
+## Building Flatbuffers with gRPC
+
+### Linux
+
+1. Download, build and install gRPC. See [instructions](https://github.com/grpc/grpc/tree/master/src/cpp).
+    * Lets say your gRPC clone is at `/your/path/to/grpc_repo`.
+    * Install gRPC in a custom directory by running `make install prefix=/your/path/to/grpc_repo/install`.
+2. `export GRPC_INSTALL_PATH=/your/path/to/grpc_repo/install`
+3. `export PROTOBUF_DOWNLOAD_PATH=/your/path/to/grpc_repo/third_party/protobuf`
+4. `mkdir build ; cd build`
+5. `cmake -DFLATBUFFERS_BUILD_GRPCTEST=ON -DGRPC_INSTALL_PATH=${GRPC_INSTALL_PATH} -DPROTOBUF_DOWNLOAD_PATH=${PROTOBUF_DOWNLOAD_PATH} ..`
+6. `make`
+
+## Running FlatBuffer gRPC tests
+
+### Linux
+
+1. `ln -s ${GRPC_INSTALL_PATH}/lib/libgrpc++_unsecure.so.6 ${GRPC_INSTALL_PATH}/lib/libgrpc++_unsecure.so.1`
+2. `export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:${GRPC_INSTALL_PATH}/lib`
+3. `make test ARGS=-V` 

grpc/tests/grpctest.cpp

@@ -22,6 +22,7 @@
 #include "monster_test_generated.h"
 
 using namespace MyGame::Example;
+int builder_tests();
 
 // The callback implementation of our server, that derives from the generated
 // code. It implements all rpcs specified in the FlatBuffers schema.
@@ -92,7 +93,7 @@ void RunServer() {
   server_instance->Wait();
 }
 
-int main(int /*argc*/, const char * /*argv*/ []) {
+int grpc_server_test() {
   // Launch server.
   std::thread server_thread(RunServer);
 
@@ -163,3 +164,8 @@ int main(int /*argc*/, const char * /*argv*/ []) {
 
   return 0;
 }
+
+int main(int /*argc*/, const char * /*argv*/ []) {
+  return builder_tests() + grpc_server_test();
+}
+

grpc/tests/message_builder_test.cpp

+#include "flatbuffers/grpc.h"
+#include "monster_test_generated.h"
+
+static int builder_test_error = 0;
+
+#define test_assert(condition) do { \
+  if(!(condition)) { \
+    fprintf(stderr, "%s:%d: %s failed.\n", __FILE__, __LINE__, #condition);\
+    builder_test_error = 1;\
+  } \
+} while(0)
+
+using namespace MyGame::Example;
+
+const std::string m1_name = "Cyberdemon";
+const Color m1_color = Color_Red;
+const std::string m2_name = "Imp";
+const Color m2_color = Color_Green;
+
+flatbuffers::Offset<Monster> populate1(flatbuffers::FlatBufferBuilder &builder) {
+  auto name_offset = builder.CreateString(m1_name);
+  return CreateMonster(builder, nullptr, 0, 0, name_offset, 0, m1_color);
+}
+
+flatbuffers::Offset<Monster> populate2(flatbuffers::FlatBufferBuilder &builder) {
+  auto name_offset = builder.CreateString(m2_name);
+  return CreateMonster(builder, nullptr, 0, 0, name_offset, 0, m2_color);
+}
+
+bool release_n_verify(flatbuffers::FlatBufferBuilder &fbb, const std::string &expected_name, Color color) {
+  flatbuffers::DetachedBuffer buf = fbb.Release();
+  const Monster *monster = flatbuffers::GetRoot<Monster>(buf.data());
+  return (monster->name()->str() == expected_name) && (monster->color() == color);
+}
+
+bool release_n_verify(flatbuffers::grpc::MessageBuilder &mbb, const std::string &expected_name, Color color) {
+  flatbuffers::grpc::Message<Monster> msg = mbb.ReleaseMessage<Monster>();
+  const Monster *monster = msg.GetRoot();
+  return (monster->name()->str() == expected_name) && (monster->color() == color);
+}
+
+struct OwnedAllocator : public flatbuffers::DefaultAllocator {};
+
+struct TestHeapMessageBuilder : public flatbuffers::FlatBufferBuilder {
+  TestHeapMessageBuilder()
+    : flatbuffers::FlatBufferBuilder(2048, new OwnedAllocator(), true) {}
+};
+
+template <class Builder>
+struct BuilderTests {
+  static void empty_builder_movector_test() {
+    Builder b1;
+    size_t b1_size = b1.GetSize();
+    Builder b2(std::move(b1));
+    size_t b2_size = b2.GetSize();
+    test_assert(b1_size == 0);
+    test_assert(b1_size == b2_size);
+  }
+
+  static void nonempty_builder_movector_test() {
+    Builder b1;
+    populate1(b1);
+    size_t b1_size = b1.GetSize();
+    Builder b2(std::move(b1));
+    test_assert(b1_size == b2.GetSize());
+    test_assert(0 == b1.GetSize());
+  }
+
+  static void builder_movector_before_finish_test() {
+    Builder b1;
+    auto root_offset1 = populate1(b1);
+    Builder b2(std::move(b1));
+    b2.Finish(root_offset1);
+    test_assert(release_n_verify(b2, m1_name, m1_color));
+    test_assert(0 == b1.GetSize());
+  }
+
+  static void builder_movector_after_finish_test() {
+    Builder b1;
+    auto root_offset1 = populate1(b1);
+    b1.Finish(root_offset1);
+    Builder b2(std::move(b1));
+    test_assert(release_n_verify(b2, m1_name, m1_color));
+    test_assert(0 == b1.GetSize());
+  }
+
+  static void builder_move_assign_before_finish_test() {
+    Builder b1;
+    auto root_offset1 = populate1(b1);
+    Builder b2;
+    populate2(b2);
+    b2 = std::move(b1);
+    b2.Finish(root_offset1);
+    test_assert(release_n_verify(b2, m1_name, m1_color));
+    test_assert(0 == b1.GetSize());
+  }
+
+  static void builder_move_assign_after_finish_test() {
+    Builder b1;
+    auto root_offset1 = populate1(b1);
+    b1.Finish(root_offset1);
+    Builder b2;
+    auto root_offset2 = populate2(b2);
+    b2.Finish(root_offset2);
+    b2 = std::move(b1);
+    test_assert(release_n_verify(b2, m1_name, m1_color));
+    test_assert(0 == b1.GetSize());
+  }
+
+  static void builder_swap_before_finish_test() {
+    Builder b1;
+    auto root_offset1 = populate1(b1);
+    auto size1 = b1.GetSize();
+    Builder b2;
+    auto root_offset2 = populate2(b2);
+    auto size2 = b2.GetSize();
+    b1.Swap(b2);
+    b1.Finish(root_offset2);
+    b2.Finish(root_offset1);
+    test_assert(b1.GetSize() > size2);
+    test_assert(b2.GetSize() > size1);
+    test_assert(release_n_verify(b1, m2_name, m2_color));
+    test_assert(release_n_verify(b2, m1_name, m1_color));
+  }
+
+  static void builder_swap_after_finish_test() {
+    Builder b1;
+    auto root_offset1 = populate1(b1);
+    b1.Finish(root_offset1);
+    auto size1 = b1.GetSize();
+    Builder b2;
+    auto root_offset2 = populate2(b2);
+    b2.Finish(root_offset2);
+    auto size2 = b2.GetSize();
+    b1.Swap(b2);
+    test_assert(b1.GetSize() == size2);
+    test_assert(b2.GetSize() == size1);
+    test_assert(release_n_verify(b1, m2_name, m2_color));
+    test_assert(release_n_verify(b2, m1_name, m1_color));
+  }
+
+  static void all_tests() {
+    empty_builder_movector_test();
+    nonempty_builder_movector_test();
+    builder_movector_before_finish_test();
+    builder_movector_after_finish_test();
+    builder_move_assign_before_finish_test();
+    builder_move_assign_after_finish_test();
+    builder_swap_before_finish_test();
+    builder_swap_after_finish_test();
+  }
+};
+
+int builder_tests() {
+  BuilderTests<flatbuffers::grpc::MessageBuilder>::all_tests();
+  BuilderTests<flatbuffers::FlatBufferBuilder>::all_tests();
+  BuilderTests<TestHeapMessageBuilder>::all_tests();
+  return builder_test_error;
+}

include/flatbuffers/flatbuffers.h

@@ -572,6 +572,32 @@ class vector_downward {
         cur_(nullptr),
         scratch_(nullptr) {}
 
+  vector_downward(vector_downward &&other)
+    : allocator_(other.allocator_),
+      own_allocator_(other.own_allocator_),
+      initial_size_(other.initial_size_),
+      buffer_minalign_(other.buffer_minalign_),
+      reserved_(other.reserved_),
+      buf_(other.buf_),
+      cur_(other.cur_),
+      scratch_(other.scratch_) {
+    other.allocator_ = nullptr;
+    other.own_allocator_ = false;
+    // No change in other.initial_size_
+    // No change in other.buffer_minalign_
+    other.reserved_ = 0;
+    other.buf_ = nullptr;
+    other.cur_ = nullptr;
+    other.scratch_ = nullptr;
+  }
+
+  vector_downward &operator=(vector_downward &&other) {
+    // Move construct a temporary and swap idiom
+    vector_downward temp(std::move(other));
+    swap(temp);
+    return *this;
+  }
+
   ~vector_downward() {
     clear_buffer();
     clear_allocator();
@@ -706,6 +732,24 @@ class vector_downward {
   void pop(size_t bytes_to_remove) { cur_ += bytes_to_remove; }
   void scratch_pop(size_t bytes_to_remove) { scratch_ -= bytes_to_remove; }
 
+  void swap(vector_downward &other) {
+    using std::swap;
+    swap(allocator_, other.allocator_);
+    swap(own_allocator_, other.own_allocator_);
+    swap(initial_size_, other.initial_size_);
+    swap(buffer_minalign_, other.buffer_minalign_);
+    swap(reserved_, other.reserved_);
+    swap(buf_, other.buf_);
+    swap(cur_, other.cur_);
+    swap(scratch_, other.scratch_);
+  }
+
+  void swap_allocator(vector_downward &other) {
+    using std::swap;
+    swap(allocator_, other.allocator_);
+    swap(own_allocator_, other.own_allocator_);
+  }
+
  private:
   // You shouldn't really be copying instances of this class.
   FLATBUFFERS_DELETE_FUNC(vector_downward(const vector_downward &))
@@ -794,6 +838,44 @@ class FlatBufferBuilder {
     EndianCheck();
   }
 
+  /// @brief Move constructor for FlatBufferBuilder.
+  FlatBufferBuilder(FlatBufferBuilder &&other)
+    : buf_(1024, nullptr, false, AlignOf<largest_scalar_t>()),
+      num_field_loc(0),
+      max_voffset_(0),
+      nested(false),
+      finished(false),
+      minalign_(1),
+      force_defaults_(false),
+      dedup_vtables_(true),
+      string_pool(nullptr) {
+    EndianCheck();
+    // Default construct and swap idiom.
+    // Lack of delegating constructors in vs2010 makes it more verbose than needed.
+    Swap(other);
+  }
+
+  /// @brief Move assignment operator for FlatBufferBuilder.
+  FlatBufferBuilder &operator=(FlatBufferBuilder &&other) {
+    // Move construct a temporary and swap idiom
+    FlatBufferBuilder temp(std::move(other));
+    Swap(temp);
+    return *this;
+  }
+
+  void Swap(FlatBufferBuilder &other) {
+    using std::swap;
+    buf_.swap(other.buf_);
+    swap(num_field_loc, other.num_field_loc);
+    swap(max_voffset_, other.max_voffset_);
+    swap(nested, other.nested);
+    swap(finished, other.finished);
+    swap(minalign_, other.minalign_);
+    swap(force_defaults_, other.force_defaults_);
+    swap(dedup_vtables_, other.dedup_vtables_);
+    swap(string_pool, other.string_pool);
+  }
+
   ~FlatBufferBuilder() {
     if (string_pool) delete string_pool;
   }

include/flatbuffers/grpc.h

@@ -88,6 +88,22 @@ class SliceAllocator : public Allocator {
   SliceAllocator(const SliceAllocator &other) = delete;
   SliceAllocator &operator=(const SliceAllocator &other) = delete;
 
+  SliceAllocator(SliceAllocator &&other)
+    : slice_(other.slice_) {
+    other.slice_ = grpc_empty_slice();
+  }
+
+  SliceAllocator &operator=(SliceAllocator &&other) {
+    slice_ = other.slice_;
+    other.slice_ = grpc_empty_slice();
+    return *this;
+  }
+
+  void swap(SliceAllocator &other) {
+    using std::swap;
+    swap(slice_, other.slice_);
+  }
+
   virtual ~SliceAllocator() { grpc_slice_unref(slice_); }
 
   virtual uint8_t *allocate(size_t size) override {
@@ -151,6 +167,29 @@ class MessageBuilder : private detail::SliceAllocatorMember,
   MessageBuilder(const MessageBuilder &other) = delete;
   MessageBuilder &operator=(const MessageBuilder &other) = delete;
 
+  MessageBuilder(MessageBuilder &&other)
+    : FlatBufferBuilder(1024, &slice_allocator_, false) {
+    // Default construct and swap idiom.
+    Swap(other);
+  }
+
+  MessageBuilder &operator=(MessageBuilder &&other) {
+    // Move construct a temporary and swap
+    MessageBuilder temp(std::move(other));
+    Swap(temp);
+    return *this;
+  }
+
+  void Swap(MessageBuilder &other) {
+    slice_allocator_.swap(other.slice_allocator_);
+    FlatBufferBuilder::Swap(other);
+    // After swapping the FlatBufferBuilder, we swap back the allocator, which restores
+    // the original allocator back in place. This is necessary because MessageBuilder's
+    // allocator is its own member (SliceAllocatorMember). The allocator passed to
+    // FlatBufferBuilder::vector_downward must point to this member.
+    buf_.swap_allocator(other.buf_);
+  }
+
   ~MessageBuilder() {}
 
   // GetMessage extracts the subslice of the buffer corresponding to the