Skip to content

Commit

Permalink
inject no long for FBS generation to remove logs in flattests
Browse files Browse the repository at this point in the history
  • Loading branch information
dbaileychess committed Apr 28, 2023
1 parent e7dc252 commit 35f8340
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 37 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,7 @@ if(FLATBUFFERS_BUILD_TESTS)
add_executable(flattests ${FlatBuffers_Tests_SRCS})
target_link_libraries(flattests PRIVATE $<BUILD_INTERFACE:ProjectConfig>)

target_include_directories(flattests PUBLIC src)
add_dependencies(flattests generated_code)

if(FLATBUFFERS_CODE_SANITIZE)
Expand Down
5 changes: 3 additions & 2 deletions include/flatbuffers/idl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1287,9 +1287,10 @@ extern bool GenerateSwift(const Parser &parser, const std::string &path,
// Generate a schema file from the internal representation, useful after
// parsing a .proto schema.
extern std::string GenerateFBS(const Parser &parser,
const std::string &file_name);
const std::string &file_name,
bool no_log);
extern bool GenerateFBS(const Parser &parser, const std::string &path,
const std::string &file_name);
const std::string &file_name, bool no_log);

// Generate a make rule for the generated TypeScript code.
// See idl_gen_ts.cpp.
Expand Down
68 changes: 44 additions & 24 deletions src/idl_gen_fbs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,34 +129,43 @@ static bool HasGapInProtoId(const std::vector<FieldDef *> &fields) {
}

static bool ProtobufIdSanityCheck(const StructDef &struct_def,
IDLOptions::ProtoIdGapAction gap_action) {
IDLOptions::ProtoIdGapAction gap_action,
bool no_log = false) {
const auto &fields = struct_def.fields.vec;
if (HasNonPositiveFieldId(fields)) {
// TODO: Use LogCompilerWarn
fprintf(stderr, "Field id in struct %s has a non positive number value\n",
struct_def.name.c_str());
if (!no_log) {
fprintf(stderr, "Field id in struct %s has a non positive number value\n",
struct_def.name.c_str());
}
return false;
}

if (HasTwiceUsedId(fields)) {
// TODO: Use LogCompilerWarn
fprintf(stderr, "Fields in struct %s have used an id twice\n",
struct_def.name.c_str());
if (!no_log) {
fprintf(stderr, "Fields in struct %s have used an id twice\n",
struct_def.name.c_str());
}
return false;
}

if (HasFieldIdFromReservedIds(fields, struct_def.reserved_ids)) {
// TODO: Use LogCompilerWarn
fprintf(stderr, "Fields in struct %s use id from reserved ids\n",
struct_def.name.c_str());
if (!no_log) {
fprintf(stderr, "Fields in struct %s use id from reserved ids\n",
struct_def.name.c_str());
}
return false;
}

if (gap_action != IDLOptions::ProtoIdGapAction::NO_OP) {
if (HasGapInProtoId(fields)) {
// TODO: Use LogCompilerWarn
fprintf(stderr, "Fields in struct %s have gap between ids\n",
struct_def.name.c_str());
if (!no_log) {
fprintf(stderr, "Fields in struct %s have gap between ids\n",
struct_def.name.c_str());
}
if (gap_action == IDLOptions::ProtoIdGapAction::ERROR) { return false; }
}
}
Expand All @@ -174,7 +183,8 @@ struct ProtobufToFbsIdMap {
};

static ProtobufToFbsIdMap MapProtoIdsToFieldsId(
const StructDef &struct_def, IDLOptions::ProtoIdGapAction gap_action) {
const StructDef &struct_def, IDLOptions::ProtoIdGapAction gap_action,
bool no_log) {
const auto &fields = struct_def.fields.vec;

if (!HasFieldWithId(fields)) {
Expand All @@ -183,7 +193,7 @@ static ProtobufToFbsIdMap MapProtoIdsToFieldsId(
return result;
}

if (!ProtobufIdSanityCheck(struct_def, gap_action)) { return {}; }
if (!ProtobufIdSanityCheck(struct_def, gap_action, no_log)) { return {}; }

static constexpr int UNION_ID = -1;
using ProtoIdFieldNamePair = std::pair<int, std::string>;
Expand All @@ -203,8 +213,10 @@ static ProtobufToFbsIdMap MapProtoIdsToFieldsId(
}
} else {
// TODO: Use LogCompilerWarn
fprintf(stderr, "Fields id in struct %s is missing\n",
struct_def.name.c_str());
if (!no_log) {
fprintf(stderr, "Fields id in struct %s is missing\n",
struct_def.name.c_str());
}
return {};
}
}
Expand Down Expand Up @@ -240,7 +252,8 @@ static void GenNameSpace(const Namespace &name_space, std::string *_schema,
}

// Generate a flatbuffer schema from the Parser's internal representation.
std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
std::string GenerateFBS(const Parser &parser, const std::string &file_name,
bool no_log = false) {
// Proto namespaces may clash with table names, escape the ones that were
// generated from a table:
for (auto it = parser.namespaces_.begin(); it != parser.namespaces_.end();
Expand Down Expand Up @@ -315,8 +328,8 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
for (auto it = parser.structs_.vec.begin(); it != parser.structs_.vec.end();
++it) {
StructDef &struct_def = **it;
const auto proto_fbs_ids =
MapProtoIdsToFieldsId(struct_def, parser.opts.proto_id_gap_action);
const auto proto_fbs_ids = MapProtoIdsToFieldsId(
struct_def, parser.opts.proto_id_gap_action, no_log);
if (!proto_fbs_ids.successful) { return {}; }

if (parser.opts.include_dependence_headers && struct_def.generated) {
Expand Down Expand Up @@ -362,23 +375,27 @@ std::string GenerateFBS(const Parser &parser, const std::string &file_name) {
}

bool GenerateFBS(const Parser &parser, const std::string &path,
const std::string &file_name) {
const std::string fbs = GenerateFBS(parser, file_name);
const std::string &file_name, bool no_log = false) {
const std::string fbs = GenerateFBS(parser, file_name, no_log);
if (fbs.empty()) { return false; }
// TODO: Use LogCompilerWarn
fprintf(stderr,
"When you use --proto, that you should check for conformity "
"yourself, using the existing --conform");
if (!no_log) {
fprintf(stderr,
"When you use --proto, that you should check for conformity "
"yourself, using the existing --conform");
}
return SaveFile((path + file_name + ".fbs").c_str(), fbs, false);
}

namespace {

class FBSCodeGenerator : public CodeGenerator {
public:
explicit FBSCodeGenerator(const bool no_log) : no_log_(no_log) {}

Status GenerateCode(const Parser &parser, const std::string &path,
const std::string &filename) override {
if (!GenerateFBS(parser, path, filename)) { return Status::ERROR; }
if (!GenerateFBS(parser, path, filename, no_log_)) { return Status::ERROR; }
return Status::OK;
}

Expand Down Expand Up @@ -424,12 +441,15 @@ class FBSCodeGenerator : public CodeGenerator {
IDLOptions::Language Language() const override { return IDLOptions::kProto; }

std::string LanguageName() const override { return "proto"; }

protected:
const bool no_log_;
};

} // namespace

std::unique_ptr<CodeGenerator> NewFBSCodeGenerator() {
return std::unique_ptr<FBSCodeGenerator>(new FBSCodeGenerator());
std::unique_ptr<CodeGenerator> NewFBSCodeGenerator(const bool no_log) {
return std::unique_ptr<FBSCodeGenerator>(new FBSCodeGenerator(no_log));
}

} // namespace flatbuffers
2 changes: 1 addition & 1 deletion src/idl_gen_fbs.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

namespace flatbuffers {

std::unique_ptr<CodeGenerator> NewFBSCodeGenerator();
std::unique_ptr<CodeGenerator> NewFBSCodeGenerator(bool no_log = false);

} // namespace flatbuffers

Expand Down
2 changes: 2 additions & 0 deletions tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ cc_test(
includes = [
"",
"include/",
"src/",
],
deps = [
":alignment_test_cc_fbs",
Expand All @@ -113,6 +114,7 @@ cc_test(
":monster_test_cc_fbs",
":native_type_test_cc_fbs",
"//:flatbuffers",
"//src:flatbuffers",
],
)

Expand Down
25 changes: 15 additions & 10 deletions tests/proto_test.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "proto_test.h"

#include "flatbuffers/code_generator.h"
#include "idl_gen_fbs.h"
#include "test_assert.h"

namespace flatbuffers {
Expand All @@ -15,7 +17,7 @@ void RunTest(const flatbuffers::IDLOptions &opts, const std::string &proto_path,
TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);

// Generate fbs.
auto fbs = flatbuffers::GenerateFBS(parser, "test");
auto fbs = flatbuffers::GenerateFBS(parser, "test", true);

// Ensure generated file is parsable.
flatbuffers::Parser parser2;
Expand All @@ -25,7 +27,7 @@ void RunTest(const flatbuffers::IDLOptions &opts, const std::string &proto_path,
flatbuffers::Parser import_parser(opts);
TEST_EQ(import_parser.Parse(import_proto_file.c_str(), include_directories),
true);
auto import_fbs = flatbuffers::GenerateFBS(import_parser, "test");
auto import_fbs = flatbuffers::GenerateFBS(import_parser, "test", true);
// Since `imported.fbs` isn't in the filesystem AbsolutePath can't figure it
// out by itself. We manually construct it so Parser works.
std::string imported_fbs = flatbuffers::PosixPath(
Expand Down Expand Up @@ -222,6 +224,8 @@ void ParseCorruptedProto(const std::string &proto_path) {

std::string proto_file;

std::unique_ptr<CodeGenerator> fbs_generator = NewFBSCodeGenerator(true);

// Parse proto with non positive id.
{
flatbuffers::Parser parser(opts);
Expand All @@ -230,8 +234,8 @@ void ParseCorruptedProto(const std::string &proto_path) {
false, &proto_file),
true);
TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
auto fbs = flatbuffers::GenerateFBS(parser, "test");
TEST_EQ(fbs.empty(), true);
TEST_NE(fbs_generator->GenerateCode(parser, "temp.fbs", "test"),
CodeGenerator::Status::OK);
}

// Parse proto with twice id.
Expand All @@ -241,8 +245,8 @@ void ParseCorruptedProto(const std::string &proto_path) {
false, &proto_file),
true);
TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
auto fbs = flatbuffers::GenerateFBS(parser, "test");
TEST_EQ(fbs.empty(), true);
TEST_NE(fbs_generator->GenerateCode(parser, "temp.fbs", "test"),
CodeGenerator::Status::OK);
}

// Parse proto with using reserved id.
Expand All @@ -252,8 +256,8 @@ void ParseCorruptedProto(const std::string &proto_path) {
false, &proto_file),
true);
TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
auto fbs = flatbuffers::GenerateFBS(parser, "test");
TEST_EQ(fbs.empty(), true);
TEST_NE(fbs_generator->GenerateCode(parser, "temp.fbs", "test"),
CodeGenerator::Status::OK);
}

// Parse proto with error on gap.
Expand All @@ -264,8 +268,9 @@ void ParseCorruptedProto(const std::string &proto_path) {
&proto_file),
true);
TEST_EQ(parser.Parse(proto_file.c_str(), include_directories), true);
auto fbs = flatbuffers::GenerateFBS(parser, "test");
TEST_EQ(fbs.empty(), true);

TEST_NE(fbs_generator->GenerateCode(parser, "temp.fbs", "test"),
CodeGenerator::Status::OK);
}
}

Expand Down
21 changes: 21 additions & 0 deletions tests/test_assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#endif

#define TEST_EQ(exp, val) TestEq(exp, val, "'" #exp "' != '" #val "'", __FILE__, __LINE__, "")
#define TEST_NE(exp, val) TestNe(exp, val, "'" #exp "' == '" #val "'", __FILE__, __LINE__, "")
#define TEST_ASSERT(val) TestEq(true, !!(val), "'" "true" "' != '" #val "'", __FILE__, __LINE__, "")
#define TEST_NOTNULL(val) TestEq(true, (val) != nullptr, "'" "nullptr" "' == '" #val "'", __FILE__, __LINE__, "")
#define TEST_EQ_STR(exp, val) TestEqStr(exp, val, "'" #exp "' != '" #val "'", __FILE__, __LINE__, "")
Expand Down Expand Up @@ -106,4 +107,24 @@ inline void TestEq<std::string, std::string>(std::string expval,
}
}

template<typename T, typename U>
void TestNe(T expval, U val, const char *exp, const char *file, int line,
const char *func) {
if (static_cast<U>(expval) == val) {
TestFail(flatbuffers::NumToString(scalar_as_underlying(expval)).c_str(),
flatbuffers::NumToString(scalar_as_underlying(val)).c_str(), exp,
file, line, func);
}
}

template<>
inline void TestNe<std::string, std::string>(std::string expval,
std::string val, const char *exp,
const char *file, int line,
const char *func) {
if (expval == val) {
TestFail(expval.c_str(), val.c_str(), exp, file, line, func);
}
}

#endif // !TEST_ASSERT_H

0 comments on commit 35f8340

Please sign in to comment.