Default strings and vectors: Parser + Rust support

include/flatbuffers/idl.h

@@ -337,6 +337,7 @@ struct FieldDef : public Definition {
     kDefault,
   };
   Presence static MakeFieldPresence(bool optional, bool required) {
+    FLATBUFFERS_ASSERT(!(required && optional));
     // clang-format off
     return required ? FieldDef::kRequired
          : optional ? FieldDef::kOptional
@@ -971,6 +972,7 @@ class Parser : public ParserState {
   bool SupportsAdvancedUnionFeatures() const;
   bool SupportsAdvancedArrayFeatures() const;
   bool SupportsOptionalScalars() const;
+  bool SupportsDefaultVectorsAndStrings() const;
   Namespace *UniqueNamespace(Namespace *ns);
 
   FLATBUFFERS_CHECKED_ERROR RecurseError();

rust/flatbuffers/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "flatbuffers"
-version = "0.8.2"
+version = "0.8.3"
 edition = "2018"
 authors = ["Robert Winslow <[email protected]>", "FlatBuffers Maintainers"]
 license = "Apache-2.0"

rust/flatbuffers/src/vector.rs

@@ -29,6 +29,14 @@ use crate::primitives::*;
 
 pub struct Vector<'a, T: 'a>(&'a [u8], usize, PhantomData<T>);
 
+impl<'a, T:'a> Default for Vector<'a, T> {
+    fn default() -> Self {
+        // Static, length 0 vector.
+        // Note that derived default causes UB due to issues in read_scalar_at /facepalm.
+        Self(&[0; core::mem::size_of::<UOffsetT>()], 0, Default::default())
+    }
+}
+
 impl<'a, T> Debug for Vector<'a, T>
 where
     T: 'a + Follow<'a>,

src/idl_gen_rust.cpp

@@ -183,6 +183,12 @@ bool IsBitFlagsEnum(const FieldDef &field) {
   return ed && IsBitFlagsEnum(*ed);
 }
 
+// TableArgs make required non-scalars "Option<_>".
+// TODO(cneo): Rework how we do defaults and stuff.
+bool IsOptionalToBuilder(const FieldDef &field) {
+  return field.IsOptional() || !IsScalar(field.value.type.base_type);
+}
+
 namespace rust {
 
 class RustGenerator : public BaseGenerator {
@@ -883,8 +889,10 @@ class RustGenerator : public BaseGenerator {
     return "VT_" + MakeUpper(Name(field));
   }
 
-  std::string GetDefaultValue(const FieldDef &field, bool for_builder) {
-    if (for_builder) {
+  enum DefaultContext { kBuilder, kAccessor, kObject };
+  std::string GetDefaultValue(const FieldDef &field,
+                              const DefaultContext context) {
+    if (context == kBuilder) {
       // Builders and Args structs model nonscalars "optional" even if they're
       // required or have defaults according to the schema. I guess its because
       // WIPOffset is not nullable.
@@ -915,24 +923,36 @@ class RustGenerator : public BaseGenerator {
         return ObjectFieldType(field, true) + "::NONE";
       }
       case ftString: {
-        // Required strings.
-        return "String::new()";  // No default strings yet.
+        // Required fields do not have defaults defined by the schema, but we
+        // need one for Rust's Default trait so we use empty string. The usual
+        // value of field.value.constant is `0`, which is non-sensical except
+        // maybe to c++ (nullptr == 0).
+        // TODO: Escape strings?
+        const std::string defval =
+            field.IsRequired() ? "\"\"" : "\"" + field.value.constant + "\"";
+        if (context == kObject) return defval + ".to_string()";
+        if (context == kAccessor) return "&" + defval;
+        FLATBUFFERS_ASSERT("Unreachable.");
+        return "INVALID_CODE_GENERATION";
       }
+
       case ftVectorOfBool:
       case ftVectorOfFloat:
       case ftVectorOfInteger:
       case ftVectorOfString:
       case ftVectorOfStruct:
       case ftVectorOfTable:
       case ftVectorOfEnumKey:
-      case ftVectorOfUnionValue: {
-        // Required vectors.
-        return "Vec::new()";  // No default strings yet.
-      }
+      case ftVectorOfUnionValue:
       case ftStruct:
       case ftTable: {
-        // Required struct/tables.
-        return "Default::default()";  // punt.
+        // We only support empty vectors which matches the defaults for
+        // &[T] and Vec<T> anyway.
+        //
+        // For required structs and tables fields, we defer to their object API
+        // defaults. This works so long as there's nothing recursive happening,
+        // but `table Infinity { i: Infinity (required); }` does compile.
+        return "Default::default()";
       }
     }
     FLATBUFFERS_ASSERT("Unreachable.");
@@ -953,30 +973,38 @@ class RustGenerator : public BaseGenerator {
   std::string TableBuilderArgsDefnType(const FieldDef &field,
                                        const std::string &lifetime) {
     const Type &type = field.value.type;
+    auto WrapOption = [&](std::string s) {
+      return IsOptionalToBuilder(field) ? "Option<" + s + ">" : s;
+    };
+    auto WrapVector = [&](std::string ty) {
+      return WrapOption("flatbuffers::WIPOffset<flatbuffers::Vector<" +
+                        lifetime + ", " + ty + ">>");
+    };
+    auto WrapUOffsetsVector = [&](std::string ty) {
+      return WrapVector("flatbuffers::ForwardsUOffset<" + ty + ">");
+    };
 
     switch (GetFullType(type)) {
       case ftInteger:
       case ftFloat:
       case ftBool: {
-        const auto typname = GetTypeBasic(type);
-        return field.IsOptional() ? "Option<" + typname + ">" : typname;
+        return WrapOption(GetTypeBasic(type));
       }
       case ftStruct: {
         const auto typname = WrapInNameSpace(*type.struct_def);
-        return "Option<&" + lifetime + " " + typname + ">";
+        return WrapOption("&" + lifetime + " " + typname);
       }
       case ftTable: {
         const auto typname = WrapInNameSpace(*type.struct_def);
-        return "Option<flatbuffers::WIPOffset<" + typname + "<" + lifetime +
-               ">>>";
+        return WrapOption("flatbuffers::WIPOffset<" + typname + "<" + lifetime +
+                          ">>");
       }
       case ftString: {
-        return "Option<flatbuffers::WIPOffset<&" + lifetime + " str>>";
+        return WrapOption("flatbuffers::WIPOffset<&" + lifetime + " str>");
       }
       case ftEnumKey:
       case ftUnionKey: {
-        const auto typname = WrapInNameSpace(*type.enum_def);
-        return field.IsOptional() ? "Option<" + typname + ">" : typname;
+        return WrapOption(WrapInNameSpace(*type.enum_def));
       }
       case ftUnionValue: {
         return "Option<flatbuffers::WIPOffset<flatbuffers::UnionWIPOffset>>";
@@ -986,36 +1014,25 @@ class RustGenerator : public BaseGenerator {
       case ftVectorOfBool:
       case ftVectorOfFloat: {
         const auto typname = GetTypeBasic(type.VectorType());
-        return "Option<flatbuffers::WIPOffset<flatbuffers::Vector<" + lifetime +
-               ", " + typname + ">>>";
+        return WrapVector(typname);
       }
       case ftVectorOfEnumKey: {
         const auto typname = WrapInNameSpace(*type.enum_def);
-        return "Option<flatbuffers::WIPOffset<flatbuffers::Vector<" + lifetime +
-               ", " + typname + ">>>";
+        return WrapVector(typname);
       }
       case ftVectorOfStruct: {
         const auto typname = WrapInNameSpace(*type.struct_def);
-        return "Option<flatbuffers::WIPOffset<flatbuffers::Vector<" + lifetime +
-               ", " + typname + ">>>";
+        return WrapVector(typname);
       }
       case ftVectorOfTable: {
         const auto typname = WrapInNameSpace(*type.struct_def);
-        return "Option<flatbuffers::WIPOffset<flatbuffers::Vector<" + lifetime +
-               ", flatbuffers::ForwardsUOffset<" + typname + "<" + lifetime +
-               ">>>>>";
+        return WrapUOffsetsVector(typname + "<" + lifetime + ">");
       }
       case ftVectorOfString: {
-        return "Option<flatbuffers::WIPOffset<flatbuffers::Vector<" + lifetime +
-               ", flatbuffers::ForwardsUOffset<&" + lifetime + " str>>>>";
+        return WrapUOffsetsVector("&" + lifetime + " str");
       }
       case ftVectorOfUnionValue: {
-        const auto typname =
-            WrapInNameSpace(*type.enum_def) + "UnionTableOffset";
-        return "Option<flatbuffers::WIPOffset<flatbuffers::Vector<" + lifetime +
-               ", flatbuffers::ForwardsUOffset<"
-               "flatbuffers::Table<" +
-               lifetime + ">>>>";
+        return WrapUOffsetsVector("flatbuffers::Table<" + lifetime + ">");
       }
     }
     return "INVALID_CODE_GENERATION";  // for return analysis
@@ -1084,8 +1101,7 @@ class RustGenerator : public BaseGenerator {
         return "INVALID_CODE_GENERATION";  // OH NO!
       }
     }
-    if (in_a_table && !IsUnion(type) &&
-        (IsScalar(type.base_type) ? field.IsOptional() : !field.IsRequired())) {
+    if (in_a_table && !IsUnion(type) && field.IsOptional()) {
       return "Option<" + ty + ">";
     } else {
       return ty;
@@ -1207,73 +1223,64 @@ class RustGenerator : public BaseGenerator {
   std::string GenTableAccessorFuncReturnType(const FieldDef &field,
                                              const std::string &lifetime) {
     const Type &type = field.value.type;
+    const auto WrapOption = [&](std::string s) {
+      return field.IsOptional() ? "Option<" + s + ">" : s;
+    };
 
     switch (GetFullType(field.value.type)) {
       case ftInteger:
       case ftFloat:
       case ftBool: {
-        const auto typname = GetTypeBasic(type);
-        return field.IsOptional() ? "Option<" + typname + ">" : typname;
+        return WrapOption(GetTypeBasic(type));
       }
       case ftStruct: {
         const auto typname = WrapInNameSpace(*type.struct_def);
-        return WrapInOptionIfNotRequired("&" + lifetime + " " + typname,
-                                         field.IsRequired());
+        return WrapOption("&" + lifetime + " " + typname);
       }
       case ftTable: {
         const auto typname = WrapInNameSpace(*type.struct_def);
-        return WrapInOptionIfNotRequired(typname + "<" + lifetime + ">",
-                                         field.IsRequired());
+        return WrapOption(typname + "<" + lifetime + ">");
       }
       case ftEnumKey:
       case ftUnionKey: {
-        const auto typname = WrapInNameSpace(*type.enum_def);
-        return field.IsOptional() ? "Option<" + typname + ">" : typname;
+        return WrapOption(WrapInNameSpace(*type.enum_def));
       }
 
       case ftUnionValue: {
-        return WrapInOptionIfNotRequired("flatbuffers::Table<" + lifetime + ">",
-                                         field.IsRequired());
+        return WrapOption("flatbuffers::Table<" + lifetime + ">");
       }
       case ftString: {
-        return WrapInOptionIfNotRequired("&" + lifetime + " str",
-                                         field.IsRequired());
+        return WrapOption("&" + lifetime + " str");
       }
       case ftVectorOfInteger:
       case ftVectorOfBool:
       case ftVectorOfFloat: {
         const auto typname = GetTypeBasic(type.VectorType());
-        if (IsOneByte(type.VectorType().base_type)) {
-          return WrapInOptionIfNotRequired(
-              "&" + lifetime + " [" + typname + "]", field.IsRequired());
-        }
-        return WrapInOptionIfNotRequired(
-            "flatbuffers::Vector<" + lifetime + ", " + typname + ">",
-            field.IsRequired());
+        const auto vector_type =
+            IsOneByte(type.VectorType().base_type)
+                ? "&" + lifetime + " [" + typname + "]"
+                : "flatbuffers::Vector<" + lifetime + ", " + typname + ">";
+        return WrapOption(vector_type);
       }
       case ftVectorOfEnumKey: {
         const auto typname = WrapInNameSpace(*type.enum_def);
-        return WrapInOptionIfNotRequired(
-            "flatbuffers::Vector<" + lifetime + ", " + typname + ">",
-            field.IsRequired());
+        return WrapOption("flatbuffers::Vector<" + lifetime + ", " + typname +
+                          ">");
       }
       case ftVectorOfStruct: {
         const auto typname = WrapInNameSpace(*type.struct_def);
-        return WrapInOptionIfNotRequired("&" + lifetime + " [" + typname + "]",
-                                         field.IsRequired());
+        return WrapOption("&" + lifetime + " [" + typname + "]");
       }
       case ftVectorOfTable: {
         const auto typname = WrapInNameSpace(*type.struct_def);
-        return WrapInOptionIfNotRequired("flatbuffers::Vector<" + lifetime +
-                                             ", flatbuffers::ForwardsUOffset<" +
-                                             typname + "<" + lifetime + ">>>",
-                                         field.IsRequired());
+        return WrapOption("flatbuffers::Vector<" + lifetime +
+                          ", flatbuffers::ForwardsUOffset<" + typname + "<" +
+                          lifetime + ">>>");
       }
       case ftVectorOfString: {
-        return WrapInOptionIfNotRequired(
-            "flatbuffers::Vector<" + lifetime +
-                ", flatbuffers::ForwardsUOffset<&" + lifetime + " str>>",
-            field.IsRequired());
+        return WrapOption("flatbuffers::Vector<" + lifetime +
+                          ", flatbuffers::ForwardsUOffset<&" + lifetime +
+                          " str>>");
       }
       case ftVectorOfUnionValue: {
         FLATBUFFERS_ASSERT(false && "vectors of unions are not yet supported");
@@ -1354,7 +1361,7 @@ class RustGenerator : public BaseGenerator {
     // Default-y fields (scalars so far) are neither optional nor required.
     const std::string default_value =
         !(field.IsOptional() || field.IsRequired())
-            ? "Some(" + GetDefaultValue(field, /*builder=*/true) + ")"
+            ? "Some(" + GetDefaultValue(field, kAccessor) + ")"
             : "None";
     const std::string unwrap = field.IsOptional() ? "" : ".unwrap()";
 
@@ -1373,18 +1380,6 @@ class RustGenerator : public BaseGenerator {
            ", " + default_value + ")" + safe_slice + unwrap;
   }
 
-  bool TableFieldReturnsOption(const FieldDef &field) {
-    if (field.IsOptional()) return true;
-    switch (GetFullType(field.value.type)) {
-      case ftInteger:
-      case ftFloat:
-      case ftBool:
-      case ftEnumKey:
-      case ftUnionKey: return false;
-      default: return true;
-    }
-  }
-
   // Generates a fully-qualified name getter for use with --gen-name-strings
   void GenFullyQualifiedNameGetter(const StructDef &struct_def,
                                    const std::string &name) {
@@ -1425,7 +1420,7 @@ class RustGenerator : public BaseGenerator {
       code_.SetValue("OFFSET_NAME", GetFieldOffsetName(field));
       code_.SetValue("OFFSET_VALUE", NumToString(field.value.offset));
       code_.SetValue("FIELD_NAME", Name(field));
-      code_.SetValue("BLDR_DEF_VAL", GetDefaultValue(field, /*builder=*/true));
+      code_.SetValue("BLDR_DEF_VAL", GetDefaultValue(field, kBuilder));
       cb(field);
     };
     const auto &fields = struct_def.fields.vec;
@@ -1498,7 +1493,7 @@ class RustGenerator : public BaseGenerator {
             if (struct_def.sortbysize &&
                 size != SizeOf(field.value.type.base_type))
               return;
-            if (TableFieldReturnsOption(field)) {
+            if (IsOptionalToBuilder(field)) {
               code_ +=
                   "      if let Some(x) = args.{{FIELD_NAME}} "
                   "{ builder.add_{{FIELD_NAME}}(x); }";
@@ -1943,7 +1938,7 @@ class RustGenerator : public BaseGenerator {
     code_ += "    Self {";
     ForAllObjectTableFields(table, [&](const FieldDef &field) {
       if (field.value.type.base_type == BASE_TYPE_UTYPE) return;
-      std::string default_value = GetDefaultValue(field, /*builder=*/false);
+      std::string default_value = GetDefaultValue(field, kObject);
       code_ += "      {{FIELD_NAME}}: " + default_value + ",";
     });
     code_ += "    }";
@@ -2064,17 +2059,17 @@ class RustGenerator : public BaseGenerator {
     }
   }
   void MapNativeTableField(const FieldDef &field, const std::string &expr) {
-    if (field.IsRequired()) {
+    if (field.IsOptional()) {
+      code_ += "    let {{FIELD_NAME}} = self.{{FIELD_NAME}}.as_ref().map(|x|{";
+      code_ += "      " + expr;
+      code_ += "    });";
+    } else {
       // For some reason Args has optional types for required fields.
       // TODO(cneo): Fix this... but its a breaking change?
       code_ += "    let {{FIELD_NAME}} = Some({";
       code_ += "      let x = &self.{{FIELD_NAME}};";
       code_ += "      " + expr;
       code_ += "    });";
-    } else {
-      code_ += "    let {{FIELD_NAME}} = self.{{FIELD_NAME}}.as_ref().map(|x|{";
-      code_ += "      " + expr;
-      code_ += "    });";
     }
   }