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

@@ -159,6 +159,11 @@ template<typename T> class SymbolTable {
   std::vector<T *> vec;  // Used to iterate in order of insertion
 };
 
+// A name space, as set in the schema.
+struct Namespace {
+  std::vector<std::string> 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<StructDef> structs_;
   SymbolTable<EnumDef> enums_;
-  std::vector<std::string> name_space_;  // As set in the schema.
+  std::vector<Namespace *> namespaces_;
   std::string error_;         // User readable error_ if Parse() == false
 
   FlatBufferBuilder builder_;  // any data contained in the file

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 "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";

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);
 }
 

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);
 }
 

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;
         }

tests/include_test2.fbs

 include "include_test2.fbs";    // should be skipped
 
+namespace MyGame.OtherNameSpace;
+
 enum FromInclude:long { IncludeVal }
 
+struct Unused {}
+
+

tests/monster_test_generated.h

@@ -5,9 +5,19 @@
 
 #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_;