Allow structs to be parsed in JSON with out of order fields.

Also simplified the code and made it faster.

Change-Id: I1d83b1165a4a9a4380d1bfb5538769c012d2d367
Tested: on Linux.

include/flatbuffers/idl.h

@@ -221,8 +221,7 @@ struct Definition {
 };
 
 struct FieldDef : public Definition {
-  FieldDef() : deprecated(false), required(false), key(false), padding(0),
-               used(false) {}
+  FieldDef() : deprecated(false), required(false), key(false), padding(0) {}
 
   Offset<reflection::Field> Serialize(FlatBufferBuilder *builder, uint16_t id)
                                                                           const;
@@ -233,7 +232,6 @@ struct FieldDef : public Definition {
   bool required;   // Field must always be present.
   bool key;        // Field functions as a key for creating sorted vectors.
   size_t padding;  // Bytes to always pad after this field.
-  bool used;       // Used during JSON parsing to check for repeated fields.
 };
 
 struct StructDef : public Definition {
@@ -378,8 +376,8 @@ class Parser {
                      const std::string &name,
                      const Type &type);
   void ParseField(StructDef &struct_def);
-  void ParseAnyValue(Value &val, FieldDef *field);
-  uoffset_t ParseTable(const StructDef &struct_def);
+  void ParseAnyValue(Value &val, FieldDef *field, size_t parent_fieldn);
+  uoffset_t ParseTable(const StructDef &struct_def, std::string *value);
   void SerializeStruct(const StructDef &struct_def, const Value &val);
   void AddVector(bool sortbysize, int count);
   uoffset_t ParseVector(const Type &type);
@@ -428,7 +426,6 @@ class Parser {
   std::vector<std::string> doc_comment_;
 
   std::vector<std::pair<Value, FieldDef *>> field_stack_;
-  std::vector<uint8_t> struct_stack_;
 
   std::set<std::string> known_attributes_;
 

src/idl_parser.cpp

@@ -531,22 +531,22 @@ void Parser::ParseField(StructDef &struct_def) {
   Expect(';');
 }
 
-void Parser::ParseAnyValue(Value &val, FieldDef *field) {
+void Parser::ParseAnyValue(Value &val, FieldDef *field, size_t parent_fieldn) {
   switch (val.type.base_type) {
     case BASE_TYPE_UNION: {
       assert(field);
-      if (!field_stack_.size() ||
+      if (!parent_fieldn ||
           field_stack_.back().second->value.type.base_type != BASE_TYPE_UTYPE)
         Error("missing type field before this union value: " + field->name);
       auto enum_idx = atot<unsigned char>(
                                     field_stack_.back().first.constant.c_str());
       auto enum_val = val.type.enum_def->ReverseLookup(enum_idx);
       if (!enum_val) Error("illegal type id for: " + field->name);
-      val.constant = NumToString(ParseTable(*enum_val->struct_def));
+      ParseTable(*enum_val->struct_def, &val.constant);
       break;
     }
     case BASE_TYPE_STRUCT:
-      val.constant = NumToString(ParseTable(*val.type.struct_def));
+      ParseTable(*val.type.struct_def, &val.constant);
       break;
     case BASE_TYPE_STRING: {
       auto s = attribute_;
@@ -578,15 +578,14 @@ void Parser::ParseAnyValue(Value &val, FieldDef *field) {
 }
 
 void Parser::SerializeStruct(const StructDef &struct_def, const Value &val) {
-  auto off = atot<uoffset_t>(val.constant.c_str());
-  assert(struct_stack_.size() - off == struct_def.bytesize);
+  assert(val.constant.length() == struct_def.bytesize);
   builder_.Align(struct_def.minalign);
-  builder_.PushBytes(&struct_stack_[off], struct_def.bytesize);
-  struct_stack_.resize(struct_stack_.size() - struct_def.bytesize);
+  builder_.PushBytes(reinterpret_cast<const uint8_t *>(val.constant.c_str()),
+                     struct_def.bytesize);
   builder_.AddStructOffset(val.offset, builder_.GetSize());
 }
 
-uoffset_t Parser::ParseTable(const StructDef &struct_def) {
+uoffset_t Parser::ParseTable(const StructDef &struct_def, std::string *value) {
   Expect('{');
   size_t fieldn = 0;
   for (;;) {
@@ -596,30 +595,26 @@ uoffset_t Parser::ParseTable(const StructDef &struct_def) {
       Expect(strict_json_ ? kTokenStringConstant : kTokenIdentifier);
     auto field = struct_def.fields.Lookup(name);
     if (!field) Error("unknown field: " + name);
-    if (struct_def.fixed && (fieldn >= struct_def.fields.vec.size()
-                            || struct_def.fields.vec[fieldn] != field)) {
-       Error("struct field appearing out of order: " + name);
-    }
     Expect(':');
     Value val = field->value;
-    ParseAnyValue(val, field);
-    field_stack_.push_back(std::make_pair(val, field));
+    ParseAnyValue(val, field, fieldn);
+    size_t i = field_stack_.size();
+    // Hardcoded insertion-sort with error-check.
+    // If fields are specified in order, then this loop exits immediately.
+    for (; i > field_stack_.size() - fieldn; i--) {
+      auto existing_field = field_stack_[i - 1].second;
+      if (existing_field == field)
+        Error("field set more than once: " + field->name);
+      if (existing_field->value.offset < field->value.offset) break;
+    }
+    field_stack_.insert(field_stack_.begin() + i, std::make_pair(val, field));
     fieldn++;
     if (IsNext('}')) break;
     Expect(',');
   }
-  for (auto it = field_stack_.rbegin();
-           it != field_stack_.rbegin() + fieldn; ++it) {
-    if (it->second->used)
-      Error("field set more than once: " + it->second->name);
-    it->second->used = true;
-  }
-  for (auto it = field_stack_.rbegin();
-           it != field_stack_.rbegin() + fieldn; ++it) {
-    it->second->used = false;
-  }
   if (struct_def.fixed && fieldn != struct_def.fields.vec.size())
-    Error("incomplete struct initialization: " + struct_def.name);
+    Error("struct: wrong number of initializers: " + struct_def.name);
+
   auto start = struct_def.fixed
                  ? builder_.StartStruct(struct_def.minalign)
                  : builder_.StartTable();
@@ -670,19 +665,20 @@ uoffset_t Parser::ParseTable(const StructDef &struct_def) {
   if (struct_def.fixed) {
     builder_.ClearOffsets();
     builder_.EndStruct();
-    // Temporarily store this struct in a side buffer, since this data has to
-    // be stored in-line later in the parent object.
-    auto off = struct_stack_.size();
-    struct_stack_.insert(struct_stack_.end(),
-                         builder_.GetCurrentBufferPointer(),
-                         builder_.GetCurrentBufferPointer() +
+    assert(value);
+    // Temporarily store this struct in the value string, since it is to
+    // be serialized in-place elsewhere.
+    value->assign(
+          reinterpret_cast<const char *>(builder_.GetCurrentBufferPointer()),
           struct_def.bytesize);
     builder_.PopBytes(struct_def.bytesize);
-    return static_cast<uoffset_t>(off);
+    return 0xFFFFFFFF;  // Value not used by the caller.
   } else {
-    return builder_.EndTable(
+    auto off = builder_.EndTable(
       start,
       static_cast<voffset_t>(struct_def.fields.vec.size()));
+    if (value) *value = NumToString(off);
+    return off;
   }
 }
 
@@ -692,7 +688,7 @@ uoffset_t Parser::ParseVector(const Type &type) {
     if ((!strict_json_ || !count) && IsNext(']')) break;
     Value val;
     val.type = type;
-    ParseAnyValue(val, nullptr);
+    ParseAnyValue(val, nullptr, 0);
     field_stack_.push_back(std::make_pair(val, nullptr));
     count++;
     if (IsNext(']')) break;
@@ -1435,7 +1431,7 @@ bool Parser::Parse(const char *source, const char **include_paths,
         if (builder_.GetSize()) {
           Error("cannot have more than one json object in a file");
         }
-        builder_.Finish(Offset<Table>(ParseTable(*root_struct_def_)),
+        builder_.Finish(Offset<Table>(ParseTable(*root_struct_def_, nullptr)),
           file_identifier_.length() ? file_identifier_.c_str() : nullptr);
       } else if (token_ == kTokenEnum) {
         ParseEnum(false);
@@ -1507,7 +1503,6 @@ bool Parser::Parse(const char *source, const char **include_paths,
     return false;
   }
   if (source_filename) files_being_parsed_.pop();
-  assert(!struct_stack_.size());
   return true;
 }
 

tests/MyGame/Example/Monster.cs

@@ -5,6 +5,7 @@ namespace MyGame.Example
 
 using FlatBuffers;
 
+/// an example documentation comment: monster object
 public sealed class Monster : Table {
   public static Monster GetRootAsMonster(ByteBuffer _bb) { return GetRootAsMonster(_bb, new Monster()); }
   public static Monster GetRootAsMonster(ByteBuffer _bb, Monster obj) { return (obj.__init(_bb.GetInt(_bb.Position) + _bb.Position, _bb)); }

tests/MyGame/Example/Monster.go

@@ -5,6 +5,7 @@ package Example
 import (
 	flatbuffers "github.com/google/flatbuffers/go"
 )
+/// an example documentation comment: monster object
 type Monster struct {
 	_tab flatbuffers.Table
 }

tests/MyGame/Example/Monster.java

@@ -8,6 +8,9 @@ import java.util.*;
 import com.google.flatbuffers.*;
 
 @SuppressWarnings("unused")
+/**
+ * an example documentation comment: monster object
+ */
 public final class Monster extends Table {
   public static Monster getRootAsMonster(ByteBuffer _bb) { return getRootAsMonster(_bb, new Monster()); }
   public static Monster getRootAsMonster(ByteBuffer _bb, Monster obj) { _bb.order(ByteOrder.LITTLE_ENDIAN); return (obj.__init(_bb.getInt(_bb.position()) + _bb.position(), _bb)); }

tests/MyGame/Example/Monster.py

@@ -4,6 +4,7 @@
 
 import flatbuffers
 
+# /// an example documentation comment: monster object
 class Monster(object):
     __slots__ = ['_tab']
 

tests/monster_test_generated.h

@@ -167,6 +167,7 @@ inline flatbuffers::Offset<Stat> CreateStat(flatbuffers::FlatBufferBuilder &_fbb
   return builder_.Finish();
 }
 
+/// an example documentation comment: monster object
 struct Monster FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
   const Vec3 *pos() const { return GetStruct<const Vec3 *>(4); }
   Vec3 *mutable_pos() { return GetStruct<Vec3 *>(4); }

tests/monsterdata_test.json

@@ -29,8 +29,8 @@
       b: 20
     },
     {
-      a: 30,
-      b: 40
+      b: 40,
+      a: 30
     }
   ],
   testarrayofstring: [

tests/test.cpp

@@ -738,7 +738,7 @@ void ErrorTest() {
   TestError("table X { Y:int; } root_type X; { \"Y\":1, }", "string constant",
             true);
   TestError("struct X { Y:int; Z:int; } table W { V:X; } root_type W; "
-            "{ V:{ Y:1 } }", "incomplete");
+            "{ V:{ Y:1 } }", "wrong number");
   TestError("enum E:byte { A } table X { Y:E; } root_type X; { Y:U }",
             "unknown enum value");
   TestError("table X { Y:byte; } root_type X; { Y:; }", "starting");