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.

Sign up for GitHub

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++] Add support for boolean types in flexbuffers #4386

Merged
merged 8 commits into from
Aug 4, 2017
Merged

[c++] Add support for boolean types in flexbuffers #4386

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
27a6802
Add support for boolean types in flexbuffers
rouzier Jul 20, 2017
1b2e6f0
Simplify casting number <=> boolean
rouzier Jul 27, 2017
1fdc564
Added comments for tests
rouzier Jul 27, 2017
9f61984
Add proper support for Booleans
rouzier Jul 27, 2017
ae18682
Bad rebase
rouzier Jul 28, 2017
5754ff0
No special case for strings
rouzier Jul 29, 2017
5a1f6dd
Removed unused test
rouzier Aug 2, 2017
561a334
Simplify logic
rouzier Aug 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions include/flatbuffers/flexbuffers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ enum Type {
TYPE_VECTOR_UINT4 = 23,
TYPE_VECTOR_FLOAT4 = 24,
TYPE_BLOB = 25,
TYPE_BOOL = 26,
};

inline bool IsInline(Type t) { return t <= TYPE_FLOAT; }
inline bool IsInline(Type t) { return t <= TYPE_FLOAT || t == TYPE_BOOL; }

inline bool IsTypedVectorElementType(Type t) {
return t >= TYPE_INT && t <= TYPE_STRING;
Expand Down Expand Up @@ -349,6 +350,7 @@ class Reference {
Type GetType() const { return type_; }

bool IsNull() const { return type_ == TYPE_NULL; }
bool IsBool() const { return type_ == TYPE_BOOL; }
bool IsInt() const { return type_ == TYPE_INT ||
type_ == TYPE_INDIRECT_INT; }
bool IsUInt() const { return type_ == TYPE_UINT||
Expand All @@ -363,6 +365,13 @@ class Reference {
bool IsMap() const { return type_ == TYPE_MAP; }
bool IsBlob() const { return type_ == TYPE_BLOB; }

bool AsBool() const {
if (type_ == TYPE_BOOL) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

return (type_ == TYPE_BOOL ? .. : ..) != 0

return ReadUInt64(data_, parent_width_) != 0;
}
return AsUInt64() != 0;
}

// Reads any type as a int64_t. Never fails, does most sensible conversion.
// Truncates floats, strings are attempted to be parsed for a number,
// vectors/maps return their size. Returns 0 if all else fails.
Expand All @@ -381,6 +390,7 @@ class Reference {
case TYPE_NULL: return 0;
case TYPE_STRING: return flatbuffers::StringToInt(AsString().c_str());
case TYPE_VECTOR: return static_cast<int64_t>(AsVector().size());
case TYPE_BOOL: return ReadInt64(data_, parent_width_);
default:
// Convert other things to int.
return 0;
Expand Down Expand Up @@ -408,6 +418,7 @@ class Reference {
case TYPE_NULL: return 0;
case TYPE_STRING: return flatbuffers::StringToUInt(AsString().c_str());
case TYPE_VECTOR: return static_cast<uint64_t>(AsVector().size());
case TYPE_BOOL: return ReadUInt64(data_, parent_width_);
default:
// Convert other things to uint.
return 0;
Expand Down Expand Up @@ -435,6 +446,8 @@ class Reference {
case TYPE_NULL: return 0.0;
case TYPE_STRING: return strtod(AsString().c_str(), nullptr);
case TYPE_VECTOR: return static_cast<double>(AsVector().size());
case TYPE_BOOL: return static_cast<double>(
ReadUInt64(data_, parent_width_));
default:
// Convert strings and other things to float.
return 0;
Expand Down Expand Up @@ -494,6 +507,8 @@ class Reference {
s += flatbuffers::NumToString(AsDouble());
} else if (IsNull()) {
s += "null";
} else if (IsBool()) {
s += AsBool() ? "true" : "false";
} else if (IsMap()) {
s += "{ ";
auto m = AsMap();
Expand Down Expand Up @@ -588,6 +603,13 @@ class Reference {
}
}

bool MutateBool(bool b) {
if (type_ == TYPE_BOOL) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

return type_ == TYPE_BOOL && ..

return Mutate(data_, b, parent_width_, BIT_WIDTH_8);
}
return false;
}

bool MutateUInt(uint64_t u) {
if (type_ == TYPE_UINT) {
return Mutate(data_, u, parent_width_, WidthU(u));
Expand Down Expand Up @@ -816,7 +838,7 @@ class Builder FLATBUFFERS_FINAL_CLASS {
void Double(double f) { stack_.push_back(Value(f)); }
void Double(const char *key, double d) { Key(key); Double(d); }

void Bool(bool b) { Int(static_cast<int64_t>(b)); }
void Bool(bool b) { stack_.push_back(Value(b)); }
void Bool(const char *key, bool b) { Key(key); Bool(b); }

void IndirectInt(int64_t i) {
Expand Down Expand Up @@ -1236,6 +1258,8 @@ class Builder FLATBUFFERS_FINAL_CLASS {

Value() : i_(0), type_(TYPE_NULL), min_bit_width_(BIT_WIDTH_8) {}

Value(bool b) : u_(static_cast<uint64_t>(b)), type_(TYPE_BOOL), min_bit_width_(BIT_WIDTH_8) {}

Value(int64_t i, Type t, BitWidth bw)
: i_(i), type_(t), min_bit_width_(bw) {}
Value(uint64_t u, Type t, BitWidth bw)
Expand Down Expand Up @@ -1294,6 +1318,7 @@ class Builder FLATBUFFERS_FINAL_CLASS {
case TYPE_INT:
Write(val.i_, byte_width);
break;
case TYPE_BOOL:
case TYPE_UINT:
Write(val.u_, byte_width);
break;
Expand Down
1 change: 1 addition & 0 deletions include/flatbuffers/idl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ inline bool IsFloat (BaseType t) { return t == BASE_TYPE_FLOAT ||
t == BASE_TYPE_DOUBLE; }
inline bool IsLong (BaseType t) { return t == BASE_TYPE_LONG ||
t == BASE_TYPE_ULONG; }
inline bool IsBool (BaseType t) { return t == BASE_TYPE_BOOL; }

extern const char *const kTypeNames[];
extern const char kTypeSizes[];
Expand Down
17 changes: 15 additions & 2 deletions src/idl_parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ std::string Namespace::GetFullyQualifiedName(const std::string &name,
TD(Attribute, 270, "attribute") \
TD(Null, 271, "null") \
TD(Service, 272, "rpc_service") \
TD(NativeInclude, 273, "native_include")
TD(NativeInclude, 273, "native_include") \
TD(BooleanConstant, 274, "boolean constant")
#ifdef __GNUC__
__extension__ // Stop GCC complaining about trailing comma with -Wpendantic.
#endif
Expand Down Expand Up @@ -391,7 +392,7 @@ CheckedError Parser::Next() {
// which simplifies our logic downstream.
if (attribute_ == "true" || attribute_ == "false") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, in ParseFlexBufferValue, you should check for strings true/false

Copy link
Collaborator

Choose a reason for hiding this comment

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

As discussed above, lets not do this.

attribute_ = NumToString(attribute_ == "true");
token_ = kTokenIntegerConstant;
token_ = kTokenBooleanConstant;
return NoError();
}
// Check for declaration keywords:
Expand Down Expand Up @@ -1338,6 +1339,11 @@ CheckedError Parser::ParseSingleValue(Value &e) {
e,
BASE_TYPE_INT,
&match));
ECHECK(TryTypedValue(kTokenBooleanConstant,
IsBool(e.type.base_type),
e,
BASE_TYPE_BOOL,
&match));
ECHECK(TryTypedValue(kTokenFloatConstant,
IsFloat(e.type.base_type),
e,
Expand Down Expand Up @@ -2007,6 +2013,9 @@ CheckedError Parser::SkipAnyJsonValue() {
case kTokenFloatConstant:
EXPECT(kTokenFloatConstant);
break;
case kTokenBooleanConstant:
EXPECT(kTokenBooleanConstant);
break;
default:
return TokenError();
}
Expand Down Expand Up @@ -2064,6 +2073,10 @@ CheckedError Parser::ParseFlexBufferValue(flexbuffers::Builder *builder) {
builder->Int(StringToInt(attribute_.c_str()));
EXPECT(kTokenIntegerConstant);
break;
case kTokenBooleanConstant:
builder->Bool(StringToInt(attribute_.c_str()) != 0);
EXPECT(kTokenBooleanConstant);
break;
case kTokenFloatConstant:
builder->Double(strtod(attribute_.c_str(), nullptr));
EXPECT(kTokenFloatConstant);
Expand Down
22 changes: 17 additions & 5 deletions tests/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,7 @@ void FlexBuffersTest() {
flexbuffers::BUILDER_FLAG_SHARE_KEYS_AND_STRINGS);

// Write the equivalent of:
// { vec: [ -100, "Fred", 4.0 ], bar: [ 1, 2, 3 ], foo: 100 }
// { vec: [ -100, "Fred", 4.0, false ], bar: [ 1, 2, 3 ], bar3: [ 1, 2, 3 ], foo: 100, bool: true, mymap: { foo: "Fred" } }
#ifndef FLATBUFFERS_CPP98_STL
// It's possible to do this without std::function support as well.
slb.Map([&]() {
Expand All @@ -1601,10 +1601,12 @@ void FlexBuffersTest() {
slb.IndirectFloat(4.0f);
uint8_t blob[] = { 77 };
slb.Blob(blob, 1);
slb += false;
});
int ints[] = { 1, 2, 3 };
slb.Vector("bar", ints, 3);
slb.FixedTypedVector("bar3", ints, 3);
slb.Bool("bool", true);
slb.Double("foo", 100);
slb.Map("mymap", [&]() {
slb.String("foo", "Fred"); // Testing key and string reuse.
Expand Down Expand Up @@ -1637,9 +1639,9 @@ void FlexBuffersTest() {
printf("\n");

auto map = flexbuffers::GetRoot(slb.GetBuffer()).AsMap();
TEST_EQ(map.size(), 5);
TEST_EQ(map.size(), 6);
auto vec = map["vec"].AsVector();
TEST_EQ(vec.size(), 4);
TEST_EQ(vec.size(), 5);
TEST_EQ(vec[0].AsInt64(), -100);
TEST_EQ_STR(vec[1].AsString().c_str(), "Fred");
TEST_EQ(vec[1].AsInt64(), 0); // Number parsing failed.
Expand All @@ -1652,17 +1654,20 @@ void FlexBuffersTest() {
auto blob = vec[3].AsBlob();
TEST_EQ(blob.size(), 1);
TEST_EQ(blob.data()[0], 77);
TEST_EQ(vec[4].IsBool(), true); // Check if type is a bool
TEST_EQ(vec[4].AsBool(), false); // Check if value is false
auto tvec = map["bar"].AsTypedVector();
TEST_EQ(tvec.size(), 3);
TEST_EQ(tvec[2].AsInt8(), 3);
auto tvec3 = map["bar3"].AsFixedTypedVector();
TEST_EQ(tvec3.size(), 3);
TEST_EQ(tvec3[2].AsInt8(), 3);
TEST_EQ(map["bool"].AsBool(), true);
TEST_EQ(map["foo"].AsUInt8(), 100);
TEST_EQ(map["unknown"].IsNull(), true);
auto mymap = map["mymap"].AsMap();
// These should be equal by pointer equality, since key and value are shared.
TEST_EQ(mymap.Keys()[0].AsKey(), map.Keys()[2].AsKey());
TEST_EQ(mymap.Keys()[0].AsKey(), map.Keys()[3].AsKey());
TEST_EQ(mymap.Values()[0].AsString().c_str(), vec[1].AsString().c_str());
// We can mutate values in the buffer.
TEST_EQ(vec[0].MutateInt(-99), true);
Expand All @@ -1673,11 +1678,14 @@ void FlexBuffersTest() {
TEST_EQ(vec[2].MutateFloat(2.0f), true);
TEST_EQ(vec[2].AsFloat(), 2.0f);
TEST_EQ(vec[2].MutateFloat(3.14159), false); // Double does not fit in float.
TEST_EQ(vec[4].AsBool(), false); // Is false before change
TEST_EQ(vec[4].MutateBool(true), true); // Can change a bool
TEST_EQ(vec[4].AsBool(), true); // Changed bool is now true

// Parse from JSON:
flatbuffers::Parser parser;
slb.Clear();
auto jsontest = "{ a: [ 123, 456.0 ], b: \"hello\" }";
auto jsontest = "{ a: [ 123, 456.0 ], b: \"hello\", c: true, d: false }";
TEST_EQ(parser.ParseFlexBuffer(jsontest, nullptr, &slb),
true);
auto jroot = flexbuffers::GetRoot(slb.GetBuffer());
Expand All @@ -1686,6 +1694,10 @@ void FlexBuffersTest() {
TEST_EQ(jvec[0].AsInt64(), 123);
TEST_EQ(jvec[1].AsDouble(), 456.0);
TEST_EQ_STR(jmap["b"].AsString().c_str(), "hello");
TEST_EQ(jmap["c"].IsBool(), true); // Parsed correctly to a bool
TEST_EQ(jmap["c"].AsBool(), true); // Parsed correctly to true
TEST_EQ(jmap["d"].IsBool(), true); // Parsed correctly to a bool
TEST_EQ(jmap["d"].AsBool(), false); // Parsed correctly to false
// And from FlexBuffer back to JSON:
auto jsonback = jroot.ToString();
TEST_EQ_STR(jsontest, jsonback.c_str());
Expand Down