Skip to content

Commit

Permalink
Disable armeabi builds for Android and re-enable CI builds. (#4970)
Browse files Browse the repository at this point in the history
armeabi support was removed from the Android NDK so we should no
longer build it.  Since this fixes the Android build failures this
commit also re-enables Travis Android builds.

While re-enabling Android builds, some recent changes broke C++98
support so this fixes those issues as well which include:
- Conditionally compiling use of move constructors, operators and
  std::move.
- Changing sample to use flatbuffers::unique_ptr rather than
  std::unique_ptr.

Finally, added the special "default_ptr_type" value for the
"cpp_ptr_type" attribute.  This expands to the value passed to
the "--cpp-ptr-type" argument of flatc.
  • Loading branch information
Stewart Miles authored and aardappel committed Oct 8, 2018
1 parent d840856 commit 569492e
Show file tree
Hide file tree
Showing 11 changed files with 158 additions and 67 deletions.
46 changes: 23 additions & 23 deletions .travis.yml
Expand Up @@ -121,26 +121,26 @@ matrix:
osx_image: xcode9.3
env: CONAN_APPLE_CLANG_VERSIONS=9.1

#- language: android
# sudo: true
# android:
# components:
# - tools
# - platform-tools
# - build-tools-25.0.2
# - android-25
# - extra-android-m2repository
# compiler:
# - gcc
# before_install:
# - git clone https://github.com/urho3d/android-ndk.git $HOME/android-ndk-root
# - export ANDROID_NDK_HOME=$HOME/android-ndk-root
# # Setup environment for Linux build which is required to build the sample.
# - if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test; fi
# - if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo apt-get update -qq; fi
# - if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo apt-get install -qq g++-$GCC_VERSION; fi
# - if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo apt-get install -qq gcc-$GCC_VERSION; fi
# - if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo ln -s -v -f $(which g++-$GCC_VERSION) /usr/bin/g++; fi
# - if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo ln -s -v -f $(which gcc-$GCC_VERSION) /usr/bin/gcc; fi
# script:
# - failed=0; for build_gradle in $(git ls-files | grep build.gradle); do ( cd "$(dirname "${build_gradle}")" && ./gradlew build ) || failed=1; done; exit $((failed))
- language: android
sudo: true
android:
components:
- tools
- platform-tools
- build-tools-25.0.2
- android-25
- extra-android-m2repository
compiler:
- gcc
before_install:
- git clone https://github.com/urho3d/android-ndk.git $HOME/android-ndk-root
- export ANDROID_NDK_HOME=$HOME/android-ndk-root
# Setup environment for Linux build which is required to build the sample.
- if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test; fi
- if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo apt-get update -qq; fi
- if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo apt-get install -qq g++-$GCC_VERSION; fi
- if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo apt-get install -qq gcc-$GCC_VERSION; fi
- if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo ln -s -v -f $(which g++-$GCC_VERSION) /usr/bin/g++; fi
- if [ "$TRAVIS_OS_NAME" == "linux" ]; then sudo ln -s -v -f $(which gcc-$GCC_VERSION) /usr/bin/gcc; fi
script:
- failed=0; for build_gradle in $(git ls-files | grep build.gradle); do ( cd "$(dirname "${build_gradle}")" && ./gradlew build ) || failed=1; done; exit $((failed))
2 changes: 1 addition & 1 deletion android/build.gradle
Expand Up @@ -66,7 +66,7 @@ android {
ndkBuild {
targets "FlatBufferTest"
arguments "-j" + Runtime.getRuntime().availableProcessors()
abiFilters "armeabi", "armeabi-v7a", "arm64-v8a", "x86", "x86_64"
abiFilters "armeabi-v7a", "arm64-v8a", "x86", "x86_64"
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion docs/source/CppUsage.md
Expand Up @@ -240,7 +240,9 @@ influence this either globally (using the `--cpp-ptr-type` argument to
`flatc`) or per field (using the `cpp_ptr_type` attribute) to by any smart
pointer type (`my_ptr<T>`), or by specifying `naked` as the type to get `T *`
pointers. Unlike the smart pointers, naked pointers do not manage memory for
you, so you'll have to manage their lifecycles manually.
you, so you'll have to manage their lifecycles manually. To reference the
pointer type specified by the `--cpp-ptr-type` argument to `flatc` from a
flatbuffer field set the `cpp_ptr_type` attribute to `default_ptr_type`.


# Using different string type.
Expand Down
46 changes: 46 additions & 0 deletions include/flatbuffers/flatbuffers.h
Expand Up @@ -104,10 +104,14 @@ template<typename T, typename IT> struct VectorIterator {
return *this;
}

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
VectorIterator &operator=(VectorIterator &&other) {
data_ = other.data_;
return *this;
}
#endif // !defined(FLATBUFFERS_CPP98_STL)
// clang-format on

bool operator==(const VectorIterator &other) const {
return data_ == other.data_;
Expand Down Expand Up @@ -469,6 +473,9 @@ class DetachedBuffer {
cur_(cur),
size_(sz) {}

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
DetachedBuffer(DetachedBuffer &&other)
: allocator_(other.allocator_),
own_allocator_(other.own_allocator_),
Expand All @@ -478,7 +485,13 @@ class DetachedBuffer {
size_(other.size_) {
other.reset();
}
// clang-format off
#endif // !defined(FLATBUFFERS_CPP98_STL)
// clang-format on

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
DetachedBuffer &operator=(DetachedBuffer &&other) {
destroy();

Expand All @@ -493,6 +506,9 @@ class DetachedBuffer {

return *this;
}
// clang-format off
#endif // !defined(FLATBUFFERS_CPP98_STL)
// clang-format on

~DetachedBuffer() { destroy(); }

Expand Down Expand Up @@ -522,10 +538,16 @@ class DetachedBuffer {
#endif
// clang-format on

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
// These may change access mode, leave these at end of public section
FLATBUFFERS_DELETE_FUNC(DetachedBuffer(const DetachedBuffer &other))
FLATBUFFERS_DELETE_FUNC(
DetachedBuffer &operator=(const DetachedBuffer &other))
// clang-format off
#endif // !defined(FLATBUFFERS_CPP98_STL)
// clang-format on

protected:
Allocator *allocator_;
Expand Down Expand Up @@ -572,7 +594,13 @@ class vector_downward {
cur_(nullptr),
scratch_(nullptr) {}

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
vector_downward(vector_downward &&other)
#else
vector_downward(vector_downward &other)
#endif // defined(FLATBUFFERS_CPP98_STL)
// clang-format on
: allocator_(other.allocator_),
own_allocator_(other.own_allocator_),
initial_size_(other.initial_size_),
Expand All @@ -591,12 +619,18 @@ class vector_downward {
other.scratch_ = nullptr;
}

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
vector_downward &operator=(vector_downward &&other) {
// Move construct a temporary and swap idiom
vector_downward temp(std::move(other));
swap(temp);
return *this;
}
// clang-format off
#endif // defined(FLATBUFFERS_CPP98_STL)
// clang-format on

~vector_downward() {
clear_buffer();
Expand Down Expand Up @@ -842,8 +876,13 @@ class FlatBufferBuilder {
EndianCheck();
}

// clang-format off
/// @brief Move constructor for FlatBufferBuilder.
#if !defined(FLATBUFFERS_CPP98_STL)
FlatBufferBuilder(FlatBufferBuilder &&other)
#else
FlatBufferBuilder(FlatBufferBuilder &other)
#endif // #if !defined(FLATBUFFERS_CPP98_STL)
: buf_(1024, nullptr, false, AlignOf<largest_scalar_t>()),
num_field_loc(0),
max_voffset_(0),
Expand All @@ -858,14 +897,21 @@ class FlatBufferBuilder {
// Lack of delegating constructors in vs2010 makes it more verbose than needed.
Swap(other);
}
// clang-format on

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
/// @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;
}
// clang-format off
#endif // defined(FLATBUFFERS_CPP98_STL)
// clang-format on

void Swap(FlatBufferBuilder &other) {
using std::swap;
Expand Down
2 changes: 1 addition & 1 deletion samples/android/build.gradle
Expand Up @@ -66,7 +66,7 @@ android {
ndkBuild {
targets "FlatBufferSample"
arguments "-j" + Runtime.getRuntime().availableProcessors()
abiFilters "armeabi", "armeabi-v7a", "arm64-v8a", "x86", "x86_64"
abiFilters "armeabi-v7a", "arm64-v8a", "x86", "x86_64"
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/idl_gen_cpp.cpp
Expand Up @@ -578,7 +578,8 @@ class CppGenerator : public BaseGenerator {
bool is_constructor) {
auto &ptr_type = PtrType(field);
if (ptr_type != "naked") {
return ptr_type + "<" + type + ">";
return (ptr_type != "default_ptr_type" ? ptr_type :
parser_.opts.cpp_object_api_pointer_type) + "<" + type + ">";
} else if (is_constructor) {
return "";
} else {
Expand Down
Binary file modified tests/monster_test.bfbs
Binary file not shown.
4 changes: 2 additions & 2 deletions tests/monster_test.fbs
Expand Up @@ -89,9 +89,9 @@ table Monster {
vector_of_referrables:[Referrable](id:35);
single_weak_reference:ulong(id:36, hash:"fnv1a_64", cpp_type:"ReferrableT");
vector_of_weak_references:[ulong](id:37, hash:"fnv1a_64", cpp_type:"ReferrableT");
vector_of_strong_referrables:[Referrable](id:38, cpp_ptr_type:"std::unique_ptr"); //was shared_ptr
vector_of_strong_referrables:[Referrable](id:38, cpp_ptr_type:"default_ptr_type"); //was shared_ptr
co_owning_reference:ulong(id:39, hash:"fnv1a_64", cpp_type:"ReferrableT", cpp_ptr_type:"naked"); //was shared_ptr as well
vector_of_co_owning_references:[ulong](id:40, hash:"fnv1a_64", cpp_type:"ReferrableT", cpp_ptr_type:"std::unique_ptr", cpp_ptr_type_get:".get()"); //was shared_ptr
vector_of_co_owning_references:[ulong](id:40, hash:"fnv1a_64", cpp_type:"ReferrableT", cpp_ptr_type:"default_ptr_type", cpp_ptr_type_get:".get()"); //was shared_ptr
non_owning_reference:ulong(id:41, hash:"fnv1a_64", cpp_type:"ReferrableT", cpp_ptr_type:"naked", cpp_ptr_type_get:""); //was weak_ptr
vector_of_non_owning_references:[ulong](id:42, hash:"fnv1a_64", cpp_type:"ReferrableT", cpp_ptr_type:"naked", cpp_ptr_type_get:""); //was weak_ptr
}
Expand Down
8 changes: 4 additions & 4 deletions tests/monster_test_generated.h
Expand Up @@ -802,9 +802,9 @@ struct MonsterT : public flatbuffers::NativeTable {
std::vector<flatbuffers::unique_ptr<ReferrableT>> vector_of_referrables;
ReferrableT *single_weak_reference;
std::vector<ReferrableT *> vector_of_weak_references;
std::vector<std::unique_ptr<ReferrableT>> vector_of_strong_referrables;
std::vector<flatbuffers::unique_ptr<ReferrableT>> vector_of_strong_referrables;
ReferrableT *co_owning_reference;
std::vector<std::unique_ptr<ReferrableT>> vector_of_co_owning_references;
std::vector<flatbuffers::unique_ptr<ReferrableT>> vector_of_co_owning_references;
ReferrableT *non_owning_reference;
std::vector<ReferrableT *> vector_of_non_owning_references;
MonsterT()
Expand Down Expand Up @@ -2072,10 +2072,10 @@ if (_resolver) (*_resolver)(reinterpret_cast<void **>(&_o->testhashu32_fnv1a), s
if (_resolver) (*_resolver)(reinterpret_cast<void **>(&_o->single_weak_reference), static_cast<flatbuffers::hash_value_t>(_e)); else _o->single_weak_reference = nullptr; };
{ auto _e = vector_of_weak_references(); if (_e) { _o->vector_of_weak_references.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { //vector resolver, naked
if (_resolver) (*_resolver)(reinterpret_cast<void **>(&_o->vector_of_weak_references[_i]), static_cast<flatbuffers::hash_value_t>(_e->Get(_i))); else _o->vector_of_weak_references[_i] = nullptr; } } };
{ auto _e = vector_of_strong_referrables(); if (_e) { _o->vector_of_strong_referrables.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { _o->vector_of_strong_referrables[_i] = std::unique_ptr<ReferrableT>(_e->Get(_i)->UnPack(_resolver)); } } };
{ auto _e = vector_of_strong_referrables(); if (_e) { _o->vector_of_strong_referrables.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { _o->vector_of_strong_referrables[_i] = flatbuffers::unique_ptr<ReferrableT>(_e->Get(_i)->UnPack(_resolver)); } } };
{ auto _e = co_owning_reference(); //scalar resolver, naked
if (_resolver) (*_resolver)(reinterpret_cast<void **>(&_o->co_owning_reference), static_cast<flatbuffers::hash_value_t>(_e)); else _o->co_owning_reference = nullptr; };
{ auto _e = vector_of_co_owning_references(); if (_e) { _o->vector_of_co_owning_references.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { //vector resolver, std::unique_ptr
{ auto _e = vector_of_co_owning_references(); if (_e) { _o->vector_of_co_owning_references.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { //vector resolver, default_ptr_type
if (_resolver) (*_resolver)(reinterpret_cast<void **>(&_o->vector_of_co_owning_references[_i]), static_cast<flatbuffers::hash_value_t>(_e->Get(_i)));/* else do nothing */; } } };
{ auto _e = non_owning_reference(); //scalar resolver, naked
if (_resolver) (*_resolver)(reinterpret_cast<void **>(&_o->non_owning_reference), static_cast<flatbuffers::hash_value_t>(_e)); else _o->non_owning_reference = nullptr; };
Expand Down
18 changes: 18 additions & 0 deletions tests/test_builder.cpp
@@ -1,3 +1,5 @@
#include "flatbuffers/stl_emulation.h"

#include "monster_test_generated.h"
#include "test_builder.h"

Expand All @@ -12,20 +14,30 @@ struct OwnedAllocator : public flatbuffers::DefaultAllocator {};

class TestHeapBuilder : public flatbuffers::FlatBufferBuilder {
private:
// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
TestHeapBuilder(const TestHeapBuilder &);
TestHeapBuilder &operator=(const TestHeapBuilder &);
#endif // !defined(FLATBUFFERS_CPP98_STL)
// clang-format on

public:
TestHeapBuilder()
: flatbuffers::FlatBufferBuilder(2048, new OwnedAllocator(), true) {}

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
TestHeapBuilder(TestHeapBuilder &&other)
: FlatBufferBuilder(std::move(other)) { }

TestHeapBuilder &operator=(TestHeapBuilder &&other) {
FlatBufferBuilder::operator=(std::move(other));
return *this;
}
// clang-format off
#endif // !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
};

// This class simulates flatbuffers::grpc::detail::SliceAllocatorMember
Expand All @@ -49,12 +61,18 @@ struct GrpcLikeMessageBuilder : private AllocatorMember,
Swap(other);
}

// clang-format off
#if !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
GrpcLikeMessageBuilder &operator=(GrpcLikeMessageBuilder &&other) {
// Construct temporary and swap idiom
GrpcLikeMessageBuilder temp(std::move(other));
Swap(temp);
return *this;
}
// clang-format off
#endif // !defined(FLATBUFFERS_CPP98_STL)
// clang-format on

void Swap(GrpcLikeMessageBuilder &other) {
// No need to swap member_allocator_ because it's stateless.
Expand Down

0 comments on commit 569492e

Please sign in to comment.