Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed length arrays #3987

Closed
wants to merge 8 commits into from
Closed

Fixed length arrays #3987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d95238f
Added type for arrays with fixed length
idoroshev Aug 12, 2016
7709b55
Added parsing for arrays with fixed length
idoroshev Aug 12, 2016
baed4b7
Restricted using of arrays with fixed length not in C++
idoroshev Aug 12, 2016
4633759
Added generation of arrays with fixed length for C++.
daksenik Aug 12, 2016
b949376
Added generation of arrays with fixed length for JSON
daksenik Aug 12, 2016
c4b7b38
Added tests of arrays with fixed length.
daksenik Aug 12, 2016
40e16d0
fixed use of undeclared identifier
daksenik Aug 12, 2016
1b2ed31
regenerated tests
daksenik Aug 12, 2016
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
23 changes: 17 additions & 6 deletions include/flatbuffers/idl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ namespace flatbuffers {
TD(LONG, "long", int64_t, long, int64, long, int64) \
TD(ULONG, "ulong", uint64_t, long, uint64, ulong, uint64) /* end int */ \
TD(FLOAT, "float", float, float, float32, float, float32) /* begin float */ \
TD(DOUBLE, "double", double, double, float64, double, float64) /* end float/scalar */
TD(DOUBLE, "double", double, double, float64, double, float64) /* end float/scalar */ \
TD(ARRAY, "", int, int, int, int, int)
#define FLATBUFFERS_GEN_TYPES_POINTER(TD) \
TD(STRING, "string", Offset<void>, int, int, StringOffset, int) \
TD(VECTOR, "", Offset<void>, int, int, VectorOffset, int) \
Expand Down Expand Up @@ -118,11 +119,13 @@ class Parser;
// and additional information for vectors/structs_.
struct Type {
explicit Type(BaseType _base_type = BASE_TYPE_NONE,
StructDef *_sd = nullptr, EnumDef *_ed = nullptr)
StructDef *_sd = nullptr, EnumDef *_ed = nullptr,
short _fixed_length = 0)
: base_type(_base_type),
element(BASE_TYPE_NONE),
struct_def(_sd),
enum_def(_ed)
enum_def(_ed),
fixed_length(_fixed_length)
{}

bool operator==(const Type &o) {
Expand All @@ -135,10 +138,11 @@ struct Type {
Offset<reflection::Type> Serialize(FlatBufferBuilder *builder) const;

BaseType base_type;
BaseType element; // only set if t == BASE_TYPE_VECTOR
BaseType element; // only set if t == BASE_TYPE_VECTOR or t == BASE_TYPE_ARRAY
StructDef *struct_def; // only set if t or element == BASE_TYPE_STRUCT
EnumDef *enum_def; // set if t == BASE_TYPE_UNION / BASE_TYPE_UTYPE,
// or for an integral type derived from an enum.
short fixed_length; // only set if t == BASE_TYPE_ARRAY
};

// Represents a parsed scalar value, it's type, and field offset.
Expand Down Expand Up @@ -269,12 +273,18 @@ inline bool IsStruct(const Type &type) {
return type.base_type == BASE_TYPE_STRUCT && type.struct_def->fixed;
}

inline bool IsArray(const Type &type) {
return type.base_type == BASE_TYPE_ARRAY;
}

inline size_t InlineSize(const Type &type) {
return IsStruct(type) ? type.struct_def->bytesize : SizeOf(type.base_type);
return IsStruct(type) ? type.struct_def->bytesize : IsArray(type)
? SizeOf(type.element) * type.fixed_length : SizeOf(type.base_type);
}

inline size_t InlineAlignment(const Type &type) {
return IsStruct(type) ? type.struct_def->minalign : SizeOf(type.base_type);
return IsStruct(type) ? type.struct_def->minalign :
SizeOf(IsArray(type) ? type.element : type.base_type);
}

struct EnumVal {
Expand Down Expand Up @@ -515,6 +525,7 @@ class Parser : public ParserState {
std::string *value, uoffset_t *ovalue);
void SerializeStruct(const StructDef &struct_def, const Value &val);
void AddVector(bool sortbysize, int count);
FLATBUFFERS_CHECKED_ERROR ParseArray(const Type &type, const short length);
FLATBUFFERS_CHECKED_ERROR ParseVector(const Type &type, uoffset_t *ovalue);
FLATBUFFERS_CHECKED_ERROR ParseMetaData(SymbolTable<Value> *attributes);
FLATBUFFERS_CHECKED_ERROR TryTypedValue(int dtoken, bool check, Value &e,
Expand Down
21 changes: 14 additions & 7 deletions include/flatbuffers/reflection_generated.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ enum BaseType {
ULong = 10,
Float = 11,
Double = 12,
String = 13,
Vector = 14,
Obj = 15,
Union = 16,
Array = 13,
String = 14,
Vector = 15,
Obj = 16,
Union = 17
};

inline const char **EnumNamesBaseType() {
static const char *names[] = { "None", "UType", "Bool", "Byte", "UByte", "Short", "UShort", "Int", "UInt", "Long", "ULong", "Float", "Double", "String", "Vector", "Obj", "Union", nullptr };
static const char *names[] = { "None", "UType", "Bool", "Byte", "UByte", "Short", "UShort", "Int", "UInt", "Long", "ULong", "Float", "Double", "Array", "String", "Vector", "Obj", "Union", nullptr };
return names;
}

Expand All @@ -52,15 +53,18 @@ struct Type FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
enum {
VT_BASE_TYPE = 4,
VT_ELEMENT = 6,
VT_INDEX = 8
VT_FIXED_LENGTH = 8,
VT_INDEX = 10
};
BaseType base_type() const { return static_cast<BaseType>(GetField<int8_t>(VT_BASE_TYPE, 0)); }
BaseType element() const { return static_cast<BaseType>(GetField<int8_t>(VT_ELEMENT, 0)); }
int16_t fixed_length() const { return GetField<int16_t>(VT_FIXED_LENGTH, 0); }
int32_t index() const { return GetField<int32_t>(VT_INDEX, -1); }
bool Verify(flatbuffers::Verifier &verifier) const {
return VerifyTableStart(verifier) &&
VerifyField<int8_t>(verifier, VT_BASE_TYPE) &&
VerifyField<int8_t>(verifier, VT_ELEMENT) &&
VerifyField<int16_t>(verifier, VT_FIXED_LENGTH) &&
VerifyField<int32_t>(verifier, VT_INDEX) &&
verifier.EndTable();
}
Expand All @@ -71,21 +75,24 @@ struct TypeBuilder {
flatbuffers::uoffset_t start_;
void add_base_type(BaseType base_type) { fbb_.AddElement<int8_t>(Type::VT_BASE_TYPE, static_cast<int8_t>(base_type), 0); }
void add_element(BaseType element) { fbb_.AddElement<int8_t>(Type::VT_ELEMENT, static_cast<int8_t>(element), 0); }
void add_fixed_length(int16_t fixed_length) { fbb_.AddElement<int16_t>(Type::VT_FIXED_LENGTH, fixed_length, 0); }
void add_index(int32_t index) { fbb_.AddElement<int32_t>(Type::VT_INDEX, index, -1); }
TypeBuilder(flatbuffers::FlatBufferBuilder &_fbb) : fbb_(_fbb) { start_ = fbb_.StartTable(); }
TypeBuilder &operator=(const TypeBuilder &);
flatbuffers::Offset<Type> Finish() {
auto o = flatbuffers::Offset<Type>(fbb_.EndTable(start_, 3));
auto o = flatbuffers::Offset<Type>(fbb_.EndTable(start_, 4));
return o;
}
};

inline flatbuffers::Offset<Type> CreateType(flatbuffers::FlatBufferBuilder &_fbb,
BaseType base_type = None,
BaseType element = None,
int16_t fixed_length = 0,
int32_t index = -1) {
TypeBuilder builder_(_fbb);
builder_.add_index(index);
builder_.add_fixed_length(fixed_length);
builder_.add_element(element);
builder_.add_base_type(base_type);
return builder_.Finish();
Expand Down
2 changes: 2 additions & 0 deletions reflection/reflection.fbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ enum BaseType : byte {
ULong,
Float,
Double,
Array,
Copy link

Choose a reason for hiding this comment

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

Adding it in the middle rather than at the end will break all reflection files out there.

String,
Vector,
Obj, // Used for tables & structs.
Expand All @@ -29,6 +30,7 @@ enum BaseType : byte {
table Type {
base_type:BaseType;
element:BaseType = None; // Only if base_type == Vector.
fixed_length:short = 0; // Only if base_type == Array.
Copy link

Choose a reason for hiding this comment

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

same here, this is an incompatible change. has to be at the end, or use ids. or maybe reuse index for this purpose.

index:int = -1; // If base_type == Object, index into "objects" below.
// If base_type == Union, UnionType, or integral derived
// from an enum, index into "enums" below.
Expand Down
54 changes: 40 additions & 14 deletions src/idl_gen_cpp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,8 @@ class CppGenerator : public BaseGenerator {
bool user_facing_type) {
return IsScalar(type.base_type)
? GenTypeBasic(type, user_facing_type) + afterbasic
: IsArray(type) ? beforeptr
+ GenTypeBasic(type.VectorType(), user_facing_type) + afterptr
: beforeptr + GenTypePointer(type) + afterptr;
}

Expand Down Expand Up @@ -1153,7 +1155,13 @@ class CppGenerator : public BaseGenerator {
it != struct_def.fields.vec.end(); ++it) {
auto &field = **it;
code += " " + GenTypeGet(field.value.type, " ", "", " ", false);
code += field.name + "_;\n";
code += field.name + "_";
if (IsArray(field.value.type)) {
code += "[";
code += NumToString<short>(field.value.type.fixed_length);
code += "]";
}
code += ";\n";
GenPadding(field, code, padding_id, PaddingDefinition);
}

Expand All @@ -1176,27 +1184,40 @@ class CppGenerator : public BaseGenerator {
it != struct_def.fields.vec.end(); ++it) {
auto &field = **it;
if (it != struct_def.fields.vec.begin()) code += ", ";
code += GenTypeGet(field.value.type, " ", "const ", " &", true);
code += GenTypeGet(field.value.type, " ", "const ",
IsArray(field.value.type) ? " *" : " &", true);
code += "_" + field.name;
}
code += ")\n : ";
padding_id = 0;
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
auto &field = **it;
if (it != struct_def.fields.vec.begin()) code += ", ";
code += field.name + "_(";
if (IsScalar(field.value.type.base_type)) {
code += "flatbuffers::EndianScalar(";
code += GenUnderlyingCast(field, false, "_" + field.name);
code += "))";
} else {
code += "_" + field.name + ")";
if (!IsArray(field.value.type)) {
if (it != struct_def.fields.vec.begin()) code += ", ";
code += field.name + "_(";
if (IsScalar(field.value.type.base_type)) {
code += "flatbuffers::EndianScalar(";
code += GenUnderlyingCast(field, false, "_" + field.name);
code += "))";
} else {
code += "_" + field.name + ")";
}
}
GenPadding(field, code, padding_id, PaddingInitializer);
}

code += " {";
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
auto &field = **it;
if (IsArray(field.value.type)) {
code += " memcpy(";
code += field.name + "_, ";
code += "_" + field.name + ", ";
code += NumToString(InlineSize(field.value.type)) + ");";
}
}
padding_id = 0;
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
Expand All @@ -1212,13 +1233,17 @@ class CppGenerator : public BaseGenerator {
auto &field = **it;
GenComment(field.doc_comment, code_ptr, nullptr, " ");
auto is_scalar = IsScalar(field.value.type.base_type);
code += " " + GenTypeGet(field.value.type, " ", "const ", " &", true);
code += " " + GenTypeGet(field.value.type, " ", "const ",
IsArray(field.value.type) ? " *" : " &", true);
code += field.name + "() const { return ";
code += GenUnderlyingCast(
field, true, is_scalar
code += GenUnderlyingCast(field, true, is_scalar
? "flatbuffers::EndianScalar(" + field.name + "_)"
: field.name + "_");
code += "; }\n";
if (IsArray(field.value.type)) {
code += " int16_t " + field.name + "_length() const { return ";
code += NumToString(field.value.type.fixed_length) + "; }\n";
}
if (parser_.opts.mutable_buffer) {
if (is_scalar) {
code += " void mutate_" + field.name + "(";
Expand All @@ -1229,7 +1254,8 @@ class CppGenerator : public BaseGenerator {
code += "); }\n";
} else {
code += " ";
code += GenTypeGet(field.value.type, "", "", " &", true);
code += GenTypeGet(field.value.type, "", "",
IsArray(field.value.type) ? " *" : " &", true);
code += "mutable_" + field.name + "() { return " + field.name;
code += "_; }\n";
}
Expand Down
1 change: 1 addition & 0 deletions src/idl_gen_general.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,7 @@ void GenStruct(StructDef &struct_def, std::string *code_ptr) {
it != struct_def.fields.vec.end();
++it) {
auto &field = **it;
assert(field.value.type.base_type != BASE_TYPE_ARRAY);
Copy link

Choose a reason for hiding this comment

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

asserts just produce a crash for the user. if people start making schemas with arrays in them, then unsupported languages need to get a proper error message somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we just print the error message and then use exit(1) to finish the program?

Copy link

Choose a reason for hiding this comment

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

It's nicer to handle errors in a centralized location, so I'm afraid we're going to have to pass an error back. To make that simpler you can stick a std::string error_ in BaseGenerator that you then check for not being empty in flatc.cpp. And return early from this function.

Copy link
Contributor Author

@daksenik daksenik Aug 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Function for struct generation is out of generator class in go and python generators, so this function has not access to the error_ field. I see two solutions here: to pass error_ as additional parameter to GenStruct function or wrap GenStruct and GenEnum into generator class.

Copy link

Choose a reason for hiding this comment

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

If you could improve the Python/Go generators while you're at it, that be best I guess.

if (field.deprecated) continue;
GenComment(field.doc_comment, code_ptr, &lang_.comment_config, " ");
std::string type_name = GenTypeGet(field.value.type);
Expand Down
1 change: 1 addition & 0 deletions src/idl_gen_go.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ static void GenStruct(const StructDef &struct_def,
it != struct_def.fields.vec.end();
++it) {
auto &field = **it;
assert(field.value.type.base_type != BASE_TYPE_ARRAY);
if (field.deprecated) continue;

GenStructAccessor(struct_def, field, code_ptr);
Expand Down
1 change: 1 addition & 0 deletions src/idl_gen_js.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ void GenStruct(const Parser &parser, StructDef &struct_def, std::string *code_pt
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
auto &field = **it;
assert(field.value.type.base_type != BASE_TYPE_ARRAY);
if (field.deprecated) continue;
auto offset_prefix = " var offset = this.bb.__offset(this.bb_pos, " +
NumToString(field.value.offset) + ");\n return offset ? ";
Expand Down
1 change: 1 addition & 0 deletions src/idl_gen_php.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,7 @@ namespace php {
it != struct_def.fields.vec.end();
++it) {
auto &field = **it;
assert(field.value.type.base_type != BASE_TYPE_ARRAY);
if (field.deprecated) continue;

GenStructAccessor(struct_def, field, code_ptr);
Expand Down
1 change: 1 addition & 0 deletions src/idl_gen_python.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ static void GenStruct(const StructDef &struct_def,
it != struct_def.fields.vec.end();
++it) {
auto &field = **it;
assert(field.value.type.base_type != BASE_TYPE_ARRAY);
if (field.deprecated) continue;

GenStructAccessor(struct_def, field, code_ptr);
Expand Down
12 changes: 11 additions & 1 deletion src/idl_gen_text.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void OutputIdentifier(const std::string &name, const IDLOptions &opts,
// Print (and its template specialization below for pointers) generate text
// for a single FlatBuffer value into JSON format.
// The general case for scalars:
template<typename T> void Print(T val, Type type, int /*indent*/,
template<typename T> void Print(T val, Type type, int indent,
StructDef * /*union_sd*/,
const IDLOptions &opts,
std::string *_text) {
Expand All @@ -63,6 +63,16 @@ template<typename T> void Print(T val, Type type, int /*indent*/,

if (type.base_type == BASE_TYPE_BOOL) {
text += val != 0 ? "true" : "false";
} else if (type.base_type == BASE_TYPE_ARRAY) {
text += '[';
text += NewLine(opts);
for (short i = 1; i < type.fixed_length; i++) {
text.append(indent + Indent(opts), ' ');
text += ",";
text += NewLine(opts);
}
text.append(indent, ' ');
text += "]";
} else {
text += NumToString(val);
}
Expand Down
Loading