Make inter-file cycles compile (#4364)

google · Jun 26, 2017 · 0e85eee · 0e85eee
1 parent b0fa5e0
commit 0e85eee
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 32 deletions.
diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h
@@ -607,10 +607,13 @@ class Parser : public ParserState {
   FLATBUFFERS_CHECKED_ERROR ParseFlexBufferValue(flexbuffers::Builder *builder);
   FLATBUFFERS_CHECKED_ERROR StartParseFile(const char *source,
                                            const char *source_filename);
-  FLATBUFFERS_CHECKED_ERROR DoParse(const char *_source,
+  FLATBUFFERS_CHECKED_ERROR ParseRoot(const char *_source,
                                     const char **include_paths,
-                                    const char *source_filename,
-                                    const char *include_filename);
+                                    const char *source_filename);
+  FLATBUFFERS_CHECKED_ERROR DoParse(const char *_source,
+                                           const char **include_paths,
+                                           const char *source_filename,
+                                           const char *include_filename);
   FLATBUFFERS_CHECKED_ERROR CheckClash(std::vector<FieldDef*> &fields,
                                        StructDef *struct_def,
                                        const char *suffix,

diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp
@@ -1632,7 +1632,9 @@ void Parser::MarkGenerated() {
   }
   for (auto it = structs_.vec.begin();
            it != structs_.vec.end(); ++it) {
-    (*it)->generated = true;
+    if (!(*it)->predecl) {
+      (*it)->generated = true;
+    }
   }
   for (auto it = services_.vec.begin();
            it != services_.vec.end(); ++it) {
@@ -2001,7 +2003,7 @@ bool Parser::ParseFlexBuffer(const char *source, const char *source_filename,
 
 bool Parser::Parse(const char *source, const char **include_paths,
                    const char *source_filename) {
-  return !DoParse(source, include_paths, source_filename, nullptr).Check();
+  return !ParseRoot(source, include_paths, source_filename).Check();
 }
 
 CheckedError Parser::StartParseFile(const char *source, const char *source_filename) {
@@ -2016,9 +2018,41 @@ CheckedError Parser::StartParseFile(const char *source, const char *source_filen
   return NoError();
 }
 
-CheckedError Parser::DoParse(const char *source, const char **include_paths,
-                             const char *source_filename,
-                             const char *include_filename) {
+CheckedError Parser::ParseRoot(const char *source, const char **include_paths,
+                             const char *source_filename) {
+  ECHECK(DoParse(source, include_paths, source_filename, nullptr));
+
+  // Check that all types were defined.
+  for (auto it = structs_.vec.begin(); it != structs_.vec.end(); ++it) {
+    if ((*it)->predecl) {
+      return Error("type referenced but not defined: " + (*it)->name);
+    }
+  }
+
+  // This check has to happen here and not earlier, because only now do we
+  // know for sure what the type of these are.
+  for (auto it = enums_.vec.begin(); it != enums_.vec.end(); ++it) {
+    auto &enum_def = **it;
+    if (enum_def.is_union) {
+      for (auto val_it = enum_def.vals.vec.begin();
+           val_it != enum_def.vals.vec.end();
+           ++val_it) {
+        auto &val = **val_it;
+        if (opts.lang_to_generate != IDLOptions::kCpp &&
+            val.union_type.struct_def && val.union_type.struct_def->fixed)
+          return Error(
+                "only tables can be union elements in the generated language: "
+                + val.name);
+      }
+    }
+  }
+  return NoError();
+}
+
+CheckedError Parser::DoParse(const char *source,
+                                    const char **include_paths,
+                                    const char *source_filename,
+                                    const char *include_filename) {
   if (source_filename &&
       included_files_.find(source_filename) == included_files_.end()) {
     included_files_[source_filename] = include_filename ? include_filename : "";
@@ -2147,28 +2181,6 @@ CheckedError Parser::DoParse(const char *source, const char **include_paths,
       ECHECK(ParseDecl());
     }
   }
-  for (auto it = structs_.vec.begin(); it != structs_.vec.end(); ++it) {
-    if ((*it)->predecl) {
-      return Error("type referenced but not defined: " + (*it)->name);
-    }
-  }
-  // This check has to happen here and not earlier, because only now do we
-  // know for sure what the type of these are.
-  for (auto it = enums_.vec.begin(); it != enums_.vec.end(); ++it) {
-    auto &enum_def = **it;
-    if (enum_def.is_union) {
-      for (auto val_it = enum_def.vals.vec.begin();
-           val_it != enum_def.vals.vec.end();
-           ++val_it) {
-        auto &val = **val_it;
-        if (opts.lang_to_generate != IDLOptions::kCpp &&
-            val.union_type.struct_def && val.union_type.struct_def->fixed)
-          return Error(
-                "only tables can be union elements in the generated language: "
-                + val.name);
-      }
-    }
-  }
   return NoError();
 }
 

diff --git a/tests/include_test/include_test1.fbs b/tests/include_test/include_test1.fbs
@@ -2,4 +2,6 @@ include "sub/include_test2.fbs";
 include "sub/include_test2.fbs";  // should be skipped
 include "include_test1.fbs";  // should be skipped
 
-
+table TableA {
+  b:MyGame.OtherNameSpace.TableB;
+}
diff --git a/tests/include_test/sub/include_test2.fbs b/tests/include_test/sub/include_test2.fbs
@@ -1,3 +1,4 @@
+include "include_test1.fbs";
 include "sub/include_test2.fbs";    // should be skipped
 
 namespace MyGame.OtherNameSpace;
@@ -6,4 +7,6 @@ enum FromInclude:long { IncludeVal }
 
 struct Unused {}
 
-
+table TableB {
+  a:TableA;
+}