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 <cneo@google.com>

diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h
index 74d8bdc..1f549f9 100644 (file)
--- 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 bd194b2..5ac5f7c 100644 (file)
--- 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 b4ad472..a5423a2 100644 (file)
--- 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 e6c8a4a..35c1a7d 100644 (file)
--- 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 e80f379..9d1d35a 100644 (file)
--- 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 f8e70bf..55192c9 100644 (file)
--- a/src/idl_gen_json_schema.cpp
+++ b/src/idl_gen_json_schema.cpp
@@ -239,7 +239,7 @@ class JsonSchemaGenerator : public BaseGenerator {
       std::vector<FieldDef *> 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 ed85b2c..4c92195 100644 (file)
--- 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 4ebf8ca..6fdd6dc 100644 (file)
--- 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 b763582..602d229 100644 (file)
--- 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 f9316ee..0bd2799 100644 (file)
--- 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<flatbuffers::WIPOffset<flatbuffers::UnionWIPOffset>>";
@@ -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 8fcd00b..347bb13 100644 (file)
--- 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 37b6d27..915b05e 100644 (file)
--- 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 aeb452b..7cdddaf 100644 (file)
--- 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<reflection::Field> 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 d57b298..d070b9d 100644 (file)
--- 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.