From c2ba7fd251b9da9fd2f0210818cc0221682f29c4 Mon Sep 17 00:00:00 2001 From: Wouter van Oortmerssen Date: Tue, 19 Aug 2014 16:37:46 -0700 Subject: [PATCH] Referring to types from other namespaces in C++ now works correctly. Previously, it would ignore the fact that the type comes from a different namespace. Now they are pre-declared in their own namespace, and referenced with a qualified name if necessary. Bug: 16851682 Change-Id: I5cb625b86d28e7436b9e93c70a0fa16a600d9884 Tested: on Linux --- include/flatbuffers/idl.h | 22 ++++++- src/idl_gen_cpp.cpp | 140 ++++++++++++++++++++++++++++------------- src/idl_gen_go.cpp | 24 +++---- src/idl_gen_java.cpp | 24 +++---- src/idl_parser.cpp | 6 +- tests/include_test2.fbs | 5 ++ tests/monster_test_generated.h | 14 +++-- 7 files changed, 158 insertions(+), 77 deletions(-) diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 680ed15..162501c 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -159,6 +159,11 @@ template class SymbolTable { std::vector vec; // Used to iterate in order of insertion }; +// A name space, as set in the schema. +struct Namespace { + std::vector components; +}; + // Base class for all definition types (fields, structs_, enums_). struct Definition { Definition() : generated(false) {} @@ -183,7 +188,8 @@ struct StructDef : public Definition { predecl(true), sortbysize(true), minalign(1), - bytesize(0) + bytesize(0), + defined_namespace(nullptr) {} void PadLastField(size_t minalign) { @@ -198,6 +204,7 @@ struct StructDef : public Definition { bool sortbysize; // Whether fields come in the declaration or size order. size_t minalign; // What the whole object needs to be aligned to. size_t bytesize; // Size if fixed. + Namespace *defined_namespace; // Where it was defined. }; inline bool IsStruct(const Type &type) { @@ -245,7 +252,16 @@ class Parser { root_struct_def(nullptr), source_(nullptr), cursor_(nullptr), - line_(1) {} + line_(1) { + // Just in case none are declared: + namespaces_.push_back(new Namespace()); + } + + ~Parser() { + for (auto it = namespaces_.begin(); it != namespaces_.end(); ++it) { + delete *it; + } + } // Parse the string containing either schema or JSON data, which will // populate the SymbolTable's or the FlatBufferBuilder above. @@ -284,7 +300,7 @@ class Parser { public: SymbolTable structs_; SymbolTable enums_; - std::vector name_space_; // As set in the schema. + std::vector namespaces_; std::string error_; // User readable error_ if Parse() == false FlatBufferBuilder builder_; // any data contained in the file diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp index 8804c8e..5f3d736 100644 --- a/src/idl_gen_cpp.cpp +++ b/src/idl_gen_cpp.cpp @@ -33,18 +33,31 @@ static std::string GenTypeBasic(const Type &type) { return ctypename[type.base_type]; } -static std::string GenTypeWire(const Type &type, const char *postfix); +static std::string GenTypeWire(const Parser &parser, const Type &type, + const char *postfix); // Return a C++ pointer type, specialized to the actual struct/table types, // and vector element types. -static std::string GenTypePointer(const Type &type) { +static std::string GenTypePointer(const Parser &parser, const Type &type) { switch (type.base_type) { case BASE_TYPE_STRING: return "flatbuffers::String"; case BASE_TYPE_VECTOR: - return "flatbuffers::Vector<" + GenTypeWire(type.VectorType(), "") + ">"; - case BASE_TYPE_STRUCT: - return type.struct_def->name; + return "flatbuffers::Vector<" + + GenTypeWire(parser, type.VectorType(), "") + ">"; + case BASE_TYPE_STRUCT: { + auto ns = type.struct_def->defined_namespace; + if (parser.namespaces_.back() != ns) { + std::string qualified_name; + for (auto it = ns->components.begin(); + it != ns->components.end(); ++it) { + qualified_name += *it + "::"; + } + return qualified_name + type.struct_def->name; + } else { + return type.struct_def->name; + } + } case BASE_TYPE_UNION: // fall through default: @@ -54,31 +67,33 @@ static std::string GenTypePointer(const Type &type) { // Return a C++ type for any type (scalar/pointer) specifically for // building a flatbuffer. -static std::string GenTypeWire(const Type &type, const char *postfix) { +static std::string GenTypeWire(const Parser &parser, const Type &type, + const char *postfix) { return IsScalar(type.base_type) ? GenTypeBasic(type) + postfix : IsStruct(type) - ? "const " + GenTypePointer(type) + " *" - : "flatbuffers::Offset<" + GenTypePointer(type) + ">" + postfix; + ? "const " + GenTypePointer(parser, type) + " *" + : "flatbuffers::Offset<" + GenTypePointer(parser, type) + ">" + postfix; } // Return a C++ type for any type (scalar/pointer) that reflects its // serialized size. -static std::string GenTypeSize(const Type &type) { +static std::string GenTypeSize(const Parser &parser, const Type &type) { return IsScalar(type.base_type) ? GenTypeBasic(type) : IsStruct(type) - ? GenTypePointer(type) + ? GenTypePointer(parser, type) : "flatbuffers::uoffset_t"; } // Return a C++ type for any type (scalar/pointer) specifically for // using a flatbuffer. -static std::string GenTypeGet(const Type &type, const char *afterbasic, - const char *beforeptr, const char *afterptr) { +static std::string GenTypeGet(const Parser &parser, const Type &type, + const char *afterbasic, const char *beforeptr, + const char *afterptr) { return IsScalar(type.base_type) ? GenTypeBasic(type) + afterbasic - : beforeptr + GenTypePointer(type) + afterptr; + : beforeptr + GenTypePointer(parser, type) + afterptr; } // Generate a documentation comment, if available. @@ -181,13 +196,13 @@ static void GenTable(const Parser &parser, StructDef &struct_def, auto &field = **it; if (!field.deprecated) { // Deprecated fields won't be accessible. GenComment(field.doc_comment, code_ptr, " "); - code += " " + GenTypeGet(field.value.type, " ", "const ", " *"); + code += " " + GenTypeGet(parser, field.value.type, " ", "const ", " *"); code += field.name + "() const { return "; // Call a different accessor for pointers, that indirects. code += IsScalar(field.value.type.base_type) ? "GetField<" : (IsStruct(field.value.type) ? "GetStruct<" : "GetPointer<"); - code += GenTypeGet(field.value.type, "", "const ", " *") + ">("; + code += GenTypeGet(parser, field.value.type, "", "const ", " *") + ">("; code += NumToString(field.value.offset); // Default value as second arg for non-pointer types. if (IsScalar(field.value.type.base_type)) @@ -213,7 +228,7 @@ static void GenTable(const Parser &parser, StructDef &struct_def, ++it) { auto &field = **it; if (!field.deprecated) { - code += prefix + "VerifyField<" + GenTypeSize(field.value.type); + code += prefix + "VerifyField<" + GenTypeSize(parser, field.value.type); code += ">(verifier, " + NumToString(field.value.offset); code += " /* " + field.name + " */)"; switch (field.value.type.base_type) { @@ -268,9 +283,10 @@ static void GenTable(const Parser &parser, StructDef &struct_def, auto &field = **it; if (!field.deprecated) { code += " void add_" + field.name + "("; - code += GenTypeWire(field.value.type, " ") + field.name + ") { fbb_.Add"; + code += GenTypeWire(parser, field.value.type, " ") + field.name; + code += ") { fbb_.Add"; if (IsScalar(field.value.type.base_type)) - code += "Element<" + GenTypeWire(field.value.type, "") + ">"; + code += "Element<" + GenTypeWire(parser, field.value.type, "") + ">"; else if (IsStruct(field.value.type)) code += "Struct"; else @@ -301,8 +317,8 @@ static void GenTable(const Parser &parser, StructDef &struct_def, ++it) { auto &field = **it; if (!field.deprecated) { - code += ",\n " + GenTypeWire(field.value.type, " ") + field.name; - code += " = " + field.value.constant; + code += ",\n " + GenTypeWire(parser, field.value.type, " "); + code += field.name + " = " + field.value.constant; } } code += ") {\n " + struct_def.name + "Builder builder_(_fbb);\n"; @@ -324,7 +340,8 @@ static void GenTable(const Parser &parser, StructDef &struct_def, } // Generate an accessor struct with constructor for a flatbuffers struct. -static void GenStruct(StructDef &struct_def, std::string *code_ptr) { +static void GenStruct(const Parser &parser, StructDef &struct_def, + std::string *code_ptr) { if (struct_def.generated) return; std::string &code = *code_ptr; @@ -341,7 +358,7 @@ static void GenStruct(StructDef &struct_def, std::string *code_ptr) { it != struct_def.fields.vec.end(); ++it) { auto &field = **it; - code += " " + GenTypeGet(field.value.type, " ", "", " "); + code += " " + GenTypeGet(parser, field.value.type, " ", "", " "); code += field.name + "_;\n"; if (field.padding) { for (int i = 0; i < 4; i++) @@ -359,7 +376,8 @@ static void GenStruct(StructDef &struct_def, std::string *code_ptr) { ++it) { auto &field = **it; if (it != struct_def.fields.vec.begin()) code += ", "; - code += GenTypeGet(field.value.type, " ", "const ", " &") + field.name; + code += GenTypeGet(parser, field.value.type, " ", "const ", " &"); + code += field.name; } code += ")\n : "; padding_id = 0; @@ -394,7 +412,7 @@ static void GenStruct(StructDef &struct_def, std::string *code_ptr) { ++it) { auto &field = **it; GenComment(field.doc_comment, code_ptr, " "); - code += " " + GenTypeGet(field.value.type, " ", "const ", " &"); + code += " " + GenTypeGet(parser, field.value.type, " ", "const ", " &"); code += field.name + "() const { return "; if (IsScalar(field.value.type.base_type)) code += "flatbuffers::EndianScalar(" + field.name + "_)"; @@ -406,6 +424,18 @@ static void GenStruct(StructDef &struct_def, std::string *code_ptr) { code += NumToString(struct_def.bytesize) + ");\n\n"; } +void GenerateNestedNameSpaces(Namespace *ns, std::string *code_ptr) { + for (auto it = ns->components.begin(); it != ns->components.end(); ++it) { + *code_ptr += "namespace " + *it + " {\n"; + } +} + +void CloseNestedNameSpaces(Namespace *ns, std::string *code_ptr) { + for (auto it = ns->components.rbegin(); it != ns->components.rend(); ++it) { + *code_ptr += "} // namespace " + *it + "\n"; + } +} + } // namespace cpp // Iterate through all definitions we haven't generate code for (enums, structs, @@ -422,18 +452,43 @@ std::string GenerateCPP(const Parser &parser, const std::string &include_guard_i // Generate forward declarations for all structs/tables, since they may // have circular references. - std::string forward_decl_code; + std::string forward_decl_code_same_namespace; + std::string forward_decl_code_other_namespace; + Namespace *cur_name_space = nullptr; for (auto it = parser.structs_.vec.begin(); it != parser.structs_.vec.end(); ++it) { - if (!(*it)->generated) - forward_decl_code += "struct " + (*it)->name + ";\n"; + auto &struct_def = **it; + auto decl = "struct " + struct_def.name + ";\n"; + if (struct_def.defined_namespace == parser.namespaces_.back()) { + forward_decl_code_same_namespace += decl; + } else { + // Wrap this decl in the correct namespace. Only open a namespace if + // the adjacent one is different. + // TODO: this could be done more intelligently, by sorting to + // namespace path and only opening/closing what is necessary, but that's + // quite a bit more complexity. + if (cur_name_space != struct_def.defined_namespace) { + if (cur_name_space) { + CloseNestedNameSpaces(cur_name_space, + &forward_decl_code_other_namespace); + } + GenerateNestedNameSpaces(struct_def.defined_namespace, + &forward_decl_code_other_namespace); + cur_name_space = struct_def.defined_namespace; + } + forward_decl_code_other_namespace += decl; + } + } + if (cur_name_space) { + CloseNestedNameSpaces(cur_name_space, + &forward_decl_code_other_namespace); } // Generate code for all structs, then all tables. std::string decl_code; for (auto it = parser.structs_.vec.begin(); it != parser.structs_.vec.end(); ++it) { - if ((**it).fixed) GenStruct(**it, &decl_code); + if ((**it).fixed) GenStruct(parser, **it, &decl_code); } for (auto it = parser.structs_.vec.begin(); it != parser.structs_.vec.end(); ++it) { @@ -441,7 +496,7 @@ std::string GenerateCPP(const Parser &parser, const std::string &include_guard_i } // Only output file-level code if there were any declarations. - if (enum_code.length() || forward_decl_code.length() || decl_code.length()) { + if (enum_code.length() || decl_code.length()) { std::string code; code = "// automatically generated by the FlatBuffers compiler," " do not modify\n\n"; @@ -449,8 +504,9 @@ std::string GenerateCPP(const Parser &parser, const std::string &include_guard_i // Generate include guard. std::string include_guard = "FLATBUFFERS_GENERATED_" + include_guard_ident; include_guard += "_"; - for (auto it = parser.name_space_.begin(); - it != parser.name_space_.end(); ++it) { + auto name_space = parser.namespaces_.back(); + for (auto it = name_space->components.begin(); + it != name_space->components.end(); ++it) { include_guard += *it + "_"; } include_guard += "H_"; @@ -461,17 +517,17 @@ std::string GenerateCPP(const Parser &parser, const std::string &include_guard_i code += "#include \"flatbuffers/flatbuffers.h\"\n\n"; - // Generate nested namespaces. - for (auto it = parser.name_space_.begin(); - it != parser.name_space_.end(); ++it) { - code += "namespace " + *it + " {\n"; - } + code += forward_decl_code_other_namespace; + code += "\n"; - // Output the main declaration code from above. + GenerateNestedNameSpaces(name_space, &code); code += "\n"; - code += enum_code; - code += forward_decl_code; + + code += forward_decl_code_same_namespace; code += "\n"; + + // Output the main declaration code from above. + code += enum_code; code += decl_code; code += enum_code_post; @@ -507,11 +563,7 @@ std::string GenerateCPP(const Parser &parser, const std::string &include_guard_i } } - // Close the namespaces. - for (auto it = parser.name_space_.rbegin(); - it != parser.name_space_.rend(); ++it) { - code += "} // namespace " + *it + "\n"; - } + CloseNestedNameSpaces(name_space, &code); // Close the include guard. code += "\n#endif // " + include_guard + "\n"; diff --git a/src/idl_gen_go.cpp b/src/idl_gen_go.cpp index f15955b..95ebf37 100755 --- a/src/idl_gen_go.cpp +++ b/src/idl_gen_go.cpp @@ -585,24 +585,24 @@ static bool SaveType(const Parser &parser, const Definition &def, bool needs_imports) { if (!classcode.length()) return true; - std::string name_space_name; - std::string name_space_dir = path; - for (auto it = parser.name_space_.begin(); - it != parser.name_space_.end(); ++it) { - if (name_space_name.length()) { - name_space_name += "."; - name_space_dir += PATH_SEPARATOR; + std::string namespace_name; + std::string namespace_dir = path; + auto &namespaces = parser.namespaces_.back()->components; + for (auto it = namespaces.begin(); it != namespaces.end(); ++it) { + if (namespace_name.length()) { + namespace_name += "."; + namespace_dir += PATH_SEPARATOR; } - name_space_name = *it; - name_space_dir += *it; - mkdir(name_space_dir.c_str(), S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); + namespace_name = *it; + namespace_dir += *it; + mkdir(namespace_dir.c_str(), S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); } std::string code = ""; - BeginFile(name_space_name, needs_imports, &code); + BeginFile(namespace_name, needs_imports, &code); code += classcode; - std::string filename = name_space_dir + PATH_SEPARATOR + def.name + ".go"; + std::string filename = namespace_dir + PATH_SEPARATOR + def.name + ".go"; return SaveFile(filename.c_str(), code, false); } diff --git a/src/idl_gen_java.cpp b/src/idl_gen_java.cpp index 0f114f0..b6fc148 100755 --- a/src/idl_gen_java.cpp +++ b/src/idl_gen_java.cpp @@ -341,27 +341,27 @@ static bool SaveClass(const Parser &parser, const Definition &def, bool needs_imports) { if (!classcode.length()) return true; - std::string name_space_java; - std::string name_space_dir = path; - for (auto it = parser.name_space_.begin(); - it != parser.name_space_.end(); ++it) { - if (name_space_java.length()) { - name_space_java += "."; - name_space_dir += kPathSeparator; + std::string namespace_java; + std::string namespace_dir = path; + auto &namespaces = parser.namespaces_.back()->components; + for (auto it = namespaces.begin(); it != namespaces.end(); ++it) { + if (namespace_java.length()) { + namespace_java += "."; + namespace_dir += kPathSeparator; } - name_space_java += *it; - name_space_dir += *it; - mkdir(name_space_dir.c_str(), S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); + namespace_java += *it; + namespace_dir += *it; + mkdir(namespace_dir.c_str(), S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); } std::string code = "// automatically generated, do not modify\n\n"; - code += "package " + name_space_java + ";\n\n"; + code += "package " + namespace_java + ";\n\n"; if (needs_imports) { code += "import java.nio.*;\nimport java.lang.*;\nimport java.util.*;\n"; code += "import flatbuffers.*;\n\n"; } code += classcode; - auto filename = name_space_dir + kPathSeparator + def.name + ".java"; + auto filename = namespace_dir + kPathSeparator + def.name + ".java"; return SaveFile(filename.c_str(), code, false); } diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index a5b3f4e..555d5e6 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -647,6 +647,7 @@ StructDef *Parser::LookupCreateStruct(const std::string &name) { structs_.Add(name, struct_def); struct_def->name = name; struct_def->predecl = true; + struct_def->defined_namespace = namespaces_.back(); } return struct_def; } @@ -838,9 +839,10 @@ bool Parser::Parse(const char *source, const char *filepath) { while (token_ != kTokenEof) { if (token_ == kTokenNameSpace) { Next(); - name_space_.clear(); + auto ns = new Namespace(); + namespaces_.push_back(ns); for (;;) { - name_space_.push_back(attribute_); + ns->components.push_back(attribute_); Expect(kTokenIdentifier); if (!IsNext('.')) break; } diff --git a/tests/include_test2.fbs b/tests/include_test2.fbs index 3b9c86e..d22c0d9 100644 --- a/tests/include_test2.fbs +++ b/tests/include_test2.fbs @@ -1,4 +1,9 @@ include "include_test2.fbs"; // should be skipped +namespace MyGame.OtherNameSpace; + enum FromInclude:long { IncludeVal } +struct Unused {} + + diff --git a/tests/monster_test_generated.h b/tests/monster_test_generated.h index 40794f7..bf0719e 100755 --- a/tests/monster_test_generated.h +++ b/tests/monster_test_generated.h @@ -6,8 +6,18 @@ #include "flatbuffers/flatbuffers.h" namespace MyGame { +namespace OtherNameSpace { +struct Unused; +} // namespace OtherNameSpace +} // namespace MyGame + +namespace MyGame { namespace Example { +struct Test; +struct Vec3; +struct Monster; + enum { Color_Red = 1, Color_Green = 2, @@ -35,10 +45,6 @@ inline const char *EnumNameAny(int e) { return EnumNamesAny()[e]; } bool VerifyAny(const flatbuffers::Verifier &verifier, const void *union_obj, uint8_t type); -struct Test; -struct Vec3; -struct Monster; - MANUALLY_ALIGNED_STRUCT(2) Test { private: int16_t a_; -- 2.7.4