Skip to content

Commit

Permalink
Fix operator==() generated for field of fixed sized array (#7749)
Browse files Browse the repository at this point in the history
* Fix operator==() generated for field of fixed sized array

* Compare address

* noexcept

* Grammer

Co-authored-by: Derek Bailey <derekbailey@google.com>
  • Loading branch information
enum-class and dbaileychess authored Jan 6, 2023
1 parent 07d9485 commit 74b5195
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 9 deletions.
10 changes: 10 additions & 0 deletions include/flatbuffers/array.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#ifndef FLATBUFFERS_ARRAY_H_
#define FLATBUFFERS_ARRAY_H_

#include <memory>

#include "flatbuffers/base.h"
#include "flatbuffers/stl_emulation.h"
#include "flatbuffers/vector.h"
Expand Down Expand Up @@ -238,6 +240,14 @@ const Array<E, length> &CastToArrayOfEnum(const T (&arr)[length]) {
return *reinterpret_cast<const Array<E, length> *>(arr);
}

template<typename T, uint16_t length>
bool operator==(const Array<T, length> &lhs,
const Array<T, length> &rhs) noexcept {
return std::addressof(lhs) == std::addressof(rhs) ||
(lhs.size() == rhs.size() &&
std::memcmp(lhs.Data(), rhs.Data(), rhs.size() * sizeof(T)) == 0);
}

} // namespace flatbuffers

#endif // FLATBUFFERS_ARRAY_H_
3 changes: 2 additions & 1 deletion src/idl_gen_cpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2044,7 +2044,6 @@ class CppGenerator : public BaseGenerator {
const auto accessor = Name(field) + accessSuffix;
const auto lhs_accessor = "lhs." + accessor;
const auto rhs_accessor = "rhs." + accessor;

if (!field.deprecated && // Deprecated fields won't be accessible.
field.value.type.base_type != BASE_TYPE_UTYPE &&
(field.value.type.base_type != BASE_TYPE_VECTOR ||
Expand All @@ -2067,6 +2066,8 @@ class CppGenerator : public BaseGenerator {
" const &b) { return (a == b) || (a && b && *a == *b); })";

compare_op += "(" + equal_length + " && " + elements_equal + ")";
} else if (field.value.type.base_type == BASE_TYPE_ARRAY) {
compare_op += "(*" + lhs_accessor + " == *" + rhs_accessor + ")";
} else {
compare_op += "(" + lhs_accessor + " == " + rhs_accessor + ")";
}
Expand Down
12 changes: 6 additions & 6 deletions tests/arrays_test_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ FLATBUFFERS_STRUCT_END(NestedStruct, 32);

inline bool operator==(const NestedStruct &lhs, const NestedStruct &rhs) {
return
(lhs.a() == rhs.a()) &&
(*lhs.a() == *rhs.a()) &&
(lhs.b() == rhs.b()) &&
(lhs.c() == rhs.c()) &&
(lhs.d() == rhs.d());
(*lhs.c() == *rhs.c()) &&
(*lhs.d() == *rhs.d());
}

inline bool operator!=(const NestedStruct &lhs, const NestedStruct &rhs) {
Expand Down Expand Up @@ -257,11 +257,11 @@ FLATBUFFERS_STRUCT_END(ArrayStruct, 160);
inline bool operator==(const ArrayStruct &lhs, const ArrayStruct &rhs) {
return
(lhs.a() == rhs.a()) &&
(lhs.b() == rhs.b()) &&
(*lhs.b() == *rhs.b()) &&
(lhs.c() == rhs.c()) &&
(lhs.d() == rhs.d()) &&
(*lhs.d() == *rhs.d()) &&
(lhs.e() == rhs.e()) &&
(lhs.f() == rhs.f());
(*lhs.f() == *rhs.f());
}

inline bool operator!=(const ArrayStruct &lhs, const ArrayStruct &rhs) {
Expand Down
4 changes: 2 additions & 2 deletions tests/key_field/key_field_sample_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ FLATBUFFERS_STRUCT_END(Baz, 5);

inline bool operator==(const Baz &lhs, const Baz &rhs) {
return
(lhs.a() == rhs.a()) &&
(*lhs.a() == *rhs.a()) &&
(lhs.b() == rhs.b());
}

Expand Down Expand Up @@ -161,7 +161,7 @@ FLATBUFFERS_STRUCT_END(Bar, 16);

inline bool operator==(const Bar &lhs, const Bar &rhs) {
return
(lhs.a() == rhs.a()) &&
(*lhs.a() == *rhs.a()) &&
(lhs.b() == rhs.b());
}

Expand Down
49 changes: 49 additions & 0 deletions tests/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,54 @@ void FixedLengthArrayConstructorTest() {
void FixedLengthArrayConstructorTest() {}
#endif

void FixedLengthArrayOperatorEqualTest() {
const int32_t nested_a[2] = { 1, 2 };
MyGame::Example::TestEnum nested_c[2] = { MyGame::Example::TestEnum::A,
MyGame::Example::TestEnum::B };

MyGame::Example::TestEnum nested_cc[2] = { MyGame::Example::TestEnum::A,
MyGame::Example::TestEnum::C };
const int64_t int64_2[2] = { -2, -1 };

std::array<MyGame::Example::NestedStruct, 2> init_d = {
{ MyGame::Example::NestedStruct(nested_a, MyGame::Example::TestEnum::B,
nested_c, int64_2),
MyGame::Example::NestedStruct(nested_a, MyGame::Example::TestEnum::B,
nested_c,
std::array<int64_t, 2>{ { -2, -1 } }) }
};

auto different = MyGame::Example::NestedStruct(
nested_a, MyGame::Example::TestEnum::B, nested_cc,
std::array<int64_t, 2>{ { -2, -1 } });

TEST_ASSERT(init_d[0] == init_d[1]);
TEST_ASSERT(init_d[0] != different);

std::array<MyGame::Example::ArrayStruct, 3> arr_struct = {
MyGame::Example::ArrayStruct(
8.125,
std::array<int32_t, 0xF>{
{ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 } },
-17, init_d, 10, int64_2),

MyGame::Example::ArrayStruct(
8.125,
std::array<int32_t, 0xF>{
{ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 } },
-17, init_d, 10, int64_2),

MyGame::Example::ArrayStruct(
8.125,
std::array<int32_t, 0xF>{
{ 1000, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 } },
-17, init_d, 10, int64_2)
};

TEST_ASSERT(arr_struct[0] == arr_struct[1]);
TEST_ASSERT(arr_struct[1] != arr_struct[2]);
}

void NativeTypeTest() {
const int N = 3;

Expand Down Expand Up @@ -1560,6 +1608,7 @@ int FlatBufferTests(const std::string &tests_data_path) {
ParseFlexbuffersFromJsonWithNullTest();
FlatbuffersSpanTest();
FixedLengthArrayConstructorTest();
FixedLengthArrayOperatorEqualTest();
FieldIdentifierTest();
StringVectorDefaultsTest();
FlexBuffersFloatingPointTest();
Expand Down

0 comments on commit 74b5195

Please sign in to comment.