Skip to content

Commit

Permalink
Fixed --keep-prefix functionality.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aardappel committed May 24, 2017
1 parent fb87c0d commit 22743ca
Show file tree
Hide file tree
Showing 15 changed files with 38 additions and 30 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion include/flatbuffers/idl.h
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,8 @@ class Parser : public ParserState {
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,
Expand Down
8 changes: 4 additions & 4 deletions src/idl_gen_cpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}
Expand Down
17 changes: 10 additions & 7 deletions src/idl_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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:
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/GoTest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/JavaScriptTest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion tests/PythonTest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions tests/TypeScriptTest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/generate_code.bat
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 2 additions & 3 deletions tests/generate_code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions tests/include_test/include_test1.fbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
include "sub/include_test2.fbs";
include "sub/include_test2.fbs"; // should be skipped
include "include_test1.fbs"; // should be skipped


Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
include "include_test2.fbs"; // should be skipped
include "sub/include_test2.fbs"; // should be skipped

namespace MyGame.OtherNameSpace;

Expand Down
5 changes: 0 additions & 5 deletions tests/include_test1.fbs

This file was deleted.

6 changes: 5 additions & 1 deletion tests/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 22743ca

Please sign in to comment.