diff --git a/grpc/src/compiler/python_generator.cc b/grpc/src/compiler/python_generator.cc index 7e50266354e..027c4a46eb5 100644 --- a/grpc/src/compiler/python_generator.cc +++ b/grpc/src/compiler/python_generator.cc @@ -130,7 +130,7 @@ class StubGenerator : public BaseGenerator { const Version& version) : BaseGenerator(parser, kStubConfig, path, version) {} - bool Generate() { + const char* Generate() { Imports imports; std::stringstream stub; std::string ns_name{}; @@ -150,8 +150,8 @@ class StubGenerator : public BaseGenerator { } private: - bool SaveStub(const std::string& filename, const Imports& imports, - const std::string& content) { + const char* SaveStub(const std::string& filename, const Imports& imports, + const std::string& content) { std::stringstream ss; ss << "# Generated by the gRPC FlatBuffers compiler. DO NOT EDIT!\n" << '\n' @@ -161,7 +161,8 @@ class StubGenerator : public BaseGenerator { ss << content << '\n'; EnsureDirExists(StripFileName(filename)); - return parser_.opts.file_saver->SaveFile(filename.c_str(), ss.str(), false); + return parser_.opts.file_saver->AttemptSave(filename.c_str(), ss.str(), + false); } void Generate(std::stringstream& ss, const ServiceDef* service, @@ -246,7 +247,7 @@ class ServiceGenerator : public BaseGenerator { const Version& version) : BaseGenerator(parser, kConfig, path, version) {} - bool Generate() { + const char* Generate() { Imports imports; std::stringstream ss; @@ -282,8 +283,8 @@ class ServiceGenerator : public BaseGenerator { } private: - bool SaveService(const std::string& filename, const Imports& imports, - const std::string& content) { + const char* SaveService(const std::string& filename, const Imports& imports, + const std::string& content) { std::stringstream ss; ss << "# Generated by the gRPC FlatBuffers compiler. DO NOT EDIT!\n" << '\n'; @@ -291,7 +292,8 @@ class ServiceGenerator : public BaseGenerator { ss << content << '\n'; EnsureDirExists(StripFileName(filename)); - return parser_.opts.file_saver->SaveFile(filename.c_str(), ss.str(), false); + return parser_.opts.file_saver->AttemptSave(filename.c_str(), ss.str(), + false); } void GenerateStub(std::stringstream& ss, const ServiceDef* service, @@ -395,14 +397,14 @@ class ServiceGenerator : public BaseGenerator { }; } // namespace -bool Generate(const Parser& parser, const std::string& path, - const Version& version) { +const char* Generate(const Parser& parser, const std::string& path, + const Version& version) { ServiceGenerator generator{parser, path, version}; return generator.Generate(); } -bool GenerateStub(const Parser& parser, const std::string& path, - const Version& version) { +const char* GenerateStub(const Parser& parser, const std::string& path, + const Version& version) { StubGenerator generator{parser, path, version}; return generator.Generate(); } diff --git a/grpc/src/compiler/python_generator.h b/grpc/src/compiler/python_generator.h index 5967e1f84c3..d4146be4e91 100644 --- a/grpc/src/compiler/python_generator.h +++ b/grpc/src/compiler/python_generator.h @@ -27,11 +27,11 @@ namespace flatbuffers { namespace python { namespace grpc { -bool Generate(const Parser& parser, const std::string& path, - const Version& version); +const char* Generate(const Parser& parser, const std::string& path, + const Version& version); -bool GenerateStub(const Parser& parser, const std::string& path, - const Version& version); +const char* GenerateStub(const Parser& parser, const std::string& path, + const Version& version); } // namespace grpc } // namespace python } // namespace flatbuffers diff --git a/include/flatbuffers/code_generators.h b/include/flatbuffers/code_generators.h index d284ac5a683..0e69eb8f12f 100644 --- a/include/flatbuffers/code_generators.h +++ b/include/flatbuffers/code_generators.h @@ -93,6 +93,10 @@ class BaseGenerator { public: virtual bool generate() = 0; + // Modern alternative to bool generate() as this provides a way to + // return a specific error message in case of an error and nullptr on success. + virtual const char* Generate() { return nullptr; } + static std::string NamespaceDir(const Parser& parser, const std::string& path, const Namespace& ns, const bool dasherize = false); diff --git a/include/flatbuffers/file_manager.h b/include/flatbuffers/file_manager.h index bafe6f29b97..ded50399b57 100644 --- a/include/flatbuffers/file_manager.h +++ b/include/flatbuffers/file_manager.h @@ -33,10 +33,18 @@ class FileSaver { virtual bool SaveFile(const char* name, const char* buf, size_t len, bool binary) = 0; + virtual const char* AttemptSave(const char* name, const char* buf, size_t len, + bool binary) = 0; + bool SaveFile(const char* name, const std::string& buf, bool binary) { return SaveFile(name, buf.c_str(), buf.size(), binary); } + const char* AttemptSave(const char* name, const std::string& buf, + bool binary) { + return AttemptSave(name, buf.c_str(), buf.size(), binary); + } + virtual void Finish() {} private: @@ -52,6 +60,9 @@ class RealFileSaver final : public FileSaver { public: bool SaveFile(const char* name, const char* buf, size_t len, bool binary) final; + + const char* AttemptSave(const char* name, const char* buf, size_t len, + bool binary) final; }; class FileNameSaver final : public FileSaver { @@ -59,6 +70,9 @@ class FileNameSaver final : public FileSaver { bool SaveFile(const char* name, const char* buf, size_t len, bool binary) final; + const char* AttemptSave(const char* name, const char* buf, size_t len, + bool binary) final; + void Finish() final; private: diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 4139529a11a..5426ad39157 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -1303,8 +1303,8 @@ bool GenerateJavaGRPC(const Parser& parser, const std::string& path, // Generate GRPC Python interfaces. // See idl_gen_grpc.cpp. -bool GeneratePythonGRPC(const Parser& parser, const std::string& path, - const std::string& file_name); +const char* GeneratePythonGRPC(const Parser& parser, const std::string& path, + const std::string& file_name); // Generate GRPC Swift interfaces. // See idl_gen_grpc.cpp. diff --git a/src/file_manager.cpp b/src/file_manager.cpp index 13b1580af04..276c5693014 100644 --- a/src/file_manager.cpp +++ b/src/file_manager.cpp @@ -14,12 +14,12 @@ * limitations under the License. */ +#include "flatbuffers/file_manager.h" + #include #include #include -#include "flatbuffers/file_manager.h" - namespace flatbuffers { bool RealFileSaver::SaveFile(const char* name, const char* buf, size_t len, @@ -30,4 +30,13 @@ bool RealFileSaver::SaveFile(const char* name, const char* buf, size_t len, return !ofs.bad(); } +const char* RealFileSaver::AttemptSave(const char* name, const char* buf, + size_t len, bool binary) { + if (!SaveFile(name, buf, len, binary)) { + static std::string error_msg; + error_msg = "Could not save file " + std::string(name ? name : "unknown"); + return error_msg.c_str(); + } + return nullptr; +} } // namespace flatbuffers diff --git a/src/file_name_manager.cpp b/src/file_name_manager.cpp index aad63de8c6b..a2d31a5133f 100644 --- a/src/file_name_manager.cpp +++ b/src/file_name_manager.cpp @@ -33,6 +33,12 @@ bool FileNameSaver::SaveFile(const char* name, const char* buf, size_t len, return true; } +const char* FileNameSaver::AttemptSave(const char* name, const char* buf, + size_t len, bool binary) { + return SaveFile(name, buf, len, binary) ? nullptr + : "Printing filename failed"; +} + void FileNameSaver::Finish() { for (const auto& file_name : file_names_) { // Just print the file names to standard output. diff --git a/src/idl_gen_grpc.cpp b/src/idl_gen_grpc.cpp index 476fa5ffe02..959e0729836 100644 --- a/src/idl_gen_grpc.cpp +++ b/src/idl_gen_grpc.cpp @@ -440,25 +440,26 @@ bool GenerateJavaGRPC(const Parser& parser, const std::string& path, return JavaGRPCGenerator(parser, path, file_name).generate(); } -bool GeneratePythonGRPC(const Parser& parser, const std::string& path, - const std::string& /*file_name*/) { +const char* GeneratePythonGRPC(const Parser& parser, const std::string& path, + const std::string& /*file_name*/) { int nservices = 0; for (auto it = parser.services_.vec.begin(); it != parser.services_.vec.end(); ++it) { if (!(*it)->generated) nservices++; } - if (!nservices) return true; + if (!nservices) return nullptr; flatbuffers::python::Version version{parser.opts.python_version}; - if (!version.IsValid()) return false; + if (!version.IsValid()) return "The provided Python version is not valid"; + + auto error = flatbuffers::python::grpc::Generate(parser, path, version); + if (error) return error; - if (!flatbuffers::python::grpc::Generate(parser, path, version)) { - return false; - } if (parser.opts.python_typing) { - return flatbuffers::python::grpc::GenerateStub(parser, path, version); + auto error = flatbuffers::python::grpc::GenerateStub(parser, path, version); + if (error) return error; } - return true; + return nullptr; } class SwiftGRPCGenerator : public flatbuffers::BaseGenerator { diff --git a/src/idl_gen_python.cpp b/src/idl_gen_python.cpp index 314c3c2d855..5be928e869a 100644 --- a/src/idl_gen_python.cpp +++ b/src/idl_gen_python.cpp @@ -57,7 +57,7 @@ class PythonStubGenerator { Keywords(version)}, version_(version) {} - bool Generate() { + const char* Generate() { if (parser_.opts.one_file) { Imports imports; std::stringstream stub; @@ -91,7 +91,8 @@ class PythonStubGenerator { std::string filename = namer_.Directories(*def->defined_namespace) + namer_.File(*def, SkipFile::Suffix); - if (!SaveFile(filename, imports, stub)) return false; + auto error = SaveFile(filename, imports, stub); + if (error) return error; } for (const StructDef* def : parser_.structs_.vec) { @@ -105,22 +106,24 @@ class PythonStubGenerator { std::string filename = namer_.Directories(*def->defined_namespace) + namer_.File(*def, SkipFile::Suffix); - if (!SaveFile(filename, imports, stub)) return false; + auto error = SaveFile(filename, imports, stub); + if (error) return error; } - return true; + return nullptr; } private: - bool SaveFile(const std::string& filename, const Imports& imports, - const std::stringstream& content) { + const char* SaveFile(const std::string& filename, const Imports& imports, + const std::stringstream& content) { std::stringstream ss; GenerateImports(ss, imports); ss << '\n'; ss << content.str() << '\n'; EnsureDirExists(StripFileName(filename)); - return parser_.opts.file_saver->SaveFile(filename.c_str(), ss.str(), false); + return parser_.opts.file_saver->AttemptSave(filename.c_str(), ss.str(), + false); } static void DeclareUOffset(std::stringstream& stub, Imports* imports) { @@ -2758,11 +2761,15 @@ class PythonGenerator : public BaseGenerator { EndBuilderBody(code_ptr); } - bool generate() { + bool generate() { return false; } + + const char* Generate() { std::string one_file_code; ImportMap one_file_imports; - if (!generateEnums(&one_file_code)) return false; - if (!generateStructs(&one_file_code, one_file_imports)) return false; + auto error = generateEnums(&one_file_code); + if (error) return error; + error = generateStructs(&one_file_code, one_file_imports); + if (error) return error; if (parser_.opts.one_file) { const std::string mod = file_name_ + parser_.opts.filename_suffix; @@ -2772,11 +2779,11 @@ class PythonGenerator : public BaseGenerator { one_file_imports, mod, true); } - return true; + return nullptr; } private: - bool generateEnums(std::string* one_file_code) const { + const char* generateEnums(std::string* one_file_code) const { for (auto it = parser_.enums_.vec.begin(); it != parser_.enums_.vec.end(); ++it) { auto& enum_def = **it; @@ -2793,17 +2800,18 @@ class PythonGenerator : public BaseGenerator { const std::string mod = namer_.File(enum_def, SkipFile::SuffixAndExtension); - if (!SaveType(namer_.File(enum_def, SkipFile::Suffix), - *enum_def.defined_namespace, enumcode, imports, mod, - false)) - return false; + auto error = SaveType(namer_.File(enum_def, SkipFile::Suffix), + *enum_def.defined_namespace, enumcode, imports, + mod, false); + printf("generateEnums: %s", error); + if (error) return error; } } - return true; + return nullptr; } - bool generateStructs(std::string* one_file_code, - ImportMap& one_file_imports) const { + const char* generateStructs(std::string* one_file_code, + ImportMap& one_file_imports) const { for (auto it = parser_.structs_.vec.begin(); it != parser_.structs_.vec.end(); ++it) { auto& struct_def = **it; @@ -2825,13 +2833,13 @@ class PythonGenerator : public BaseGenerator { } else { const std::string mod = namer_.File(struct_def, SkipFile::SuffixAndExtension); - if (!SaveType(namer_.File(struct_def, SkipFile::Suffix), - *struct_def.defined_namespace, declcode, imports, mod, - true)) - return false; + auto error = SaveType(namer_.File(struct_def, SkipFile::Suffix), + *struct_def.defined_namespace, declcode, imports, + mod, true); + if (error) return error; } } - return true; + return nullptr; } // Begin by declaring namespace and imports. @@ -2871,10 +2879,10 @@ class PythonGenerator : public BaseGenerator { } // Save out the generated code for a Python Table type. - bool SaveType(const std::string& defname, const Namespace& ns, - const std::string& classcode, const ImportMap& imports, - const std::string& mod, bool needs_imports) const { - if (classcode.empty()) return true; + const char* SaveType(const std::string& defname, const Namespace& ns, + const std::string& classcode, const ImportMap& imports, + const std::string& mod, bool needs_imports) const { + if (classcode.empty()) return nullptr; std::string code = ""; BeginFile(LastNamespacePart(ns), needs_imports, &code, mod, imports); @@ -2885,15 +2893,15 @@ class PythonGenerator : public BaseGenerator { EnsureDirExists(directories); for (size_t i = directories.find(kPathSeparator, path_.size()); - i != std::string::npos; - i = directories.find(kPathSeparator, i + 1)) { + i != std::string::npos; i = directories.find(kPathSeparator, i + 1)) { const std::string init_py = directories.substr(0, i) + kPathSeparator + "__init__.py"; - parser_.opts.file_saver->SaveFile(init_py.c_str(), "", false); + parser_.opts.file_saver->AttemptSave(init_py.c_str(), "", + false); // todo return on error } const std::string filename = directories + defname; - return parser_.opts.file_saver->SaveFile(filename.c_str(), code, false); + return parser_.opts.file_saver->AttemptSave(filename.c_str(), code, false); } private: @@ -2904,16 +2912,18 @@ class PythonGenerator : public BaseGenerator { } // namespace python static const char* GeneratePython(const Parser& parser, const std::string& path, - const std::string& file_name) { + const std::string& file_name) { python::Version version{parser.opts.python_version}; if (!version.IsValid()) return "The provided Python version is not valid"; python::PythonGenerator generator(parser, path, file_name, version); - if (!generator.generate()) return "could not generate Python code"; + auto error = generator.Generate(); + if (error) return error; if (parser.opts.python_typing) { python::PythonStubGenerator stub_generator(parser, path, version); - if (!stub_generator.Generate()) return "could not generate Python type stubs"; + auto error = stub_generator.Generate(); + if (error) return error; } return nullptr; } @@ -2948,7 +2958,9 @@ class PythonCodeGenerator : public CodeGenerator { Status GenerateGrpcCode(const Parser& parser, const std::string& path, const std::string& filename) override { - if (!GeneratePythonGRPC(parser, path, filename)) { // TODO add status GeneratePythonGRPC + auto err = GeneratePythonGRPC(parser, path, filename); + if (err) { + status_detail = " " + std::string(err); return Status::ERROR; } return Status::OK;