Optional-ness in reflection (#6097)

* Optional scalars in reflection

* fixed name collision

* Remove code duplicated by merge

Co-authored-by: Casper Neo <cneo@google.com>

2 files changed, 24 insertions, 18 deletions

diff --git a/src/idl_gen_swift.cpp b/src/idl_gen_swift.cpp
index 02ca4bb7..88214a9f 100644
--- a/src/idl_gen_swift.cpp
+++ b/src/idl_gen_swift.cpp
@@ -503,7 +503,8 @@ class SwiftGenerator : public BaseGenerator {
     auto &create_func_header = *create_header;
     auto name = Name(field);
     auto type = GenType(field.value.type);
-    auto nullable_type = (field.optional ? type + "?" : type);
+    bool opt_scalar = field.optional && IsScalar(field.value.type.base_type);
+    auto nullable_type = opt_scalar ? type + "?" : type;
     code_.SetValue("VALUENAME", name);
     code_.SetValue("VALUETYPE", nullable_type);
     code_.SetValue("OFFSET", name);
@@ -590,9 +591,10 @@ class SwiftGenerator : public BaseGenerator {
     code_.SetValue("VALUETYPE", type);
     code_.SetValue("OFFSET", name);
     code_.SetValue("CONSTANT", field.value.constant);
-    std::string nullable = field.optional ? "nil" : "{{CONSTANT}}";
-    std::string optional = field.optional ? "?" : "";
-    auto const_string = "return o == 0 ? " + nullable + " : ";
+    bool opt_scalar = field.optional && 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 + " : ";
     GenComment(field.doc_comment);
     if (IsScalar(field.value.type.base_type) && !IsEnum(field.value.type) &&
         !IsBool(field.value.type.base_type)) {
diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp
index 5e0b7d35..90e07674 100644
--- a/src/idl_parser.cpp
+++ b/src/idl_parser.cpp
@@ -743,19 +743,6 @@ CheckedError Parser::ParseField(StructDef &struct_def) {
       return Error(
           "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 && !SupportsOptionalScalars()) {
-      return Error(
-        "Optional scalars are not yet supported in at least one the of "
-        "the specified programming languages."
-      );
-    }
-  }
-
   // 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)) {
@@ -845,6 +832,23 @@ CheckedError Parser::ParseField(StructDef &struct_def) {
   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'");
+
+  // 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 && !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;
+  }
+
   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'");
@@ -3307,7 +3311,7 @@ Offset<reflection::Field> FieldDef::Serialize(FlatBufferBuilder *builder,
       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__);
+      attr__, docs__, optional);
   // 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.
 }