From e581013e3d42af13d2fe37b0ac46a3fd43f3638c Mon Sep 17 00:00:00 2001 From: Casper Date: Mon, 25 Jan 2021 12:29:43 -0500 Subject: [PATCH] Refactor FieldDef to model presense as an enum rather than 2 bools. (#6420) * Define presence. * Migrate to IsRequired and IsOptional methods * moved stuff around * Removed optional and required bools from FieldDef * change assert to return error * Fix tests.cpp * MakeFieldPresence helper * fmt * old c++ compatibility stuff Co-authored-by: Casper Neo --- include/flatbuffers/idl.h | 36 ++++++-- src/idl_gen_cpp.cpp | 8 +- src/idl_gen_csharp.cpp | 2 +- src/idl_gen_fbs.cpp | 2 +- src/idl_gen_java.cpp | 4 +- src/idl_gen_json_schema.cpp | 2 +- src/idl_gen_kotlin.cpp | 2 +- src/idl_gen_lobster.cpp | 6 +- src/idl_gen_php.cpp | 4 +- src/idl_gen_rust.cpp | 64 +++++++-------- src/idl_gen_swift.cpp | 40 ++++----- src/idl_gen_ts.cpp | 12 +-- src/idl_parser.cpp | 159 ++++++++++++++++++------------------ tests/test.cpp | 2 +- 14 files changed, 183 insertions(+), 160 deletions(-) diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 74d8bdc95..1f549f935 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -291,12 +291,11 @@ struct Definition { struct FieldDef : public Definition { FieldDef() : deprecated(false), - required(false), key(false), shared(false), native_inline(false), flexbuffer(false), - optional(false), + presence(kDefault), nested_flatbuffer(NULL), padding(0) {} @@ -306,21 +305,46 @@ struct FieldDef : public Definition { bool Deserialize(Parser &parser, const reflection::Field *field); bool IsScalarOptional() const { - return IsScalar(value.type.base_type) && optional; + return IsScalar(value.type.base_type) && IsOptional(); + } + bool IsOptional() const { + return presence == kOptional; + } + bool IsRequired() const { + return presence == kRequired; + } + bool IsDefault() const { + return presence == kDefault; } Value value; bool deprecated; // Field is allowed to be present in old data, but can't be. // written in new data nor accessed in new code. - bool required; // Field must always be present. bool key; // Field functions as a key for creating sorted vectors. bool shared; // Field will be using string pooling (i.e. CreateSharedString) // as default serialization behavior if field is a string. bool native_inline; // Field will be defined inline (instead of as a pointer) // for native tables if field is a struct. bool flexbuffer; // This field contains FlexBuffer data. - bool optional; // If True, this field is Null (as opposed to default - // valued). + + enum Presence { + // Field must always be present. + kRequired, + // Non-presence should be signalled to and controlled by users. + kOptional, + // Non-presence is hidden from users. + // Implementations may omit writing default values. + kDefault, + }; + Presence static MakeFieldPresence(bool optional, bool required) { + // clang-format off + return required ? FieldDef::kRequired + : optional ? FieldDef::kOptional + : FieldDef::kDefault; + // clang-format on + } + Presence presence; + StructDef *nested_flatbuffer; // This field contains nested FlatBuffer data. size_t padding; // Bytes to always pad after this field. }; diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp index bd194b24a..5ac5f7c46 100644 --- a/src/idl_gen_cpp.cpp +++ b/src/idl_gen_cpp.cpp @@ -1870,7 +1870,7 @@ class CppGenerator : public BaseGenerator { void GenVerifyCall(const FieldDef &field, const char *prefix) { code_.SetValue("PRE", prefix); code_.SetValue("NAME", Name(field)); - code_.SetValue("REQUIRED", field.required ? "Required" : ""); + code_.SetValue("REQUIRED", field.IsRequired() ? "Required" : ""); code_.SetValue("SIZE", GenTypeSize(field.value.type)); code_.SetValue("OFFSET", GenFieldOffsetName(field)); if (IsScalar(field.value.type.base_type) || IsStruct(field.value.type)) { @@ -2339,7 +2339,7 @@ class CppGenerator : public BaseGenerator { for (auto it = struct_def.fields.vec.begin(); it != struct_def.fields.vec.end(); ++it) { const auto &field = **it; - if (!field.deprecated && field.required) { + if (!field.deprecated && field.IsRequired()) { code_.SetValue("FIELD_NAME", Name(field)); code_.SetValue("OFFSET_NAME", GenFieldOffsetName(field)); code_ += " fbb_.Required(o, {{STRUCT_NAME}}::{{OFFSET_NAME}});"; @@ -2684,7 +2684,7 @@ class CppGenerator : public BaseGenerator { // in _o->field before attempting to access it. If there isn't, // depending on set_empty_strings_to_null either set it to 0 or an empty // string. - if (!field.required) { + if (!field.IsRequired()) { auto empty_value = opts_.set_empty_strings_to_null ? "0" : "_fbb.CreateSharedString(\"\")"; @@ -2793,7 +2793,7 @@ class CppGenerator : public BaseGenerator { // If set_empty_vectors_to_null option is enabled, for optional fields, // check to see if there actually is any data in _o->field before // attempting to access it. - if (opts_.set_empty_vectors_to_null && !field.required) { + if (opts_.set_empty_vectors_to_null && !field.IsRequired()) { code = value + ".size() ? " + code + " : 0"; } break; diff --git a/src/idl_gen_csharp.cpp b/src/idl_gen_csharp.cpp index b4ad47249..a5423a235 100644 --- a/src/idl_gen_csharp.cpp +++ b/src/idl_gen_csharp.cpp @@ -1142,7 +1142,7 @@ class CSharpGenerator : public BaseGenerator { for (auto it = struct_def.fields.vec.begin(); it != struct_def.fields.vec.end(); ++it) { auto &field = **it; - if (!field.deprecated && field.required) { + if (!field.deprecated && field.IsRequired()) { code += " builder.Required(o, "; code += NumToString(field.value.offset); code += "); // " + field.name + "\n"; diff --git a/src/idl_gen_fbs.cpp b/src/idl_gen_fbs.cpp index e6c8a4a85..35c1a7d4f 100644 --- a/src/idl_gen_fbs.cpp +++ b/src/idl_gen_fbs.cpp @@ -136,7 +136,7 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) { GenComment(field.doc_comment, &schema, nullptr, " "); schema += " " + field.name + ":" + GenType(field.value.type); if (field.value.constant != "0") schema += " = " + field.value.constant; - if (field.required) schema += " (required)"; + if (field.IsRequired()) schema += " (required)"; schema += ";\n"; } } diff --git a/src/idl_gen_java.cpp b/src/idl_gen_java.cpp index e80f37965..9d1d35a7e 100644 --- a/src/idl_gen_java.cpp +++ b/src/idl_gen_java.cpp @@ -649,7 +649,7 @@ class JavaGenerator : public BaseGenerator { std::string src_cast = SourceCast(field.value.type); std::string method_start = " public " + - (field.required ? "" : GenNullableAnnotation(field.value.type)) + + (field.IsRequired() ? "" : GenNullableAnnotation(field.value.type)) + GenPureAnnotation(field.value.type) + type_name_dest + optional + " " + MakeCamel(field.name, false); std::string obj = "obj"; @@ -1095,7 +1095,7 @@ class JavaGenerator : public BaseGenerator { for (auto it = struct_def.fields.vec.begin(); it != struct_def.fields.vec.end(); ++it) { auto &field = **it; - if (!field.deprecated && field.required) { + if (!field.deprecated && field.IsRequired()) { code += " builder.required(o, "; code += NumToString(field.value.offset); code += "); // " + field.name + "\n"; diff --git a/src/idl_gen_json_schema.cpp b/src/idl_gen_json_schema.cpp index f8e70bf66..55192c92c 100644 --- a/src/idl_gen_json_schema.cpp +++ b/src/idl_gen_json_schema.cpp @@ -239,7 +239,7 @@ class JsonSchemaGenerator : public BaseGenerator { std::vector requiredProperties; std::copy_if(properties.begin(), properties.end(), back_inserter(requiredProperties), - [](FieldDef const *prop) { return prop->required; }); + [](FieldDef const *prop) { return prop->IsRequired(); }); if (!requiredProperties.empty()) { auto required_string(Indent(3) + "\"required\" : ["); for (auto req_prop = requiredProperties.cbegin(); diff --git a/src/idl_gen_kotlin.cpp b/src/idl_gen_kotlin.cpp index ed85b2cba..4c9219585 100644 --- a/src/idl_gen_kotlin.cpp +++ b/src/idl_gen_kotlin.cpp @@ -646,7 +646,7 @@ class KotlinGenerator : public BaseGenerator { writer.IncrementIdentLevel(); for (auto it = field_vec.begin(); it != field_vec.end(); ++it) { auto &field = **it; - if (field.deprecated || !field.required) { continue; } + if (field.deprecated || !field.IsRequired()) { continue; } writer.SetValue("offset", NumToString(field.value.offset)); writer += "builder.required(o, {{offset}})"; } diff --git a/src/idl_gen_lobster.cpp b/src/idl_gen_lobster.cpp index 4ebf8cad6..6fdd6dc26 100644 --- a/src/idl_gen_lobster.cpp +++ b/src/idl_gen_lobster.cpp @@ -110,13 +110,13 @@ class LobsterGenerator : public BaseGenerator { offsets + ")"; } else { - auto defval = field.optional ? "0" : field.value.constant; + auto defval = field.IsOptional() ? "0" : field.value.constant; acc = "buf_.flatbuffers_field_" + GenTypeName(field.value.type) + "(pos_, " + offsets + ", " + defval + ")"; } if (field.value.type.enum_def) acc = NormalizedName(*field.value.type.enum_def) + "(" + acc + ")"; - if (field.optional) + if (field.IsOptional()) acc += ", buf_.flatbuffers_field_present(pos_, " + offsets + ")"; code += def + "():\n return " + acc + "\n"; return; @@ -201,7 +201,7 @@ class LobsterGenerator : public BaseGenerator { NormalizedName(field) + ":" + LobsterType(field.value.type) + "):\n b_.Prepend" + GenMethod(field.value.type) + "Slot(" + NumToString(offset) + ", " + NormalizedName(field); - if (IsScalar(field.value.type.base_type) && !field.optional) + if (IsScalar(field.value.type.base_type) && !field.IsOptional()) code += ", " + field.value.constant; code += ")\n return this\n"; } diff --git a/src/idl_gen_php.cpp b/src/idl_gen_php.cpp index b763582a1..602d229ee 100644 --- a/src/idl_gen_php.cpp +++ b/src/idl_gen_php.cpp @@ -536,7 +536,7 @@ class PhpGenerator : public BaseGenerator { for (auto it = struct_def.fields.vec.begin(); it != struct_def.fields.vec.end(); ++it) { auto &field = **it; - if (!field.deprecated && field.required) { + if (!field.deprecated && field.IsRequired()) { code += Indent + Indent + "$builder->required($o, "; code += NumToString(field.value.offset); code += "); // " + field.name + "\n"; @@ -645,7 +645,7 @@ class PhpGenerator : public BaseGenerator { for (auto it = struct_def.fields.vec.begin(); it != struct_def.fields.vec.end(); ++it) { auto &field = **it; - if (!field.deprecated && field.required) { + if (!field.deprecated && field.IsRequired()) { code += Indent + Indent + "$builder->required($o, "; code += NumToString(field.value.offset); code += "); // " + field.name + "\n"; diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index f9316ee81..0bd279968 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -887,15 +887,15 @@ class RustGenerator : public BaseGenerator { switch (GetFullType(field.value.type)) { case ftInteger: case ftFloat: { - return field.optional ? "None" : field.value.constant; + return field.IsOptional() ? "None" : field.value.constant; } case ftBool: { - return field.optional ? "None" + return field.IsOptional() ? "None" : field.value.constant == "0" ? "false" : "true"; } case ftUnionKey: case ftEnumKey: { - if (field.optional) { return "None"; } + if (field.IsOptional()) { return "None"; } auto ev = field.value.type.enum_def->FindByValue(field.value.constant); if (!ev) return "Default::default()"; // Bitflags enum. return WrapInNameSpace(field.value.type.enum_def->defined_namespace, @@ -930,7 +930,7 @@ class RustGenerator : public BaseGenerator { case ftFloat: case ftBool: { const auto typname = GetTypeBasic(type); - return field.optional ? "Option<" + typname + ">" : typname; + return field.IsOptional() ? "Option<" + typname + ">" : typname; } case ftStruct: { const auto typname = WrapInNameSpace(*type.struct_def); @@ -947,7 +947,7 @@ class RustGenerator : public BaseGenerator { case ftEnumKey: case ftUnionKey: { const auto typname = WrapInNameSpace(*type.enum_def); - return field.optional ? "Option<" + typname + ">" : typname; + return field.IsOptional() ? "Option<" + typname + ">" : typname; } case ftUnionValue: { return "Option>"; @@ -1056,7 +1056,7 @@ class RustGenerator : public BaseGenerator { } } if (in_a_table && !IsUnion(type) && - (IsScalar(type.base_type) ? field.optional : !field.required)) { + (IsScalar(type.base_type) ? field.IsOptional() : !field.IsRequired())) { return "Option<" + ty + ">"; } else { return ty; @@ -1137,14 +1137,14 @@ class RustGenerator : public BaseGenerator { case ftBool: case ftFloat: { const auto typname = GetTypeBasic(field.value.type); - return (field.optional ? "self.fbb_.push_slot_always::<" + return (field.IsOptional() ? "self.fbb_.push_slot_always::<" : "self.fbb_.push_slot::<") + typname + ">"; } case ftEnumKey: case ftUnionKey: { const auto underlying_typname = GetTypeBasic(type); - return (field.optional ? "self.fbb_.push_slot_always::<" + return (field.IsOptional() ? "self.fbb_.push_slot_always::<" : "self.fbb_.push_slot::<") + underlying_typname + ">"; } @@ -1184,31 +1184,31 @@ class RustGenerator : public BaseGenerator { case ftFloat: case ftBool: { const auto typname = GetTypeBasic(type); - return field.optional ? "Option<" + typname + ">" : typname; + return field.IsOptional() ? "Option<" + typname + ">" : typname; } case ftStruct: { const auto typname = WrapInNameSpace(*type.struct_def); return WrapInOptionIfNotRequired("&" + lifetime + " " + typname, - field.required); + field.IsRequired()); } case ftTable: { const auto typname = WrapInNameSpace(*type.struct_def); return WrapInOptionIfNotRequired(typname + "<" + lifetime + ">", - field.required); + field.IsRequired()); } case ftEnumKey: case ftUnionKey: { const auto typname = WrapInNameSpace(*type.enum_def); - return field.optional ? "Option<" + typname + ">" : typname; + return field.IsOptional() ? "Option<" + typname + ">" : typname; } case ftUnionValue: { return WrapInOptionIfNotRequired("flatbuffers::Table<" + lifetime + ">", - field.required); + field.IsRequired()); } case ftString: { return WrapInOptionIfNotRequired("&" + lifetime + " str", - field.required); + field.IsRequired()); } case ftVectorOfInteger: case ftVectorOfBool: @@ -1216,35 +1216,35 @@ class RustGenerator : public BaseGenerator { const auto typname = GetTypeBasic(type.VectorType()); if (IsOneByte(type.VectorType().base_type)) { return WrapInOptionIfNotRequired( - "&" + lifetime + " [" + typname + "]", field.required); + "&" + lifetime + " [" + typname + "]", field.IsRequired()); } return WrapInOptionIfNotRequired( "flatbuffers::Vector<" + lifetime + ", " + typname + ">", - field.required); + field.IsRequired()); } case ftVectorOfEnumKey: { const auto typname = WrapInNameSpace(*type.enum_def); return WrapInOptionIfNotRequired( "flatbuffers::Vector<" + lifetime + ", " + typname + ">", - field.required); + field.IsRequired()); } case ftVectorOfStruct: { const auto typname = WrapInNameSpace(*type.struct_def); return WrapInOptionIfNotRequired("&" + lifetime + " [" + typname + "]", - field.required); + field.IsRequired()); } case ftVectorOfTable: { const auto typname = WrapInNameSpace(*type.struct_def); return WrapInOptionIfNotRequired("flatbuffers::Vector<" + lifetime + ", flatbuffers::ForwardsUOffset<" + typname + "<" + lifetime + ">>>", - field.required); + field.IsRequired()); } case ftVectorOfString: { return WrapInOptionIfNotRequired( "flatbuffers::Vector<" + lifetime + ", flatbuffers::ForwardsUOffset<&" + lifetime + " str>>", - field.required); + field.IsRequired()); } case ftVectorOfUnionValue: { FLATBUFFERS_ASSERT(false && "vectors of unions are not yet supported"); @@ -1324,10 +1324,10 @@ class RustGenerator : public BaseGenerator { const std::string typname = FollowType(field.value.type, lifetime); // Default-y fields (scalars so far) are neither optional nor required. const std::string default_value = - !(field.optional || field.required) + !(field.IsOptional() || field.IsRequired()) ? "Some(" + GetDefaultScalarValue(field) + ")" : "None"; - const std::string unwrap = field.optional ? "" : ".unwrap()"; + const std::string unwrap = field.IsOptional() ? "" : ".unwrap()"; const auto t = GetFullType(field.value.type); @@ -1345,7 +1345,7 @@ class RustGenerator : public BaseGenerator { } bool TableFieldReturnsOption(const FieldDef &field) { - if (field.optional) return true; + if (field.IsOptional()) return true; switch (GetFullType(field.value.type)) { case ftInteger: case ftFloat: @@ -1571,7 +1571,7 @@ class RustGenerator : public BaseGenerator { return; } } - if (field.optional) { + if (field.IsOptional()) { code_ += " let {{FIELD_NAME}} = self.{{FIELD_NAME}}().map(|x| {"; code_ += " {{EXPR}}"; code_ += " });"; @@ -1638,7 +1638,7 @@ class RustGenerator : public BaseGenerator { code_.SetValue("NESTED", WrapInNameSpace(*nested_root)); code_ += " pub fn {{FIELD_NAME}}_nested_flatbuffer(&'a self) -> \\"; - if (field.required) { + if (field.IsRequired()) { code_ += "{{NESTED}}<'a> {"; code_ += " let data = self.{{FIELD_NAME}}();"; code_ += " use flatbuffers::Follow;"; @@ -1687,7 +1687,7 @@ class RustGenerator : public BaseGenerator { // The following logic is not tested in the integration test, // as of April 10, 2020 - if (field.required) { + if (field.IsRequired()) { code_ += " let u = self.{{FIELD_NAME}}();"; code_ += " Some({{U_ELEMENT_TABLE_TYPE}}::init_from_table(u))"; @@ -1719,7 +1719,7 @@ class RustGenerator : public BaseGenerator { ForAllTableFields(struct_def, [&](const FieldDef &field) { if (GetFullType(field.value.type) == ftUnionKey) return; - code_.SetValue("IS_REQ", field.required ? "true" : "false"); + code_.SetValue("IS_REQ", field.IsRequired() ? "true" : "false"); if (GetFullType(field.value.type) != ftUnionValue) { // All types besides unions. code_.SetValue("TY", FollowType(field.value.type, "'_")); @@ -1770,7 +1770,7 @@ class RustGenerator : public BaseGenerator { code_ += " {{STRUCT_NAME}}Args {"; ForAllTableFields(struct_def, [&](const FieldDef &field) { code_ += " {{FIELD_NAME}}: {{DEFAULT_VALUE}},\\"; - code_ += field.required ? " // required field" : ""; + code_ += field.IsRequired() ? " // required field" : ""; }); code_ += " }"; code_ += " }"; @@ -1807,7 +1807,7 @@ class RustGenerator : public BaseGenerator { code_ += " pub fn add_{{FIELD_NAME}}(&mut self, {{FIELD_NAME}}: " "{{FIELD_TYPE}}) {"; - if (is_scalar && !field.optional) { + if (is_scalar && !field.IsOptional()) { code_ += " {{FUNC_BODY}}({{FIELD_OFFSET}}, {{FIELD_NAME}}, " "{{DEFAULT_VALUE}});"; @@ -1838,7 +1838,7 @@ class RustGenerator : public BaseGenerator { code_ += " let o = self.fbb_.end_table(self.start_);"; ForAllTableFields(struct_def, [&](const FieldDef &field) { - if (!field.required) return; + if (!field.IsRequired()) return; code_ += " self.fbb_.required(o, {{STRUCT_NAME}}::{{OFFSET_NAME}}," "\"{{FIELD_NAME}}\");"; @@ -1948,7 +1948,7 @@ class RustGenerator : public BaseGenerator { } case ftStruct: { // Hold the struct in a variable so we can reference it. - if (field.required) { + if (field.IsRequired()) { code_ += " let {{FIELD_NAME}}_tmp = " "Some(self.{{FIELD_NAME}}.pack());"; @@ -2023,7 +2023,7 @@ class RustGenerator : public BaseGenerator { } } void MapNativeTableField(const FieldDef &field, const std::string &expr) { - if (field.required) { + if (field.IsRequired()) { // For some reason Args has optional types for required fields. // TODO(cneo): Fix this... but its a breaking change? code_ += " let {{FIELD_NAME}} = Some({"; diff --git a/src/idl_gen_swift.cpp b/src/idl_gen_swift.cpp index 8fcd00b81..347bb1353 100644 --- a/src/idl_gen_swift.cpp +++ b/src/idl_gen_swift.cpp @@ -460,7 +460,7 @@ class SwiftGenerator : public BaseGenerator { auto &field = **it; if (field.deprecated) continue; if (field.key) key_field = &field; - if (field.required) + if (field.IsRequired()) require_fields.push_back(NumToString(field.value.offset)); GenTableWriterFields(field, &create_func_body, &create_func_header); @@ -537,7 +537,7 @@ class SwiftGenerator : public BaseGenerator { auto &create_func_header = *create_header; auto name = Name(field); auto type = GenType(field.value.type); - auto opt_scalar = field.optional && IsScalar(field.value.type.base_type); + auto opt_scalar = field.IsOptional() && IsScalar(field.value.type.base_type); auto nullable_type = opt_scalar ? type + "?" : type; code_.SetValue("VALUENAME", name); code_.SetValue("VALUETYPE", nullable_type); @@ -559,17 +559,17 @@ class SwiftGenerator : public BaseGenerator { code_ += "{{VALUETYPE}}" + builder_string + "fbb.add(element: {{VALUENAME}}\\"; - code_ += field.optional ? (optional_enum + "\\") + code_ += field.IsOptional() ? (optional_enum + "\\") : (is_enum + ", def: {{CONSTANT}}\\"); code_ += ", at: {{TABLEOFFSET}}.{{OFFSET}}.p) }"; auto default_value = IsEnum(field.value.type) - ? (field.optional ? "nil" : GenEnumDefaultValue(field)) + ? (field.IsOptional() ? "nil" : GenEnumDefaultValue(field)) : field.value.constant; create_func_header.push_back("" + name + ": " + nullable_type + " = " + - (field.optional ? "nil" : default_value)); + (field.IsOptional() ? "nil" : default_value)); return; } @@ -578,13 +578,13 @@ class SwiftGenerator : public BaseGenerator { "0" == field.value.constant ? "false" : "true"; code_.SetValue("CONSTANT", default_value); - code_.SetValue("VALUETYPE", field.optional ? "Bool?" : "Bool"); + code_.SetValue("VALUETYPE", field.IsOptional() ? "Bool?" : "Bool"); code_ += "{{VALUETYPE}}" + builder_string + "fbb.add(element: {{VALUENAME}},\\"; - code_ += field.optional ? "\\" : " def: {{CONSTANT}},"; + code_ += field.IsOptional() ? "\\" : " def: {{CONSTANT}},"; code_ += " at: {{TABLEOFFSET}}.{{OFFSET}}.p) }"; create_func_header.push_back(name + ": " + nullable_type + " = " + - (field.optional ? "nil" : default_value)); + (field.IsOptional() ? "nil" : default_value)); return; } @@ -647,7 +647,7 @@ class SwiftGenerator : public BaseGenerator { code_.SetValue("VALUETYPE", type); code_.SetValue("OFFSET", name); code_.SetValue("CONSTANT", field.value.constant); - bool opt_scalar = field.optional && IsScalar(field.value.type.base_type); + bool opt_scalar = field.IsOptional() && IsScalar(field.value.type.base_type); std::string def_Val = opt_scalar ? "nil" : "{{CONSTANT}}"; std::string optional = opt_scalar ? "?" : ""; auto const_string = "return o == 0 ? " + def_Val + " : "; @@ -674,7 +674,7 @@ class SwiftGenerator : public BaseGenerator { } if (IsEnum(field.value.type)) { - auto default_value = field.optional ? "nil" : GenEnumDefaultValue(field); + auto default_value = field.IsOptional() ? "nil" : GenEnumDefaultValue(field); code_.SetValue("BASEVALUE", GenTypeBasic(field.value.type, false)); code_ += GenReaderMainBody(optional) + "\\"; code_ += GenOffset() + "return o == 0 ? " + default_value + " : " + @@ -684,8 +684,8 @@ class SwiftGenerator : public BaseGenerator { return; } - std::string is_required = field.required ? "!" : "?"; - auto required_reader = field.required ? "return " : const_string; + std::string is_required = field.IsRequired() ? "!" : "?"; + auto required_reader = field.IsRequired() ? "return " : const_string; if (IsStruct(field.value.type) && field.value.type.struct_def->fixed) { code_.SetValue("VALUETYPE", GenType(field.value.type)); @@ -1024,7 +1024,7 @@ class SwiftGenerator : public BaseGenerator { case BASE_TYPE_STRING: { unpack_body.push_back("{{STRUCTNAME}}." + body + "__" + name + builder); - if (field.required) { + if (field.IsRequired()) { code_ += "let __" + name + " = builder.create(string: obj." + name + ")"; } else { @@ -1145,7 +1145,7 @@ class SwiftGenerator : public BaseGenerator { auto type = GenType(field.value.type); code_.SetValue("VALUENAME", name); code_.SetValue("VALUETYPE", type); - std::string is_required = field.required ? "" : "?"; + std::string is_required = field.IsRequired() ? "" : "?"; switch (field.value.type.base_type) { case BASE_TYPE_STRUCT: { @@ -1154,7 +1154,7 @@ class SwiftGenerator : public BaseGenerator { auto optional = (field.value.type.struct_def && field.value.type.struct_def->fixed); std::string question_mark = - (field.required || (optional && is_fixed) ? "" : "?"); + (field.IsRequired() || (optional && is_fixed) ? "" : "?"); code_ += "{{ACCESS_TYPE}} var {{VALUENAME}}: {{VALUETYPE}}" + question_mark; @@ -1165,7 +1165,7 @@ class SwiftGenerator : public BaseGenerator { } else { buffer_constructor.push_back("var __" + name + " = _t." + name); buffer_constructor.push_back("" + name + " = __" + name + - (field.required ? "!" : question_mark) + + (field.IsRequired() ? "!" : question_mark) + ".unpack()"); } break; @@ -1179,7 +1179,7 @@ class SwiftGenerator : public BaseGenerator { case BASE_TYPE_STRING: { code_ += "{{ACCESS_TYPE}} var {{VALUENAME}}: String" + is_required; buffer_constructor.push_back(name + " = _t." + name); - if (field.required) base_constructor.push_back(name + " = \"\""); + if (field.IsRequired()) base_constructor.push_back(name + " = \"\""); break; } case BASE_TYPE_UTYPE: break; @@ -1190,12 +1190,12 @@ class SwiftGenerator : public BaseGenerator { } default: { buffer_constructor.push_back(name + " = _t." + name); - std::string nullable = field.optional ? "?" : ""; + std::string nullable = field.IsOptional() ? "?" : ""; if (IsScalar(field.value.type.base_type) && !IsBool(field.value.type.base_type) && !IsEnum(field.value.type)) { code_ += "{{ACCESS_TYPE}} var {{VALUENAME}}: {{VALUETYPE}}" + nullable; - if (!field.optional) + if (!field.IsOptional()) base_constructor.push_back(name + " = " + field.value.constant); break; } @@ -1213,7 +1213,7 @@ class SwiftGenerator : public BaseGenerator { code_ += "{{ACCESS_TYPE}} var {{VALUENAME}}: Bool" + nullable; std::string default_value = "0" == field.value.constant ? "false" : "true"; - if (!field.optional) + if (!field.IsOptional()) base_constructor.push_back(name + " = " + default_value); } } diff --git a/src/idl_gen_ts.cpp b/src/idl_gen_ts.cpp index 37b6d2727..915b05e41 100644 --- a/src/idl_gen_ts.cpp +++ b/src/idl_gen_ts.cpp @@ -355,7 +355,7 @@ class TsGenerator : public BaseGenerator { } else { *arguments += ", " + nameprefix + field.name + ": " + - GenTypeName(imports, field, field.value.type, true, field.optional); + GenTypeName(imports, field, field.value.type, true, field.IsOptional()); } } } @@ -1141,7 +1141,7 @@ class TsGenerator : public BaseGenerator { if (field.value.type.enum_def) { code += "):" + GenTypeName(imports, struct_def, field.value.type, false, - field.optional) + + field.IsOptional()) + " {\n"; } else { code += "):" + @@ -1488,8 +1488,8 @@ class TsGenerator : public BaseGenerator { for (auto it = struct_def.fields.vec.begin(); it != struct_def.fields.vec.end(); ++it) { auto &field = **it; - if (!field.deprecated && field.required) { - code += " builder.requiredField(offset, "; + if (!field.deprecated && field.IsRequired()) { + code += " builder.IsRequired()Field(offset, "; code += NumToString(field.value.offset); code += ") // " + field.name + "\n"; } @@ -1566,13 +1566,13 @@ class TsGenerator : public BaseGenerator { } static bool HasNullDefault(const FieldDef &field) { - return field.optional && field.value.constant == "null"; + return field.IsOptional() && field.value.constant == "null"; } std::string GetArgType(import_set &imports, const Definition &owner, const FieldDef &field, bool allowNull) { return GenTypeName(imports, owner, field.value.type, true, - allowNull && field.optional); + allowNull && field.IsOptional()); } static std::string GetArgName(const FieldDef &field) { diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index aeb452bda..7cdddaffd 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -800,34 +800,6 @@ CheckedError Parser::ParseField(StructDef &struct_def) { "default values currently only supported for scalars in tables"); } - // Mark the optional scalars. Note that a side effect of ParseSingleValue is - // fixing field->value.constant to null. - if (IsScalar(type.base_type)) { - field->optional = (field->value.constant == "null"); - if (field->optional) { - if (type.enum_def && type.enum_def->Lookup("null")) { - FLATBUFFERS_ASSERT(IsInteger(type.base_type)); - return Error( - "the default 'null' is reserved for declaring optional scalar " - "fields, it conflicts with declaration of enum '" + - type.enum_def->name + "'."); - } - if (field->attributes.Lookup("key")) { - return Error( - "only a non-optional scalar field can be used as a 'key' field"); - } - if (!SupportsOptionalScalars()) { - return Error( - "Optional scalars are not yet supported in at least one the of " - "the specified programming languages."); - } - } - } else { - // For nonscalars, only required fields are non-optional. - // At least until https://github.com/google/flatbuffers/issues/6053 - field->optional = !field->required; - } - // Append .0 if the value has not it (skip hex and scientific floats). // This suffix needed for generated C++ code. if (IsFloat(type.base_type)) { @@ -843,27 +815,6 @@ CheckedError Parser::ParseField(StructDef &struct_def) { field->value.constant += ".0"; } } - if (type.enum_def) { - // The type.base_type can only be scalar, union, array or vector. - // Table, struct or string can't have enum_def. - // Default value of union and vector in NONE, NULL translated to "0". - FLATBUFFERS_ASSERT(IsInteger(type.base_type) || - (type.base_type == BASE_TYPE_UNION) || IsVector(type) || - IsArray(type)); - if (IsVector(type)) { - // Vector can't use initialization list. - FLATBUFFERS_ASSERT(field->value.constant == "0"); - } else { - // All unions should have the NONE ("0") enum value. - auto in_enum = type.enum_def->attributes.Lookup("bit_flags") || - field->IsScalarOptional() || - type.enum_def->FindByValue(field->value.constant); - if (false == in_enum) - return Error("default value of " + field->value.constant + - " for field " + name + " is not part of enum " + - type.enum_def->name); - } - } field->doc_comment = dc; ECHECK(ParseMetaData(&field->attributes)); @@ -898,6 +849,75 @@ CheckedError Parser::ParseField(StructDef &struct_def) { "hashing."); } } + + // For historical convenience reasons, string keys are assumed required. + // Scalars are kDefault unless otherwise specified. + // Nonscalars are kOptional unless required; + field->key = field->attributes.Lookup("key") != nullptr; + const bool required = field->attributes.Lookup("required") != nullptr || + (IsString(type) && field->key); + const bool optional = + IsScalar(type.base_type) ? (field->value.constant == "null") : !required; + if (required && optional) { + return Error("Fields cannot be both optional and required."); + } + field->presence = FieldDef::MakeFieldPresence(optional, required); + + if (required && (struct_def.fixed || IsScalar(type.base_type))) { + return Error("only non-scalar fields in tables may be 'required'"); + } + if (field->key) { + if (struct_def.has_key) return Error("only one field may be set as 'key'"); + struct_def.has_key = true; + if (!IsScalar(type.base_type) && !IsString(type)) { + return Error("'key' field must be string or scalar type"); + } + } + + if (field->IsScalarOptional()) { + if (type.enum_def && type.enum_def->Lookup("null")) { + FLATBUFFERS_ASSERT(IsInteger(type.base_type)); + return Error( + "the default 'null' is reserved for declaring optional scalar " + "fields, it conflicts with declaration of enum '" + + type.enum_def->name + "'."); + } + if (field->attributes.Lookup("key")) { + return Error( + "only a non-optional scalar field can be used as a 'key' field"); + } + if (!SupportsOptionalScalars()) { + return Error( + "Optional scalars are not yet supported in at least one the of " + "the specified programming languages."); + } + } + + if (type.enum_def) { + // The type.base_type can only be scalar, union, array or vector. + // Table, struct or string can't have enum_def. + // Default value of union and vector in NONE, NULL translated to "0". + FLATBUFFERS_ASSERT(IsInteger(type.base_type) || + (type.base_type == BASE_TYPE_UNION) || IsVector(type) || + IsArray(type)); + if (IsVector(type)) { + // Vector can't use initialization list. + FLATBUFFERS_ASSERT(field->value.constant == "0"); + } else { + // All unions should have the NONE ("0") enum value. + auto in_enum = field->IsOptional() || + type.enum_def->attributes.Lookup("bit_flags") || + type.enum_def->FindByValue(field->value.constant); + if (false == in_enum) + return Error("default value of " + field->value.constant + + " for field " + name + " is not part of enum " + + type.enum_def->name); + } + } + + if (field->deprecated && struct_def.fixed) + return Error("can't deprecate fields in a struct"); + auto cpp_type = field->attributes.Lookup("cpp_type"); if (cpp_type) { if (!hash_name) @@ -911,29 +931,7 @@ CheckedError Parser::ParseField(StructDef &struct_def) { field->attributes.Add("cpp_ptr_type", val); } } - if (field->deprecated && struct_def.fixed) - return Error("can't deprecate fields in a struct"); - field->required = field->attributes.Lookup("required") != nullptr; - if (field->required && (struct_def.fixed || IsScalar(type.base_type))) - return Error("only non-scalar fields in tables may be 'required'"); - if (!IsScalar(type.base_type)) { - // For nonscalars, only required fields are non-optional. - // At least until https://github.com/google/flatbuffers/issues/6053 - field->optional = !field->required; - } - - field->key = field->attributes.Lookup("key") != nullptr; - if (field->key) { - if (struct_def.has_key) return Error("only one field may be set as 'key'"); - struct_def.has_key = true; - if (!IsScalar(type.base_type)) { - field->required = true; - field->optional = false; - if (type.base_type != BASE_TYPE_STRING) - return Error("'key' field must be string or scalar type"); - } - } field->shared = field->attributes.Lookup("shared") != nullptr; if (field->shared && field->value.type.base_type != BASE_TYPE_STRING) return Error("shared can only be defined on strings"); @@ -972,7 +970,7 @@ CheckedError Parser::ParseField(StructDef &struct_def) { if (typefield) { if (!IsScalar(typefield->value.type.base_type)) { // this is a union vector field - typefield->required = field->required; + typefield->presence = field->presence; } // If this field is a union, and it has a manually assigned id, // the automatically added type field should have an id as well (of N - 1). @@ -1267,7 +1265,7 @@ CheckedError Parser::ParseTable(const StructDef &struct_def, std::string *value, for (auto field_it = struct_def.fields.vec.begin(); field_it != struct_def.fields.vec.end(); ++field_it) { auto required_field = *field_it; - if (!required_field->required) { continue; } + if (!required_field->IsRequired()) { continue; } bool found = false; for (auto pf_it = field_stack_.end() - fieldn_outer; pf_it != field_stack_.end(); ++pf_it) { @@ -1299,7 +1297,7 @@ CheckedError Parser::ParseTable(const StructDef &struct_def, std::string *value, if (!struct_def.sortbysize || size == SizeOf(field_value.type.base_type)) { switch (field_value.type.base_type) { -// clang-format off + // clang-format off #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE, ...) \ case BASE_TYPE_ ## ENUM: \ builder_.Pad(field->padding); \ @@ -1485,7 +1483,7 @@ CheckedError Parser::ParseVector(const Type &type, uoffset_t *ovalue, // start at the back, since we're building the data backwards. auto &val = field_stack_.back().first; switch (val.type.base_type) { -// clang-format off + // clang-format off #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE,...) \ case BASE_TYPE_ ## ENUM: \ if (IsStruct(val.type)) SerializeStruct(*val.type.struct_def, val); \ @@ -2792,7 +2790,9 @@ CheckedError Parser::ParseProtoFields(StructDef *struct_def, bool isextend, } if (!field) ECHECK(AddField(*struct_def, name, type, &field)); field->doc_comment = field_comment; - if (!IsScalar(type.base_type)) field->required = required; + if (!IsScalar(type.base_type) && required) { + field->presence = FieldDef::kRequired; + } // See if there's a default specified. if (Is('[')) { NEXT(); @@ -3497,8 +3497,8 @@ Offset FieldDef::Serialize(FlatBufferBuilder *builder, // Is uint64>max(int64) tested? IsInteger(value.type.base_type) ? StringToInt(value.constant.c_str()) : 0, // result may be platform-dependent if underlying is float (not double) - IsFloat(value.type.base_type) ? d : 0.0, deprecated, required, key, - attr__, docs__, optional); + IsFloat(value.type.base_type) ? d : 0.0, deprecated, IsRequired(), key, + attr__, docs__, IsOptional()); // TODO: value.constant is almost always "0", we could save quite a bit of // space by sharing it. Same for common values of value.type. } @@ -3513,8 +3513,7 @@ bool FieldDef::Deserialize(Parser &parser, const reflection::Field *field) { } else if (IsFloat(value.type.base_type)) { value.constant = FloatToString(field->default_real(), 16); } - deprecated = field->deprecated(); - required = field->required(); + presence = FieldDef::MakeFieldPresence(field->optional(), field->required()); key = field->key(); if (!DeserializeAttributes(parser, field->attributes())) return false; // TODO: this should probably be handled by a separate attribute diff --git a/tests/test.cpp b/tests/test.cpp index d57b29855..d070b9d04 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -3654,7 +3654,7 @@ void OptionalScalarsTest() { flatbuffers::Parser parser; TEST_ASSERT(parser.Parse(schema->c_str())); const auto *mana = parser.structs_.Lookup("Monster")->fields.Lookup("mana"); - TEST_EQ(mana->optional, has_null); + TEST_EQ(mana->IsOptional(), has_null); } // Test if nullable scalars are allowed for each language.