From 0e85eeef2c6ed3eb9ec201aaea6caa62612a8522 Mon Sep 17 00:00:00 2001 From: Robbie McElrath Date: Mon, 26 Jun 2017 09:07:02 -0700 Subject: [PATCH] Make inter-file cycles compile (#4364) --- include/flatbuffers/idl.h | 9 ++-- src/idl_parser.cpp | 66 ++++++++++++++---------- tests/include_test/include_test1.fbs | 4 +- tests/include_test/sub/include_test2.fbs | 5 +- 4 files changed, 52 insertions(+), 32 deletions(-) diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 68426c25426..d40481245a7 100644 --- 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 &fields, StructDef *struct_def, const char *suffix, diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index b572b9fd26b..2cce5194583 100644 --- 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 index 39bc666beaf..804856a28df 100644 --- 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 index 13aa229e887..3257ffc144e 100644 --- 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; +}