Schema parser: prohibit declaration of an array of pointers inside structs (#5907)

* Fix issue #5906, Prohibit declaration of an array of pointers inside structs

- idl_parser.cpp: Prohibit declaration of an array of pointers inside structs
- idl_gen_cpp.cpp: Extract GenStructConstructor() method from GenStruct() to simplify future modification
- idl_gen_cpp.cpp: Add assert for checking of Array fields in structs on code-generation stage

* Fix the error 'unused local variable' in release build

* Fix: format the PR code according to coding rules

* Add test-case and fix review notes
This commit is contained in:
Vladimir Glavnyy
2020-05-15 01:39:58 +07:00
committed by GitHub
parent c3faa83463
commit 424a473e1f
3 changed files with 83 additions and 63 deletions

View File

@@ -2854,6 +2854,76 @@ class CppGenerator : public BaseGenerator {
*code_ptr += " (void)padding" + NumToString((*id)++) + "__;";
}
void GenStructConstructor(const StructDef& struct_def) {
std::string arg_list;
std::string init_list;
int padding_id = 0;
auto first = struct_def.fields.vec.begin();
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
const auto &field = **it;
const auto &field_type = field.value.type;
if (IsArray(field_type)) {
first++;
continue;
}
const auto member_name = Name(field) + "_";
const auto arg_name = "_" + Name(field);
const auto arg_type =
GenTypeGet(field_type, " ", "const ", " &", true);
if (it != first) { arg_list += ", "; }
arg_list += arg_type;
arg_list += arg_name;
if (!IsArray(field_type)) {
if (it != first && init_list != "") { init_list += ",\n "; }
init_list += member_name;
if (IsScalar(field_type.base_type)) {
auto type = GenUnderlyingCast(field, false, arg_name);
init_list += "(flatbuffers::EndianScalar(" + type + "))";
} else {
init_list += "(" + arg_name + ")";
}
}
if (field.padding) {
GenPadding(field, &init_list, &padding_id, PaddingInitializer);
}
}
if (!arg_list.empty()) {
code_.SetValue("ARG_LIST", arg_list);
code_.SetValue("INIT_LIST", init_list);
if (!init_list.empty()) {
code_ += " {{STRUCT_NAME}}({{ARG_LIST}})";
code_ += " : {{INIT_LIST}} {";
} else {
code_ += " {{STRUCT_NAME}}({{ARG_LIST}}) {";
}
padding_id = 0;
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
const auto &field = **it;
const auto &field_type = field.value.type;
if (IsArray(field_type)) {
const auto &elem_type = field_type.VectorType();
(void)elem_type;
FLATBUFFERS_ASSERT(
(IsScalar(elem_type.base_type) || IsStruct(elem_type)) &&
"invalid declaration");
const auto &member = Name(field) + "_";
code_ +=
" std::memset(" + member + ", 0, sizeof(" + member + "));";
}
if (field.padding) {
std::string padding;
GenPadding(field, &padding, &padding_id, PaddingNoop);
code_ += padding;
}
}
code_ += " }";
}
}
// Generate an accessor struct with constructor for a flatbuffers struct.
void GenStruct(const StructDef &struct_def) {
// Generate an accessor struct, with private variables of the form:
@@ -2912,66 +2982,7 @@ class CppGenerator : public BaseGenerator {
// Generate a constructor that takes all fields as arguments,
// excluding arrays
std::string arg_list;
std::string init_list;
padding_id = 0;
auto first = struct_def.fields.vec.begin();
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
const auto &field = **it;
if (IsArray(field.value.type)) {
first++;
continue;
}
const auto member_name = Name(field) + "_";
const auto arg_name = "_" + Name(field);
const auto arg_type =
GenTypeGet(field.value.type, " ", "const ", " &", true);
if (it != first) { arg_list += ", "; }
arg_list += arg_type;
arg_list += arg_name;
if (!IsArray(field.value.type)) {
if (it != first && init_list != "") { init_list += ",\n "; }
init_list += member_name;
if (IsScalar(field.value.type.base_type)) {
auto type = GenUnderlyingCast(field, false, arg_name);
init_list += "(flatbuffers::EndianScalar(" + type + "))";
} else {
init_list += "(" + arg_name + ")";
}
}
if (field.padding) {
GenPadding(field, &init_list, &padding_id, PaddingInitializer);
}
}
if (!arg_list.empty()) {
code_.SetValue("ARG_LIST", arg_list);
code_.SetValue("INIT_LIST", init_list);
if (!init_list.empty()) {
code_ += " {{STRUCT_NAME}}({{ARG_LIST}})";
code_ += " : {{INIT_LIST}} {";
} else {
code_ += " {{STRUCT_NAME}}({{ARG_LIST}}) {";
}
padding_id = 0;
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
const auto &field = **it;
if (IsArray(field.value.type)) {
const auto &member = Name(field) + "_";
code_ +=
" std::memset(" + member + ", 0, sizeof(" + member + "));";
}
if (field.padding) {
std::string padding;
GenPadding(field, &padding, &padding_id, PaddingNoop);
code_ += padding;
}
}
code_ += " }";
}
GenStructConstructor(struct_def);
// Generate accessor methods of the form:
// type name() const { return flatbuffers::EndianScalar(name_); }

View File

@@ -677,9 +677,15 @@ CheckedError Parser::ParseField(StructDef &struct_def) {
Type type;
ECHECK(ParseType(type));
if (struct_def.fixed && !IsScalar(type.base_type) && !IsStruct(type) &&
!IsArray(type))
return Error("structs_ may contain only scalar or struct fields");
if (struct_def.fixed) {
auto valid = IsScalar(type.base_type) || IsStruct(type);
if (!valid && IsArray(type)) {
const auto& elem_type = type.VectorType();
valid |= IsScalar(elem_type.base_type) || IsStruct(elem_type);
}
if (!valid)
return Error("structs may contain only scalar or struct fields");
}
if (!struct_def.fixed && IsArray(type))
return Error("fixed-length array in table must be wrapped in struct");

View File

@@ -1623,6 +1623,9 @@ void ErrorTest() {
TestError("table X { Y:int; } root_type X; { Y:1.0 }", "float");
TestError("table X { Y:bool; } root_type X; { Y:1.0 }", "float");
TestError("enum X:bool { Y = true }", "must be integral");
// Array of non-scalar
TestError("table X { x:int; } struct Y { y:[X:2]; }",
"may contain only scalar or struct fields");
}
template<typename T>