From 22743ca45a3f835df423eb608837022a4a57dcc0 Mon Sep 17 00:00:00 2001 From: Wouter van Oortmerssen <aardappel@gmail.com> Date: Wed, 24 May 2017 15:21:26 -0700 Subject: [PATCH] Fixed --keep-prefix functionality. Changing to keep include prefixes had two side effects: the main file being parsed wasn't filtered out anymore, and include directory paths would be added to the path in the include statement. Also moved the include_test*.fbs files to sub directories so we can actually test the handling of -I etc. tested: on Linux. Change-Id: Ibae095cea7ab0cccbac15cfb5171719f6b5cad8c --- CMakeLists.txt | 1 + appveyor.yml | 2 +- include/flatbuffers/idl.h | 3 ++- src/idl_gen_cpp.cpp | 8 ++++---- src/idl_parser.cpp | 17 ++++++++++------- tests/GoTest.sh | 2 +- tests/JavaScriptTest.sh | 2 +- tests/PythonTest.sh | 2 +- tests/TypeScriptTest.sh | 4 ++-- tests/generate_code.bat | 4 ++-- tests/generate_code.sh | 5 ++--- tests/include_test/include_test1.fbs | 5 +++++ tests/{ => include_test/sub}/include_test2.fbs | 2 +- tests/include_test1.fbs | 5 ----- tests/test.cpp | 6 +++++- 15 files changed, 38 insertions(+), 30 deletions(-) create mode 100644 tests/include_test/include_test1.fbs rename tests/{ => include_test/sub}/include_test2.fbs (61%) delete mode 100644 tests/include_test1.fbs diff --git a/CMakeLists.txt b/CMakeLists.txt index 4038104f..d3139807 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -195,6 +195,7 @@ function(compile_flatbuffers_schema_to_cpp SRC_FBS) OUTPUT ${GEN_HEADER} COMMAND "${FLATBUFFERS_FLATC_EXECUTABLE}" -c --no-includes --gen-mutable --gen-object-api -o "${SRC_FBS_DIR}" + -I "${CMAKE_CURRENT_SOURCE_DIR}/tests/include_test" "${CMAKE_CURRENT_SOURCE_DIR}/${SRC_FBS}" DEPENDS flatc) endfunction() diff --git a/appveyor.yml b/appveyor.yml index 0dd5a2ed..262d9b07 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -35,7 +35,7 @@ test_script: - "JavaTest.bat" - rem "---------------- JS -----------------" - "node --version" - - "..\\%CONFIGURATION%\\flatc -b monster_test.fbs unicode_test.json" + - "..\\%CONFIGURATION%\\flatc -b -I include_test monster_test.fbs unicode_test.json" - "node JavaScriptTest ./monster_test_generated" - rem "---------------- C# -----------------" # Have to compile this here rather than in "build" above because AppVeyor only diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 262d2193..2781852c 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -590,7 +590,8 @@ private: FLATBUFFERS_CHECKED_ERROR SkipJsonString(); FLATBUFFERS_CHECKED_ERROR DoParse(const char *_source, const char **include_paths, - const char *source_filename); + 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_gen_cpp.cpp b/src/idl_gen_cpp.cpp index 37899044..6bd958f5 100644 --- a/src/idl_gen_cpp.cpp +++ b/src/idl_gen_cpp.cpp @@ -72,11 +72,11 @@ class CppGenerator : public BaseGenerator { } for (auto it = parser_.included_files_.begin(); it != parser_.included_files_.end(); ++it) { - auto basename = flatbuffers::StripExtension(it->first); - if (!parser_.opts.keep_include_path) - basename = flatbuffers::StripPath(basename); + auto noext = flatbuffers::StripExtension(it->first); + auto basename = flatbuffers::StripPath(noext); if (basename != file_name_) { - code_ += "#include \"" + parser_.opts.include_prefix + basename + + code_ += "#include \"" + parser_.opts.include_prefix + + (parser_.opts.keep_include_path ? noext : basename) + "_generated.h\""; num_includes++; } diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index 4c255627..f03ad9a8 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -1917,15 +1917,17 @@ CheckedError Parser::SkipJsonString() { bool Parser::Parse(const char *source, const char **include_paths, const char *source_filename) { - return !DoParse(source, include_paths, source_filename).Check(); + return !DoParse(source, include_paths, source_filename, + source_filename).Check(); } CheckedError Parser::DoParse(const char *source, const char **include_paths, - const char *source_filename) { + const char *source_filename, + const char *include_filename) { file_being_parsed_ = source_filename ? source_filename : ""; if (source_filename && - included_files_.find(source_filename) == included_files_.end()) { - included_files_[source_filename] = true; + included_files_.find(include_filename) == included_files_.end()) { + included_files_[include_filename] = true; files_included_per_file_[source_filename] = std::set<std::string>(); } if (!include_paths) { @@ -1974,13 +1976,14 @@ CheckedError Parser::DoParse(const char *source, const char **include_paths, return Error("unable to locate include file: " + name); if (source_filename) files_included_per_file_[source_filename].insert(filepath); - if (included_files_.find(filepath) == included_files_.end()) { + if (included_files_.find(name) == included_files_.end()) { // We found an include file that we have not parsed yet. // Load it and parse it. std::string contents; if (!LoadFile(filepath.c_str(), true, &contents)) return Error("unable to load include file: " + name); - ECHECK(DoParse(contents.c_str(), include_paths, filepath.c_str())); + ECHECK(DoParse(contents.c_str(), include_paths, filepath.c_str(), + name.c_str())); // We generally do not want to output code for any included files: if (!opts.generate_all) MarkGenerated(); // This is the easiest way to continue this file after an include: @@ -1990,7 +1993,7 @@ CheckedError Parser::DoParse(const char *source, const char **include_paths, // entered into included_files_. // This is recursive, but only go as deep as the number of include // statements. - return DoParse(source, include_paths, source_filename); + return DoParse(source, include_paths, source_filename, include_filename); } EXPECT(';'); } else { diff --git a/tests/GoTest.sh b/tests/GoTest.sh index 7be4affb..248a13f2 100755 --- a/tests/GoTest.sh +++ b/tests/GoTest.sh @@ -20,7 +20,7 @@ go_path=${test_dir}/go_gen go_src=${go_path}/src # Emit Go code for the example schema in the test dir: -../flatc -g monster_test.fbs +../flatc -g -I include_test monster_test.fbs # Go requires a particular layout of files in order to link multiple packages. # Copy flatbuffer Go files to their own package directories to compile the diff --git a/tests/JavaScriptTest.sh b/tests/JavaScriptTest.sh index cdba5fa6..c0286a05 100755 --- a/tests/JavaScriptTest.sh +++ b/tests/JavaScriptTest.sh @@ -15,5 +15,5 @@ # limitations under the License. pushd "$(dirname $0)" >/dev/null -../flatc -b monster_test.fbs unicode_test.json +../flatc -b -I include_test monster_test.fbs unicode_test.json node JavaScriptTest ./monster_test_generated diff --git a/tests/PythonTest.sh b/tests/PythonTest.sh index 00fee7c7..e4dbe8da 100755 --- a/tests/PythonTest.sh +++ b/tests/PythonTest.sh @@ -20,7 +20,7 @@ gen_code_path=${test_dir} runtime_library_dir=${test_dir}/../python # Emit Python code for the example schema in the test dir: -${test_dir}/../flatc -p -o ${gen_code_path} monster_test.fbs +${test_dir}/../flatc -p -o ${gen_code_path} -I include_test monster_test.fbs # Syntax: run_tests <interpreter> <benchmark vtable dedupes> # <benchmark read count> <benchmark build count> diff --git a/tests/TypeScriptTest.sh b/tests/TypeScriptTest.sh index 54c6d086..b033a302 100755 --- a/tests/TypeScriptTest.sh +++ b/tests/TypeScriptTest.sh @@ -15,8 +15,8 @@ # limitations under the License. pushd "$(dirname $0)" >/dev/null -../flatc --ts --no-fb-import --gen-mutable -o ts monster_test.fbs -../flatc -b monster_test.fbs unicode_test.json +../flatc --ts --no-fb-import --gen-mutable -o ts -I include_test monster_test.fbs +../flatc -b -I include_test monster_test.fbs unicode_test.json npm install @types/flatbuffers tsc --strict --noUnusedParameters --noUnusedLocals --noImplicitReturns --strictNullChecks ts/monster_test_generated.ts npm uninstall @types/flatbuffers diff --git a/tests/generate_code.bat b/tests/generate_code.bat index e7660a2f..2abfe9b3 100644 --- a/tests/generate_code.bat +++ b/tests/generate_code.bat @@ -15,6 +15,6 @@ set buildtype=Release if "%1"=="-b" set buildtype=%2 -..\%buildtype%\flatc.exe --cpp --java --csharp --go --binary --python --js --php --grpc --gen-mutable --gen-object-api --no-includes monster_test.fbs monsterdata_test.json +..\%buildtype%\flatc.exe --cpp --java --csharp --go --binary --python --js --php --grpc --gen-mutable --gen-object-api --no-includes -I include_test monster_test.fbs monsterdata_test.json ..\%buildtype%\flatc.exe --cpp --java --csharp --go --binary --python --js --php --gen-mutable -o namespace_test namespace_test\namespace_test1.fbs namespace_test\namespace_test2.fbs -..\%buildtype%\flatc.exe --binary --schema monster_test.fbs +..\%buildtype%\flatc.exe --binary --schema -I include_test monster_test.fbs diff --git a/tests/generate_code.sh b/tests/generate_code.sh index bdce8405..8e3ac511 100755 --- a/tests/generate_code.sh +++ b/tests/generate_code.sh @@ -14,11 +14,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -../flatc --cpp --java --csharp --go --binary --python --js --ts --php --grpc --gen-mutable --gen-object-api --no-includes --no-fb-import monster_test.fbs monsterdata_test.json +../flatc --cpp --java --csharp --go --binary --python --js --ts --php --grpc --gen-mutable --gen-object-api --no-includes --no-fb-import -I include_test monster_test.fbs monsterdata_test.json ../flatc --cpp --java --csharp --go --binary --python --js --ts --php --gen-mutable --no-fb-import -o namespace_test namespace_test/namespace_test1.fbs namespace_test/namespace_test2.fbs ../flatc --cpp --gen-mutable --gen-object-api -o union_vector ./union_vector/union_vector.fbs -../flatc -b --schema --bfbs-comments monster_test.fbs +../flatc -b --schema --bfbs-comments -I include_test monster_test.fbs cd ../samples ../flatc --cpp --gen-mutable --gen-object-api monster.fbs cd ../reflection -sh generate_code.sh diff --git a/tests/include_test/include_test1.fbs b/tests/include_test/include_test1.fbs new file mode 100644 index 00000000..39bc666b --- /dev/null +++ b/tests/include_test/include_test1.fbs @@ -0,0 +1,5 @@ +include "sub/include_test2.fbs"; +include "sub/include_test2.fbs"; // should be skipped +include "include_test1.fbs"; // should be skipped + + diff --git a/tests/include_test2.fbs b/tests/include_test/sub/include_test2.fbs similarity index 61% rename from tests/include_test2.fbs rename to tests/include_test/sub/include_test2.fbs index d22c0d9b..13aa229e 100644 --- a/tests/include_test2.fbs +++ b/tests/include_test/sub/include_test2.fbs @@ -1,4 +1,4 @@ -include "include_test2.fbs"; // should be skipped +include "sub/include_test2.fbs"; // should be skipped namespace MyGame.OtherNameSpace; diff --git a/tests/include_test1.fbs b/tests/include_test1.fbs deleted file mode 100644 index 11aebe81..00000000 --- a/tests/include_test1.fbs +++ /dev/null @@ -1,5 +0,0 @@ -include "include_test2.fbs"; -include "include_test2.fbs"; // should be skipped -include "include_test1.fbs"; // should be skipped - - diff --git a/tests/test.cpp b/tests/test.cpp index 3f5cddc4..3141a968 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -472,7 +472,11 @@ void ParseAndGenerateTextTest() { // parse schema first, so we can use it to parse the data after flatbuffers::Parser parser; - const char *include_directories[] = { test_data_path.c_str(), nullptr }; + auto include_test_path = + flatbuffers::ConCatPathFileName(test_data_path, "include_test"); + const char *include_directories[] = { + test_data_path.c_str(), include_test_path.c_str(), nullptr + }; TEST_EQ(parser.Parse(schemafile.c_str(), include_directories), true); TEST_EQ(parser.Parse(jsonfile.c_str(), include_directories), true); -- GitLab