From 9e6c5f9f2c543a5ca608e8c1c4c9205139a87dcd Mon Sep 17 00:00:00 2001 From: Wouter van Oortmerssen Date: Mon, 20 Jun 2016 16:06:30 -0700 Subject: [PATCH] JSON Parser allows union type fields to come after unions. This is useful because many JSON generators will sort the fields, cause X_type to follow X. Change-Id: I00ef3ac05418224fc05aee93e6b3b3597e73ffe3 Tested: on Linux. Bug: 29221752 --- include/flatbuffers/idl.h | 28 +++++++++++++-------- include/flatbuffers/reflection.h | 4 ++- src/idl_gen_cpp.cpp | 12 ++++----- src/idl_parser.cpp | 53 +++++++++++++++++++++++++++++++--------- tests/test.cpp | 13 +++++++++- 5 files changed, 80 insertions(+), 30 deletions(-) diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index dc385ef..e302725 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -359,6 +359,19 @@ struct IDLOptions { lang(IDLOptions::kJava) {} }; +// This encapsulates where the parser is in the current source file. +struct ParserState { + ParserState() : cursor_(nullptr), line_(1), token_(-1) {} + + protected: + const char *cursor_; + int line_; // the current line being parsed + int token_; + + std::string attribute_; + std::vector doc_comment_; +}; + // A way to make error propagation less error prone by requiring values to be // checked. // Once you create a value of this type you must either: @@ -400,14 +413,12 @@ class CheckedError { #define FLATBUFFERS_CHECKED_ERROR CheckedError #endif -class Parser { +class Parser : public ParserState { public: explicit Parser(const IDLOptions &options = IDLOptions()) : root_struct_def_(nullptr), opts(options), source_(nullptr), - cursor_(nullptr), - line_(1), anonymous_counter(0) { // Just in case none are declared: namespaces_.push_back(new Namespace()); @@ -478,7 +489,8 @@ private: FieldDef **dest); FLATBUFFERS_CHECKED_ERROR ParseField(StructDef &struct_def); FLATBUFFERS_CHECKED_ERROR ParseAnyValue(Value &val, FieldDef *field, - size_t parent_fieldn); + size_t parent_fieldn, + const StructDef *parent_struct_def); FLATBUFFERS_CHECKED_ERROR ParseTable(const StructDef &struct_def, std::string *value, uoffset_t *ovalue); void SerializeStruct(const StructDef &struct_def, const Value &val); @@ -538,13 +550,9 @@ private: IDLOptions opts; private: - const char *source_, *cursor_; - int line_; // the current line being parsed - int token_; - std::string file_being_parsed_; + const char *source_; - std::string attribute_; - std::vector doc_comment_; + std::string file_being_parsed_; std::vector> field_stack_; diff --git a/include/flatbuffers/reflection.h b/include/flatbuffers/reflection.h index ababe6a..8709143 100644 --- a/include/flatbuffers/reflection.h +++ b/include/flatbuffers/reflection.h @@ -335,6 +335,8 @@ template pointer_inside_vector piv(T *ptr, return pointer_inside_vector(ptr, vec); } +inline const char *UnionTypeFieldSuffix() { return "_type"; } + // Helper to figure out the actual table type a union refers to. inline const reflection::Object &GetUnionType( const reflection::Schema &schema, const reflection::Object &parent, @@ -342,7 +344,7 @@ inline const reflection::Object &GetUnionType( auto enumdef = schema.enums()->Get(unionfield.type()->index()); // TODO: this is clumsy and slow, but no other way to find it? auto type_field = parent.fields()->LookupByKey( - (unionfield.name()->str() + "_type").c_str()); + (unionfield.name()->str() + UnionTypeFieldSuffix()).c_str()); assert(type_field); auto union_type = GetFieldI(table, *type_field); auto enumval = enumdef->values()->LookupByKey(union_type); diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp index b1402d8..6c79814 100644 --- a/src/idl_gen_cpp.cpp +++ b/src/idl_gen_cpp.cpp @@ -604,8 +604,8 @@ class CppGenerator : public BaseGenerator { switch (field.value.type.base_type) { case BASE_TYPE_UNION: code += prefix + "Verify" + field.value.type.enum_def->name; - code += - "(verifier, " + field.name + "(), " + field.name + "_type())"; + code += "(verifier, " + field.name + "(), " + field.name + + UnionTypeFieldSuffix() + "())"; break; case BASE_TYPE_STRUCT: if (!field.value.type.struct_def->fixed) { @@ -868,8 +868,8 @@ class CppGenerator : public BaseGenerator { // so that namespaces are properly opened and closed void SetNameSpace(const Namespace *ns, std::string *code_ptr) { if (cur_name_space_ == ns) return; - // compute the size of the longest common namespace prefix. - // if cur_name_space is A::B::C::D and ns is A::B::E::F::G, + // compute the size of the longest common namespace prefix. + // if cur_name_space is A::B::C::D and ns is A::B::E::F::G, // the common prefix is A::B:: and we have old_size = 4, new_size = 5 // and common_prefix_size = 2 auto old_size = @@ -881,13 +881,13 @@ class CppGenerator : public BaseGenerator { cur_name_space_->components[common_prefix_size]) common_prefix_size++; // close cur_name_space in reverse order to reach the common prefix - // in the previous example, D then C are closed + // in the previous example, D then C are closed for (auto j = old_size; j > common_prefix_size; --j) *code_ptr += "} // namespace " + cur_name_space_->components[j - 1] + "\n"; if (old_size != common_prefix_size) *code_ptr += "\n"; // open namespace parts to reach the ns namespace - // in the previous example, E, then F, then G are opened + // in the previous example, E, then F, then G are opened for (auto j = common_prefix_size; j != new_size; ++j) *code_ptr += "namespace " + ns->components[j] + " {\n"; if (new_size != common_prefix_size) *code_ptr += "\n"; diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index 518283d..8fddb3c 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -583,9 +583,9 @@ CheckedError Parser::ParseField(StructDef &struct_def) { FieldDef *typefield = nullptr; if (type.base_type == BASE_TYPE_UNION) { // For union fields, add a second auto-generated field to hold the type, - // with _type appended as the name. - ECHECK(AddField(struct_def, name + "_type", type.enum_def->underlying_type, - &typefield)); + // with a special suffix. + ECHECK(AddField(struct_def, name + UnionTypeFieldSuffix(), + type.enum_def->underlying_type, &typefield)); } FieldDef *field; @@ -686,17 +686,45 @@ CheckedError Parser::ParseField(StructDef &struct_def) { } CheckedError Parser::ParseAnyValue(Value &val, FieldDef *field, - size_t parent_fieldn) { + size_t parent_fieldn, + const StructDef *parent_struct_def) { switch (val.type.base_type) { case BASE_TYPE_UNION: { assert(field); + std::string constant; if (!parent_fieldn || - field_stack_.back().second->value.type.base_type != BASE_TYPE_UTYPE) - return Error("missing type field before this union value: " + - field->name); + field_stack_.back().second->value.type.base_type != BASE_TYPE_UTYPE) { + // We haven't seen the type field yet. Sadly a lot of JSON writers + // output these in alphabetical order, meaning it comes after this + // value. So we scan past the value to find it, then come back here. + auto type_name = field->name + UnionTypeFieldSuffix(); + assert(parent_struct_def); + auto type_field = parent_struct_def->fields.Lookup(type_name); + assert(type_field); // Guaranteed by ParseField(). + // Remember where we are in the source file, so we can come back here. + auto backup = *static_cast(this); + ECHECK(SkipAnyJsonValue()); // The table. + EXPECT(','); + auto next_name = attribute_; + if (Is(kTokenStringConstant)) { + NEXT(); + } else { + EXPECT(kTokenIdentifier); + } + if (next_name != type_name) + return Error("missing type field after this union value: " + + type_name); + EXPECT(':'); + Value type_val = type_field->value; + ECHECK(ParseAnyValue(type_val, type_field, 0, nullptr)); + constant = type_val.constant; + // Got the information we needed, now rewind: + *static_cast(this) = backup; + } else { + constant = field_stack_.back().first.constant; + } uint8_t enum_idx; - ECHECK(atot(field_stack_.back().first.constant.c_str(), *this, - &enum_idx)); + ECHECK(atot(constant.c_str(), *this, &enum_idx)); auto enum_val = val.type.enum_def->ReverseLookup(enum_idx); if (!enum_val) return Error("illegal type id for: " + field->name); ECHECK(ParseTable(*enum_val->struct_def, &val.constant, nullptr)); @@ -771,7 +799,7 @@ CheckedError Parser::ParseTable(const StructDef &struct_def, std::string *value, NEXT(); // Ignore this field. } else { Value val = field->value; - ECHECK(ParseAnyValue(val, field, fieldn)); + ECHECK(ParseAnyValue(val, field, fieldn, &struct_def)); size_t i = field_stack_.size(); // Hardcoded insertion-sort with error-check. // If fields are specified in order, then this loop exits immediately. @@ -870,7 +898,7 @@ CheckedError Parser::ParseVector(const Type &type, uoffset_t *ovalue) { if ((!opts.strict_json || !count) && Is(']')) { NEXT(); break; } Value val; val.type = type; - ECHECK(ParseAnyValue(val, nullptr, 0)); + ECHECK(ParseAnyValue(val, nullptr, 0, nullptr)); field_stack_.push_back(std::make_pair(val, nullptr)); count++; if (Is(']')) { NEXT(); break; } @@ -1324,7 +1352,8 @@ CheckedError Parser::ParseDecl() { } } - ECHECK(CheckClash(fields, struct_def, "_type", BASE_TYPE_UNION)); + ECHECK(CheckClash(fields, struct_def, UnionTypeFieldSuffix(), + BASE_TYPE_UNION)); ECHECK(CheckClash(fields, struct_def, "Type", BASE_TYPE_UNION)); ECHECK(CheckClash(fields, struct_def, "_length", BASE_TYPE_VECTOR)); ECHECK(CheckClash(fields, struct_def, "Length", BASE_TYPE_VECTOR)); diff --git a/tests/test.cpp b/tests/test.cpp index 73e009c..6f3b062 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -783,7 +783,7 @@ void ErrorTest() { TestError("table X { Y:int; Y:int; }", "field already"); TestError("struct X { Y:string; }", "only scalar"); TestError("struct X { Y:int (deprecated); }", "deprecate"); - TestError("union Z { X } table X { Y:Z; } root_type X; { Y: {", + TestError("union Z { X } table X { Y:Z; } root_type X; { Y: {}, A:1 }", "missing type field"); TestError("union Z { X } table X { Y:Z; } root_type X; { Y_type: 99, Y: {", "type id"); @@ -951,6 +951,16 @@ void UnknownFieldsTest() { TEST_EQ(jsongen == "{str: \"test\",i: 10}", true); } +void ParseUnionTest() { + // Unions must be parseable with the type field following the object. + flatbuffers::Parser parser; + TEST_EQ(parser.Parse("table T { A:int; }" + "union U { T }" + "table V { X:U; }" + "root_type V;" + "{ X:{ A:1 }, X_type: T }"), true); +} + int main(int /*argc*/, const char * /*argv*/[]) { // Run our various test suites: @@ -979,6 +989,7 @@ int main(int /*argc*/, const char * /*argv*/[]) { UnicodeSurrogatesTest(); UnicodeInvalidSurrogatesTest(); UnknownFieldsTest(); + ParseUnionTest(); if (!testing_fails) { TEST_OUTPUT_LINE("ALL TESTS PASSED"); -- 2.7.4