diff --git a/BUILD b/BUILD index c346dd3e4..092973891 100644 --- a/BUILD +++ b/BUILD @@ -136,6 +136,7 @@ cc_test( ":tests/prototest/imported.proto", ":tests/prototest/test.golden", ":tests/prototest/test.proto", + ":tests/prototest/test_union.golden", ":tests/union_vector/union_vector.fbs", ], includes = ["include/"], diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 40e111465..77e7422a4 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -367,6 +367,7 @@ struct IDLOptions { bool mutable_buffer; bool one_file; bool proto_mode; + bool proto_oneof_union; bool generate_all; bool skip_unexpected_fields_in_json; bool generate_name_strings; @@ -426,6 +427,7 @@ struct IDLOptions { mutable_buffer(false), one_file(false), proto_mode(false), + proto_oneof_union(false), generate_all(false), skip_unexpected_fields_in_json(false), generate_name_strings(false), @@ -661,6 +663,9 @@ class Parser : public ParserState { FLATBUFFERS_CHECKED_ERROR ParseNamespace(); FLATBUFFERS_CHECKED_ERROR StartStruct(const std::string &name, StructDef **dest); + FLATBUFFERS_CHECKED_ERROR StartEnum(const std::string &name, + bool is_union, + EnumDef **dest); FLATBUFFERS_CHECKED_ERROR ParseDecl(); FLATBUFFERS_CHECKED_ERROR ParseService(); FLATBUFFERS_CHECKED_ERROR ParseProtoFields(StructDef *struct_def, diff --git a/src/flatc.cpp b/src/flatc.cpp index def85b400..7b430a921 100644 --- a/src/flatc.cpp +++ b/src/flatc.cpp @@ -101,6 +101,7 @@ std::string FlatCompiler::GetUsageString(const char *program_name) const { " --raw-binary Allow binaries without file_indentifier to be read.\n" " This may crash flatc given a mismatched schema.\n" " --proto Input is a .proto, translate to .fbs.\n" + " --oneof-union Translate .proto oneofs to flatbuffer unions.\n" " --grpc Generate GRPC interfaces for the specified languages\n" " --schema Serialize schemas instead of JSON (use with -b)\n" " --bfbs-comments Add doc comments to the binary schema files.\n" @@ -236,6 +237,8 @@ int FlatCompiler::Compile(int argc, const char **argv) { binary_files_from = filenames.size(); } else if (arg == "--proto") { opts.proto_mode = true; + } else if (arg == "--oneof-union") { + opts.proto_oneof_union = true; } else if (arg == "--schema") { schema_binary = true; } else if (arg == "-M") { diff --git a/src/idl_gen_fbs.cpp b/src/idl_gen_fbs.cpp index 80b943bd3..37c60267f 100644 --- a/src/idl_gen_fbs.cpp +++ b/src/idl_gen_fbs.cpp @@ -91,13 +91,19 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) { EnumDef &enum_def = **enum_def_it; GenNameSpace(*enum_def.defined_namespace, &schema, &last_namespace); GenComment(enum_def.doc_comment, &schema, nullptr); - schema += "enum " + enum_def.name + " : "; + if (enum_def.is_union) + schema += "union " + enum_def.name; + else + schema += "enum " + enum_def.name + " : "; schema += GenType(enum_def.underlying_type, true) + " {\n"; for (auto it = enum_def.vals.vec.begin(); it != enum_def.vals.vec.end(); ++it) { auto &ev = **it; GenComment(ev.doc_comment, &schema, nullptr, " "); - schema += " " + ev.name + " = " + NumToString(ev.value) + ",\n"; + if (enum_def.is_union) + schema += " " + GenType(ev.union_type) + ",\n"; + else + schema += " " + ev.name + " = " + NumToString(ev.value) + ",\n"; } schema += "}\n\n"; } diff --git a/src/idl_gen_general.cpp b/src/idl_gen_general.cpp index 78146e788..749159245 100644 --- a/src/idl_gen_general.cpp +++ b/src/idl_gen_general.cpp @@ -27,21 +27,6 @@ namespace flatbuffers { -// Convert an underscore_based_indentifier in to camelCase. -// Also uppercases the first character if first is true. -std::string MakeCamel(const std::string &in, bool first) { - std::string s; - for (size_t i = 0; i < in.length(); i++) { - if (!i && first) - s += static_cast(toupper(in[0])); - else if (in[i] == '_' && i + 1 < in.length()) - s += static_cast(toupper(in[++i])); - else - s += in[i]; - } - return s; -} - // These arrays need to correspond to the IDLOptions::k enum. struct LanguageParameters { diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index b1a9363d8..89e6e80d8 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -76,6 +76,21 @@ static bool ValidateUTF8(const std::string &str) { return true; } +// Convert an underscore_based_indentifier in to camelCase. +// Also uppercases the first character if first is true. +std::string MakeCamel(const std::string &in, bool first) { + std::string s; + for (size_t i = 0; i < in.length(); i++) { + if (!i && first) + s += static_cast(toupper(in[0])); + else if (in[i] == '_' && i + 1 < in.length()) + s += static_cast(toupper(in[++i])); + else + s += in[i]; + } + return s; +} + void Parser::Message(const std::string &msg) { error_ = file_being_parsed_.length() ? AbsolutePath(file_being_parsed_) : ""; // clang-format off @@ -1467,42 +1482,29 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) { NEXT(); std::string enum_name = attribute_; EXPECT(kTokenIdentifier); - auto &enum_def = *new EnumDef(); - enum_def.name = enum_name; - enum_def.file = file_being_parsed_; - enum_def.doc_comment = enum_comment; - enum_def.is_union = is_union; - enum_def.defined_namespace = current_namespace_; - if (enums_.Add(current_namespace_->GetFullyQualifiedName(enum_name), - &enum_def)) - return Error("enum already exists: " + enum_name); - if (is_union) { - enum_def.underlying_type.base_type = BASE_TYPE_UTYPE; - enum_def.underlying_type.enum_def = &enum_def; - } else { - if (opts.proto_mode) { - enum_def.underlying_type.base_type = BASE_TYPE_INT; + EnumDef *enum_def; + ECHECK(StartEnum(enum_name, is_union, &enum_def)); + enum_def->doc_comment = enum_comment; + if (!is_union && !opts.proto_mode) { + // Give specialized error message, since this type spec used to + // be optional in the first FlatBuffers release. + if (!Is(':')) { + return Error( + "must specify the underlying integer type for this" + " enum (e.g. \': short\', which was the default)."); } else { - // Give specialized error message, since this type spec used to - // be optional in the first FlatBuffers release. - if (!Is(':')) { - return Error( - "must specify the underlying integer type for this" - " enum (e.g. \': short\', which was the default)."); - } else { - NEXT(); - } - // Specify the integer type underlying this enum. - ECHECK(ParseType(enum_def.underlying_type)); - if (!IsInteger(enum_def.underlying_type.base_type)) - return Error("underlying enum type must be integral"); + NEXT(); } + // Specify the integer type underlying this enum. + ECHECK(ParseType(enum_def->underlying_type)); + if (!IsInteger(enum_def->underlying_type.base_type)) + return Error("underlying enum type must be integral"); // Make this type refer back to the enum it was derived from. - enum_def.underlying_type.enum_def = &enum_def; + enum_def->underlying_type.enum_def = enum_def; } - ECHECK(ParseMetaData(&enum_def.attributes)); + ECHECK(ParseMetaData(&enum_def->attributes)); EXPECT('{'); - if (is_union) enum_def.vals.Add("NONE", new EnumVal("NONE", 0)); + if (is_union) enum_def->vals.Add("NONE", new EnumVal("NONE", 0)); for (;;) { if (opts.proto_mode && attribute_ == "option") { ECHECK(ParseProtoOption()); @@ -1520,11 +1522,12 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) { std::replace(value_name.begin(), value_name.end(), '.', '_'); } } - auto prevsize = enum_def.vals.vec.size(); - auto value = - !enum_def.vals.vec.empty() ? enum_def.vals.vec.back()->value + 1 : 0; + auto prevsize = enum_def->vals.vec.size(); + auto value = !enum_def->vals.vec.empty() + ? enum_def->vals.vec.back()->value + 1 + : 0; auto &ev = *new EnumVal(value_name, value); - if (enum_def.vals.Add(value_name, &ev)) + if (enum_def->vals.Add(value_name, &ev)) return Error("enum value already exists: " + value_name); ev.doc_comment = value_comment; if (is_union) { @@ -1534,7 +1537,7 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) { if (ev.union_type.base_type != BASE_TYPE_STRUCT && ev.union_type.base_type != BASE_TYPE_STRING) return Error("union value type may only be table/struct/string"); - enum_def.uses_type_aliases = true; + enum_def->uses_type_aliases = true; } else { ev.union_type = Type(BASE_TYPE_STRUCT, LookupCreateStruct(full_name)); } @@ -1544,7 +1547,7 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) { ev.value = StringToInt(attribute_.c_str()); EXPECT(kTokenIntegerConstant); if (!opts.proto_mode && prevsize && - enum_def.vals.vec[prevsize - 1]->value >= ev.value) + enum_def->vals.vec[prevsize - 1]->value >= ev.value) return Error("enum values must be specified in ascending order"); } if (is_union) { @@ -1563,18 +1566,18 @@ CheckedError Parser::ParseEnum(bool is_union, EnumDef **dest) { if (Is('}')) break; } EXPECT('}'); - if (enum_def.attributes.Lookup("bit_flags")) { - for (auto it = enum_def.vals.vec.begin(); it != enum_def.vals.vec.end(); + if (enum_def->attributes.Lookup("bit_flags")) { + for (auto it = enum_def->vals.vec.begin(); it != enum_def->vals.vec.end(); ++it) { if (static_cast((*it)->value) >= - SizeOf(enum_def.underlying_type.base_type) * 8) + SizeOf(enum_def->underlying_type.base_type) * 8) return Error("bit flag out of range of underlying integral type"); (*it)->value = 1LL << (*it)->value; } } - if (dest) *dest = &enum_def; - types_.Add(current_namespace_->GetFullyQualifiedName(enum_def.name), - new Type(BASE_TYPE_UNION, nullptr, &enum_def)); + if (dest) *dest = enum_def; + types_.Add(current_namespace_->GetFullyQualifiedName(enum_def->name), + new Type(BASE_TYPE_UNION, nullptr, enum_def)); return NoError(); } @@ -1864,6 +1867,24 @@ CheckedError Parser::ParseProtoDecl() { return NoError(); } +CheckedError Parser::StartEnum(const std::string &enum_name, bool is_union, + EnumDef **dest) { + auto &enum_def = *new EnumDef(); + enum_def.name = enum_name; + enum_def.file = file_being_parsed_; + enum_def.doc_comment = doc_comment_; + enum_def.is_union = is_union; + enum_def.defined_namespace = current_namespace_; + if (enums_.Add(current_namespace_->GetFullyQualifiedName(enum_name), + &enum_def)) + return Error("enum already exists: " + enum_name); + enum_def.underlying_type.base_type = is_union ? BASE_TYPE_UTYPE + : BASE_TYPE_INT; + enum_def.underlying_type.enum_def = &enum_def; + if (dest) *dest = &enum_def; + return NoError(); +} + CheckedError Parser::ParseProtoFields(StructDef *struct_def, bool isextend, bool inside_oneof) { EXPECT('{'); @@ -1910,12 +1931,19 @@ CheckedError Parser::ParseProtoFields(StructDef *struct_def, bool isextend, } } StructDef *anonymous_struct = nullptr; + EnumDef *oneof_union = nullptr; Type type; if (IsIdent("group") || oneof) { if (!oneof) NEXT(); - auto name = "Anonymous" + NumToString(anonymous_counter++); - ECHECK(StartStruct(name, &anonymous_struct)); - type = Type(BASE_TYPE_STRUCT, anonymous_struct); + if (oneof && opts.proto_oneof_union) { + auto name = MakeCamel(attribute_, true) + "Union"; + ECHECK(StartEnum(name, true, &oneof_union)); + type = Type(BASE_TYPE_UNION, nullptr, oneof_union); + } else { + auto name = "Anonymous" + NumToString(anonymous_counter++); + ECHECK(StartStruct(name, &anonymous_struct)); + type = Type(BASE_TYPE_STRUCT, anonymous_struct); + } } else { ECHECK(ParseTypeFromProtoType(&type)); } @@ -1974,6 +2002,27 @@ CheckedError Parser::ParseProtoFields(StructDef *struct_def, bool isextend, if (anonymous_struct) { ECHECK(ParseProtoFields(anonymous_struct, false, oneof)); if (Is(';')) NEXT(); + } else if (oneof_union) { + // Parse into a temporary StructDef, then transfer fields into an + // EnumDef describing the oneof as a union. + StructDef oneof_struct; + ECHECK(ParseProtoFields(&oneof_struct, false, oneof)); + if (Is(';')) NEXT(); + for (auto field_it = oneof_struct.fields.vec.begin(); + field_it != oneof_struct.fields.vec.end(); ++field_it) { + const auto &oneof_field = **field_it; + const auto &oneof_type = oneof_field.value.type; + if (oneof_type.base_type != BASE_TYPE_STRUCT || + !oneof_type.struct_def || oneof_type.struct_def->fixed) + return Error("oneof '" + name + + "' cannot be mapped to a union because member '" + + oneof_field.name + "' is not a table type."); + auto enum_val = new EnumVal(oneof_type.struct_def->name, + oneof_union->vals.vec.size()); + enum_val->union_type = oneof_type; + enum_val->doc_comment = oneof_field.doc_comment; + oneof_union->vals.Add(oneof_field.name, enum_val); + } } else { EXPECT(';'); } diff --git a/tests/prototest/test.golden b/tests/prototest/test.golden index 389133d6f..ad59295a2 100644 --- a/tests/prototest/test.golden +++ b/tests/prototest/test.golden @@ -36,6 +36,8 @@ table ProtoMessage { n:proto.test.ProtoMessage_.OtherMessage; o:[string]; z:proto.test.ImportedMessage; + /// doc comment for r. + r:proto.test.ProtoMessage_.Anonymous0; } namespace proto.test.ProtoMessage_; @@ -46,3 +48,11 @@ table OtherMessage { b:float = 3.14149; } +table Anonymous0 { + /// doc comment for s. + s:proto.test.ImportedMessage; + /// doc comment for t on 2 + /// lines. + t:proto.test.ProtoMessage_.OtherMessage; +} + diff --git a/tests/prototest/test.proto b/tests/prototest/test.proto index 913b56462..cae1f3cfc 100644 --- a/tests/prototest/test.proto +++ b/tests/prototest/test.proto @@ -42,4 +42,12 @@ message ProtoMessage { optional OtherMessage n = 12; repeated string o = 14; optional ImportedMessage z = 16; + /// doc comment for r. + oneof r { + /// doc comment for s. + ImportedMessage s = 17; + /// doc comment for t on 2 + /// lines. + OtherMessage t = 18; + } } diff --git a/tests/prototest/test_union.golden b/tests/prototest/test_union.golden new file mode 100644 index 000000000..33fa0849e --- /dev/null +++ b/tests/prototest/test_union.golden @@ -0,0 +1,62 @@ +// Generated from test.proto + +namespace proto.test; + +/// Enum doc comment. +enum ProtoEnum : int { + FOO = 1, + /// Enum 2nd value doc comment misaligned. + BAR = 5, +} + +namespace proto.test.ProtoMessage_; + +union RUnion { + /// doc comment for s. + proto.test.ImportedMessage, + /// doc comment for t on 2 + /// lines. + proto.test.ProtoMessage_.OtherMessage, +} + +namespace proto.test; + +table ImportedMessage { + a:int; +} + +/// 2nd table doc comment with +/// many lines. +table ProtoMessage { + c:int = 16; + d:long; + p:uint; + e:ulong; + /// doc comment for f. + f:int = -1; + g:long; + h:uint; + q:ulong; + i:int; + j:long; + /// doc comment for k. + k:bool; + /// doc comment for l on 2 + /// lines + l:string (required); + m:[ubyte]; + n:proto.test.ProtoMessage_.OtherMessage; + o:[string]; + z:proto.test.ImportedMessage; + /// doc comment for r. + r:proto.test.ProtoMessage_.RUnion; +} + +namespace proto.test.ProtoMessage_; + +table OtherMessage { + a:double; + /// doc comment for b. + b:float = 3.14149; +} + diff --git a/tests/test.cpp b/tests/test.cpp index 2f0401244..4977878a3 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -863,6 +863,7 @@ void ParseProtoTest() { // load the .proto and the golden file from disk std::string protofile; std::string goldenfile; + std::string goldenunionfile; TEST_EQ( flatbuffers::LoadFile((test_data_path + "prototest/test.proto").c_str(), false, &protofile), @@ -871,6 +872,11 @@ void ParseProtoTest() { flatbuffers::LoadFile((test_data_path + "prototest/test.golden").c_str(), false, &goldenfile), true); + TEST_EQ( + flatbuffers::LoadFile((test_data_path + + "prototest/test_union.golden").c_str(), + false, &goldenunionfile), + true); flatbuffers::IDLOptions opts; opts.include_dependence_headers = false; @@ -889,6 +895,19 @@ void ParseProtoTest() { flatbuffers::Parser parser2; TEST_EQ(parser2.Parse(fbs.c_str(), nullptr), true); TEST_EQ_STR(fbs.c_str(), goldenfile.c_str()); + + // Parse proto with --oneof-union option. + opts.proto_oneof_union = true; + flatbuffers::Parser parser3(opts); + TEST_EQ(parser3.Parse(protofile.c_str(), include_directories), true); + + // Generate fbs. + auto fbs_union = flatbuffers::GenerateFBS(parser3, "test"); + + // Ensure generated file is parsable. + flatbuffers::Parser parser4; + TEST_EQ(parser4.Parse(fbs_union.c_str(), nullptr), true); + TEST_EQ_STR(fbs_union.c_str(), goldenunionfile.c_str()); } template