Commit b8ef8c15 authored by Vladimir Glavnyy's avatar Vladimir Glavnyy Committed by Wouter van Oortmerssen

Fix issues with uint64 enums (#5265)

* Fix issues with uint64 enums

- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix #5108
- new tests
- enums with bit_flags attribute should be unsigned

* Refine objectives of EnumDef's FindByValue and ReverseLookup methods

- move EnumDef::ReverseLookup implementation to idl_parser.cpp
- fix typos

* Make the IsUInt64 method private
parent 6cc30b32
......@@ -53,6 +53,11 @@ class CodeWriter {
value_map_[key] = value;
}
std::string GetValue(const std::string &key) const {
const auto it = value_map_.find(key);
return it == value_map_.end() ? "" : it->second;
}
// Appends the given text to the generated code as well as a newline
// character. Any text within {{ and }} delimeters is replaced by values
// previously stored in the CodeWriter by calling SetValue above. The newline
......
......@@ -123,6 +123,13 @@ inline bool IsLong (BaseType t) { return t == BASE_TYPE_LONG ||
inline bool IsBool (BaseType t) { return t == BASE_TYPE_BOOL; }
inline bool IsOneByte(BaseType t) { return t >= BASE_TYPE_UTYPE &&
t <= BASE_TYPE_UCHAR; }
inline bool IsUnsigned(BaseType t) {
return (t == BASE_TYPE_UTYPE) || (t == BASE_TYPE_UCHAR) ||
(t == BASE_TYPE_USHORT) || (t == BASE_TYPE_UINT) ||
(t == BASE_TYPE_ULONG);
}
// clang-format on
extern const char *const kTypeNames[];
......@@ -327,52 +334,96 @@ inline size_t InlineAlignment(const Type &type) {
return IsStruct(type) ? type.struct_def->minalign : SizeOf(type.base_type);
}
struct EnumVal {
EnumVal(const std::string &_name, int64_t _val) : name(_name), value(_val) {}
EnumVal() : value(0) {}
struct EnumDef;
struct EnumValBuilder;
struct EnumVal {
Offset<reflection::EnumVal> Serialize(FlatBufferBuilder *builder, const Parser &parser) const;
bool Deserialize(const Parser &parser, const reflection::EnumVal *val);
uint64_t GetAsUInt64() const { return static_cast<uint64_t>(value); }
int64_t GetAsInt64() const { return value; }
bool IsZero() const { return 0 == value; }
bool IsNonZero() const { return !IsZero(); }
std::string name;
std::vector<std::string> doc_comment;
int64_t value;
Type union_type;
private:
friend EnumDef;
friend EnumValBuilder;
friend bool operator==(const EnumVal &lhs, const EnumVal &rhs);
EnumVal(const std::string &_name, int64_t _val) : name(_name), value(_val) {}
EnumVal() : value(0) {}
int64_t value;
};
struct EnumDef : public Definition {
EnumDef() : is_union(false), uses_multiple_type_instances(false) {}
EnumVal *ReverseLookup(int64_t enum_idx, bool skip_union_default = true) {
for (auto it = Vals().begin() +
static_cast<int>(is_union && skip_union_default);
it != Vals().end(); ++it) {
if ((*it)->value == enum_idx) { return *it; }
}
return nullptr;
}
Offset<reflection::Enum> Serialize(FlatBufferBuilder *builder, const Parser &parser) const;
Offset<reflection::Enum> Serialize(FlatBufferBuilder *builder,
const Parser &parser) const;
bool Deserialize(Parser &parser, const reflection::Enum *values);
template<typename T> void ChangeEnumValue(EnumVal *ev, T new_val);
void SortByValue();
void RemoveDuplicates();
std::string AllFlags() const;
const EnumVal *MinValue() const;
const EnumVal *MaxValue() const;
// Returns the number of integer steps from v1 to v2.
uint64_t Distance(const EnumVal *v1, const EnumVal *v2) const;
// Returns the number of integer steps from Min to Max.
uint64_t Distance() const { return Distance(MinValue(), MaxValue()); }
EnumVal *ReverseLookup(int64_t enum_idx,
bool skip_union_default = false) const;
EnumVal *FindByValue(const std::string &constant) const;
std::string ToString(const EnumVal &ev) const {
return IsUInt64() ? NumToString(ev.GetAsUInt64())
: NumToString(ev.GetAsInt64());
}
size_t size() const { return vals.vec.size(); }
const std::vector<EnumVal *> &Vals() const {
FLATBUFFERS_ASSERT(false == vals.vec.empty());
return vals.vec;
}
SymbolTable<EnumVal> vals;
const EnumVal *Lookup(const std::string &enum_name) const {
return vals.Lookup(enum_name);
}
bool is_union;
// Type is a union which uses type aliases where at least one type is
// available under two different names.
bool uses_multiple_type_instances;
Type underlying_type;
private:
bool IsUInt64() const {
return (BASE_TYPE_ULONG == underlying_type.base_type);
}
friend EnumValBuilder;
SymbolTable<EnumVal> vals;
};
inline bool operator==(const EnumVal &lhs, const EnumVal &rhs) {
return lhs.value == rhs.value;
}
inline bool operator!=(const EnumVal &lhs, const EnumVal &rhs) {
return !(lhs == rhs);
}
inline bool EqualByName(const Type &a, const Type &b) {
return a.base_type == b.base_type && a.element == b.element &&
(a.struct_def == b.struct_def ||
......
This diff is collapsed.
......@@ -242,9 +242,9 @@ class DartGenerator : public BaseGenerator {
// holes.
if (!is_bit_flags) {
code += " static const int minValue = " +
NumToString(enum_def.vals.vec.front()->value) + ";\n";
enum_def.ToString(*enum_def.MinValue()) + ";\n";
code += " static const int maxValue = " +
NumToString(enum_def.vals.vec.back()->value) + ";\n";
enum_def.ToString(*enum_def.MaxValue()) + ";\n";
}
code +=
......@@ -259,13 +259,13 @@ class DartGenerator : public BaseGenerator {
GenDocComment(ev.doc_comment, &code, "", " ");
}
code += " static const " + name + " " + ev.name + " = ";
code += "const " + name + "._(" + NumToString(ev.value) + ");\n";
code += "const " + name + "._(" + enum_def.ToString(ev) + ");\n";
}
code += " static get values => {";
for (auto it = enum_def.Vals().begin(); it != enum_def.Vals().end(); ++it) {
auto &ev = **it;
code += NumToString(ev.value) + ": " + ev.name + ",";
code += enum_def.ToString(ev) + ": " + ev.name + ",";
}
code += "};\n\n";
......@@ -507,7 +507,7 @@ class DartGenerator : public BaseGenerator {
auto &ev = **en_it;
auto enum_name = NamespaceAliasFromUnionType(ev.name);
code += " case " + NumToString(ev.value) + ": return " +
code += " case " + enum_def.ToString(ev) + ": return " +
enum_name + ".reader.vTableGet(_bc, _bcOffset, " +
NumToString(field.value.offset) + ", null);\n";
}
......
......@@ -102,7 +102,7 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
if (enum_def.is_union)
schema += " " + GenType(ev.union_type) + ",\n";
else
schema += " " + ev.name + " = " + NumToString(ev.value) + ",\n";
schema += " " + ev.name + " = " + enum_def.ToString(ev) + ",\n";
}
schema += "}\n\n";
}
......
......@@ -439,21 +439,12 @@ class GeneralGenerator : public BaseGenerator {
}
std::string GenEnumDefaultValue(const FieldDef &field) const {
auto& value = field.value;
auto enum_def = value.type.enum_def;
auto vec = enum_def->vals.vec;
auto default_value = StringToInt(value.constant.c_str());
auto result = value.constant;
for (auto it = vec.begin(); it != vec.end(); ++it) {
auto enum_val = **it;
if (enum_val.value == default_value) {
result = WrapInNameSpace(*enum_def) + "." + enum_val.name;
break;
}
}
return result;
auto &value = field.value;
FLATBUFFERS_ASSERT(value.type.enum_def);
auto &enum_def = *value.type.enum_def;
auto enum_val = enum_def.FindByValue(value.constant);
return enum_val ? (WrapInNameSpace(enum_def) + "." + enum_val->name)
: value.constant;
}
std::string GenDefaultValue(const FieldDef &field, bool enableLangOverrides) const {
......@@ -552,7 +543,7 @@ class GeneralGenerator : public BaseGenerator {
code += GenTypeBasic(enum_def.underlying_type, false);
}
code += " " + ev.name + " = ";
code += NumToString(ev.value);
code += enum_def.ToString(ev);
code += lang_.enum_separator;
}
......@@ -562,21 +553,22 @@ class GeneralGenerator : public BaseGenerator {
// Problem is, if values are very sparse that could generate really big
// tables. Ideally in that case we generate a map lookup instead, but for
// the moment we simply don't output a table at all.
auto range = enum_def.vals.vec.back()->value -
enum_def.vals.vec.front()->value + 1;
auto range = enum_def.Distance();
// Average distance between values above which we consider a table
// "too sparse". Change at will.
static const int kMaxSparseness = 5;
if (range / static_cast<int64_t>(enum_def.vals.vec.size()) <
kMaxSparseness) {
static const uint64_t kMaxSparseness = 5;
if (range / static_cast<uint64_t>(enum_def.size()) < kMaxSparseness) {
code += "\n public static";
code += lang_.const_decl;
code += lang_.string_type;
code += "[] names = { ";
auto val = enum_def.Vals().front()->value;
auto val = enum_def.Vals().front();
for (auto it = enum_def.Vals().begin(); it != enum_def.Vals().end();
++it) {
while (val++ != (*it)->value) code += "\"\", ";
auto ev = *it;
for (auto k = enum_def.Distance(val, ev); k > 1; --k)
code += "\"\", ";
val = ev;
code += "\"" + (*it)->name + "\", ";
}
code += "};\n\n";
......@@ -584,8 +576,8 @@ class GeneralGenerator : public BaseGenerator {
code += lang_.string_type;
code += " " + MakeCamel("name", lang_.first_camel_upper);
code += "(int e) { return names[e";
if (enum_def.vals.vec.front()->value)
code += " - " + enum_def.vals.vec.front()->name;
if (enum_def.MinValue()->IsNonZero())
code += " - " + enum_def.MinValue()->name;
code += "]; }\n";
}
}
......
......@@ -162,7 +162,7 @@ class GoGenerator : public BaseGenerator {
code += " ";
code += GetEnumTypeName(enum_def);
code += " = ";
code += NumToString(ev.value) + "\n";
code += enum_def.ToString(ev) + "\n";
}
// End enum code.
......
......@@ -359,13 +359,13 @@ class JsTsGenerator : public BaseGenerator {
// Generate mapping between EnumName: EnumValue(int)
if (reverse) {
code += " " + NumToString(ev.value);
code += " " + enum_def.ToString(ev);
code += lang_.language == IDLOptions::kTs ? "= " : ": ";
code += "'" + ev.name + "'";
} else {
code += " " + ev.name;
code += lang_.language == IDLOptions::kTs ? "= " : ": ";
code += NumToString(ev.value);
code += enum_def.ToString(ev);
}
code += (it + 1) != enum_def.Vals().end() ? ",\n" : "\n";
......@@ -431,8 +431,7 @@ class JsTsGenerator : public BaseGenerator {
std::string GenDefaultValue(const Value &value, const std::string &context) {
if (value.type.enum_def) {
if (auto val = value.type.enum_def->ReverseLookup(
StringToInt(value.constant.c_str()), false)) {
if (auto val = value.type.enum_def->FindByValue(value.constant)) {
if (lang_.language == IDLOptions::kTs) {
return GenPrefixedTypeName(WrapInNameSpace(*value.type.enum_def),
value.type.enum_def->file) +
......
......@@ -263,7 +263,7 @@ class LobsterGenerator : public BaseGenerator {
auto &ev = **it;
GenComment(ev.doc_comment, code_ptr, nullptr, " ");
code += " " + enum_def.name + "_" + NormalizedName(ev) + " = " +
NumToString(ev.value);
enum_def.ToString(ev);
if (it + 1 != enum_def.Vals().end()) code += ",";
code += "\n";
}
......
......@@ -111,8 +111,8 @@ namespace lua {
// A single enum member.
void EnumMember(const EnumDef &enum_def, const EnumVal &ev, std::string *code_ptr) {
std::string &code = *code_ptr;
code += std::string(Indent) + NormalizedName(ev) + " = " + NumToString(ev.value) + ",\n";
(void)enum_def;
code += std::string(Indent) + NormalizedName(ev) + " = " +
enum_def.ToString(ev) + ",\n";
}
// End enum code.
......
......@@ -125,8 +125,7 @@ class PhpGenerator : public BaseGenerator {
code += Indent + "const ";
code += ev.name;
code += " = ";
code += NumToString(ev.value) + ";\n";
(void)enum_def;
code += enum_def.ToString(ev) + ";\n";
}
// End enum code.
......@@ -875,8 +874,7 @@ class PhpGenerator : public BaseGenerator {
std::string GenDefaultValue(const Value &value) {
if (value.type.enum_def) {
if (auto val = value.type.enum_def->ReverseLookup(
StringToInt(value.constant.c_str()), false)) {
if (auto val = value.type.enum_def->FindByValue(value.constant)) {
return WrapInNameSpace(*value.type.enum_def) + "::" + val->name;
}
}
......
......@@ -118,8 +118,7 @@ class PythonGenerator : public BaseGenerator {
code += Indent;
code += NormalizedName(ev);
code += " = ";
code += NumToString(ev.value) + "\n";
(void)enum_def;
code += enum_def.ToString(ev) + "\n";
}
// End enum code.
......
......@@ -589,20 +589,17 @@ class RustGenerator : public BaseGenerator {
code_ += "#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]";
code_ += "pub enum " + Name(enum_def) + " {";
int64_t anyv = 0;
const EnumVal *minv = nullptr, *maxv = nullptr;
for (auto it = enum_def.Vals().begin(); it != enum_def.Vals().end(); ++it) {
const auto &ev = **it;
GenComment(ev.doc_comment, " ");
code_.SetValue("KEY", Name(ev));
code_.SetValue("VALUE", NumToString(ev.value));
code_.SetValue("VALUE", enum_def.ToString(ev));
code_ += " {{KEY}} = {{VALUE}},";
minv = !minv || minv->value > ev.value ? &ev : minv;
maxv = !maxv || maxv->value < ev.value ? &ev : maxv;
anyv |= ev.value;
}
const EnumVal *minv = enum_def.MinValue();
const EnumVal *maxv = enum_def.MaxValue();
FLATBUFFERS_ASSERT(minv && maxv);
code_ += "";
code_ += "}";
......@@ -611,8 +608,8 @@ class RustGenerator : public BaseGenerator {
code_.SetValue("ENUM_NAME", Name(enum_def));
code_.SetValue("ENUM_NAME_SNAKE", MakeSnakeCase(Name(enum_def)));
code_.SetValue("ENUM_NAME_CAPS", MakeUpper(MakeSnakeCase(Name(enum_def))));
code_.SetValue("ENUM_MIN_BASE_VALUE", NumToString(minv->value));
code_.SetValue("ENUM_MAX_BASE_VALUE", NumToString(maxv->value));
code_.SetValue("ENUM_MIN_BASE_VALUE", enum_def.ToString(*minv));
code_.SetValue("ENUM_MAX_BASE_VALUE", enum_def.ToString(*maxv));
// Generate enum constants, and impls for Follow, EndianScalar, and Push.
code_ += "const ENUM_MIN_{{ENUM_NAME_CAPS}}: {{BASE_TYPE}} = \\";
......@@ -671,34 +668,36 @@ class RustGenerator : public BaseGenerator {
// Problem is, if values are very sparse that could generate really big
// tables. Ideally in that case we generate a map lookup instead, but for
// the moment we simply don't output a table at all.
auto range =
enum_def.vals.vec.back()->value - enum_def.vals.vec.front()->value + 1;
auto range = enum_def.Distance();
// Average distance between values above which we consider a table
// "too sparse". Change at will.
static const int kMaxSparseness = 5;
if (range / static_cast<int64_t>(enum_def.vals.vec.size()) <
kMaxSparseness) {
static const uint64_t kMaxSparseness = 5;
if (range / static_cast<uint64_t>(enum_def.size()) < kMaxSparseness) {
code_ += "#[allow(non_camel_case_types)]";
code_ += "const ENUM_NAMES_{{ENUM_NAME_CAPS}}:[&'static str; " +
NumToString(range) + "] = [";
NumToString(range + 1) + "] = [";
auto val = enum_def.Vals().front()->value;
auto val = enum_def.Vals().front();
for (auto it = enum_def.Vals().begin(); it != enum_def.Vals().end();
++it) {
const auto &ev = **it;
while (val++ != ev.value) { code_ += " \"\","; }
auto suffix = *it != enum_def.vals.vec.back() ? "," : "";
code_ += " \"" + Name(ev) + "\"" + suffix;
auto ev = *it;
for (auto k = enum_def.Distance(val, ev); k > 1; --k) {
code_ += " \"\",";
}
val = ev;
auto suffix = *it != enum_def.Vals().back() ? "," : "";
code_ += " \"" + Name(*ev) + "\"" + suffix;
}
code_ += "];";
code_ += "";
code_ += "pub fn enum_name_{{ENUM_NAME_SNAKE}}(e: {{ENUM_NAME}}) -> "
code_ +=
"pub fn enum_name_{{ENUM_NAME_SNAKE}}(e: {{ENUM_NAME}}) -> "
"&'static str {";
code_ += " let index = e as {{BASE_TYPE}}\\";
if (enum_def.vals.vec.front()->value) {
auto vals = GetEnumValUse(enum_def, *enum_def.vals.vec.front());
if (enum_def.MinValue()->IsNonZero()) {
auto vals = GetEnumValUse(enum_def, *enum_def.MinValue());
code_ += " - " + vals + " as {{BASE_TYPE}}\\";
}
code_ += ";";
......@@ -735,8 +734,7 @@ class RustGenerator : public BaseGenerator {
}
case ftUnionKey:
case ftEnumKey: {
auto ev = field.value.type.enum_def->ReverseLookup(
StringToInt(field.value.constant.c_str()), false);
auto ev = field.value.type.enum_def->FindByValue(field.value.constant);
assert(ev);
return WrapInNameSpace(field.value.type.enum_def->defined_namespace,
GetEnumValUse(*field.value.type.enum_def, *ev));
......
......@@ -51,10 +51,10 @@ bool Print(T val, Type type, int /*indent*/, Type * /*union_type*/,
const IDLOptions &opts, std::string *_text) {
std::string &text = *_text;
if (type.enum_def && opts.output_enum_identifiers) {
auto enum_val = type.enum_def->ReverseLookup(static_cast<int64_t>(val));
if (enum_val) {
auto ev = type.enum_def->ReverseLookup(static_cast<int64_t>(val));
if (ev) {
text += "\"";
text += enum_val->name;
text += ev->name;
text += "\"";
return true;
}
......@@ -251,7 +251,7 @@ static bool GenStruct(const StructDef &struct_def, const Table *table,
}
if (fd.value.type.base_type == BASE_TYPE_UTYPE) {
auto enum_val = fd.value.type.enum_def->ReverseLookup(
table->GetField<uint8_t>(fd.value.offset, 0));
table->GetField<uint8_t>(fd.value.offset, 0), true);
union_type = enum_val ? &enum_val->union_type : nullptr;
}
}
......
This diff is collapsed.
......@@ -1297,7 +1297,6 @@ void ErrorTest() {
TestError("enum X:float {}", "underlying");
TestError("enum X:byte { Y, Y }", "value already");
TestError("enum X:byte { Y=2, Z=1 }", "ascending");
TestError("enum X:byte (bit_flags) { Y=8 }", "bit flag out");
TestError("table X { Y:int; } table X {", "datatype already");
TestError("struct X (force_align: 7) { Y:int; }", "force_align");
TestError("struct X {}", "size 0");
......@@ -1416,6 +1415,13 @@ void EnumStringsTest() {
"root_type T;"
"{ F:[ \"E.C\", \"E.A E.B E.C\" ] }"),
true);
// unsigned bit_flags
flatbuffers::Parser parser3;
TEST_EQ(
parser3.Parse("enum E:uint16 (bit_flags) { F0, F07=7, F08, F14=14, F15 }"
" table T { F: E = \"F15 F08\"; }"
"root_type T;"),
true);
}
void EnumNamesTest() {
......@@ -1436,9 +1442,10 @@ void EnumNamesTest() {
void EnumOutOfRangeTest() {
TestError("enum X:byte { Y = 128 }", "enum value does not fit");
TestError("enum X:byte { Y = -129 }", "enum value does not fit");
TestError("enum X:byte { Y = 127, Z }", "enum value does not fit");
TestError("enum X:byte { Y = 126, Z0, Z1 }", "enum value does not fit");
TestError("enum X:ubyte { Y = -1 }", "enum value does not fit");
TestError("enum X:ubyte { Y = 256 }", "enum value does not fit");
TestError("enum X:ubyte { Y = 255, Z }", "enum value does not fit");
// Unions begin with an implicit "NONE = 0".
TestError("table Y{} union X { Y = -1 }",
"enum values must be specified in ascending order");
......@@ -1448,12 +1455,13 @@ void EnumOutOfRangeTest() {
TestError("enum X:int { Y = 2147483648 }", "enum value does not fit");
TestError("enum X:uint { Y = -1 }", "enum value does not fit");
TestError("enum X:uint { Y = 4294967297 }", "enum value does not fit");
TestError("enum X:long { Y = 9223372036854775808 }", "constant does not fit");
TestError("enum X:long { Y = 9223372036854775807, Z }", "enum value overflows");
TestError("enum X:ulong { Y = -1 }", "enum value does not fit");
// TODO: these are perfectly valid constants that shouldn't fail
TestError("enum X:ulong { Y = 13835058055282163712 }", "constant does not fit");
TestError("enum X:ulong { Y = 18446744073709551615 }", "constant does not fit");
TestError("enum X:long { Y = 9223372036854775808 }", "does not fit");
TestError("enum X:long { Y = 9223372036854775807, Z }", "enum value does not fit");
TestError("enum X:ulong { Y = -1 }", "does not fit");
TestError("enum X:ubyte (bit_flags) { Y=8 }", "bit flag out");
TestError("enum X:byte (bit_flags) { Y=7 }", "must be unsigned"); // -128
// bit_flgs out of range
TestError("enum X:ubyte (bit_flags) { Y0,Y1,Y2,Y3,Y4,Y5,Y6,Y7,Y8 }", "out of range");
}
void EnumValueTest() {
......@@ -1472,6 +1480,15 @@ void EnumValueTest() {
// Generate json with defaults and check.
TEST_EQ(TestValue<int>(nullptr, "E", "enum E:int { Z, V=5 }"), 0);
TEST_EQ(TestValue<int>(nullptr, "E=V", "enum E:int { Z, V=5 }"), 5);
// u84 test
TEST_EQ(TestValue<uint64_t>(nullptr, "E=V",
"enum E:ulong { V = 13835058055282163712 }"),
13835058055282163712ULL);
TEST_EQ(TestValue<uint64_t>(nullptr, "E=V",
"enum E:ulong { V = 18446744073709551615 }"),
18446744073709551615ULL);
// Assign non-enum value to enum field. Is it right?
TEST_EQ(TestValue<int>("{ Y:7 }", "E", "enum E:int { V = 0 }"), 7);
}
void IntegerOutOfRangeTest() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment