From 37cde3ab1cab49e4ff2bee65b582d9a30e23a581 Mon Sep 17 00:00:00 2001 From: Henner Zeller Date: Wed, 15 May 2024 15:40:24 -0700 Subject: [PATCH] Get ready for latest protobuf version. The protobuffer error collector interfaces changed to accept string_views instead. Since these are virtual methods, we need to override both in the transition period (but not add any 'override' to it as this depends on the available version we override). Once we are on latest protobuf (i.e. once we can update grpc), this will automatically compile with the new version. After that, we can remove the old AddError()/AddWarning() methods. Issues: https://github.com/google/xls/issues/1408 PiperOrigin-RevId: 634099367 --- dependency_support/load_external.bzl | 1 + xls/common/file/filesystem.cc | 20 ++++++++- xls/tools/proto_to_dslx.cc | 61 ++++++++++++++++++++++++++-- 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/dependency_support/load_external.bzl b/dependency_support/load_external.bzl index d1d3b49802..3aea6a82ce 100644 --- a/dependency_support/load_external.bzl +++ b/dependency_support/load_external.bzl @@ -270,6 +270,7 @@ def load_external_repositories(): # https://github.com/grpc/grpc/releases/tag/v1.60.0 # Can be updated to latest once https://github.com/grpc/grpc/issues/36132 # is fixed and made it into a grpc release. + # TODO(google/xls#1408) Once updated, fix related TODOs. http_archive( name = "com_github_grpc_grpc", urls = ["https://github.com/grpc/grpc/archive/v1.60.0.tar.gz"], diff --git a/xls/common/file/filesystem.cc b/xls/common/file/filesystem.cc index ba2f93ff77..750123d54e 100644 --- a/xls/common/file/filesystem.cc +++ b/xls/common/file/filesystem.cc @@ -62,7 +62,15 @@ class ParseTextProtoFileErrorCollector : public google::protobuf::io::ErrorColle const std::filesystem::path& file_name, const google::protobuf::Message& proto) : file_name_(file_name), proto_(proto) {} - void AddError(int line, int column, const std::string& message) override { + // The Interface changed up-stream so this is an attempt to be + // compatible with older and newer versions. We can't add 'override' on any + // of the next two functions as it depends on the proto version we're using. + + // New interface. + // TODO(google/xls#1408) Once grpc/protobuf is updated, add 'override'. + // NOLINTNEXTLINE(modernize-use-override) + void RecordError(int line, google::protobuf::io::ColumnNumber column, + std::string_view message) /* override */ { status_.Update(absl::Status( absl::StatusCode::kFailedPrecondition, absl::StrCat("Failed to parse ", proto_.GetDescriptor()->name(), @@ -71,7 +79,15 @@ class ParseTextProtoFileErrorCollector : public google::protobuf::io::ErrorColle "'. Proto parser error:\n", message))); } - absl::Status status() { return status_; } + // Deprecated interface, compatible with older proto. + // TODO(google/xls#1408) Once grpc/protobuf is updated, remove this method. + // NOLINTNEXTLINE(modernize-use-override) + void AddError(int line, int column, + const std::string& message) /* override */ { + RecordError(line, column, message); + } + + absl::Status status() const { return status_; } private: absl::Status status_; diff --git a/xls/tools/proto_to_dslx.cc b/xls/tools/proto_to_dslx.cc index 393a6ad67e..f2fc904d7d 100644 --- a/xls/tools/proto_to_dslx.cc +++ b/xls/tools/proto_to_dslx.cc @@ -240,12 +240,39 @@ std::string GetParentPrefixedName(const std::string& top_package, // SourceTreeDescriptorDatabase. class DbErrorCollector : public google::protobuf::compiler::MultiFileErrorCollector { public: + // Compatiblity: new interface is Record*(), old interface is Add*(). + // For a transition period, be compatible with both, but note, we can't + // put 'override' on any of these depending on what version we're on. + + // New interface. + // TODO(google/xls#1408) Once grpc/protobuf is updated, add 'override'. + // NOLINTNEXTLINE(modernize-use-override) + void RecordError(std::string_view filename, int line, int column, + std::string_view message) /* override */ { + LOG(ERROR) << message; + } + + // TODO(google/xls#1408) Once grpc/protobuf is updated, add 'override'. + // NOLINTNEXTLINE(modernize-use-override) + void RecordWarning(std::string_view filename, int line, int column, + std::string_view message) /* override */ { + LOG(WARNING) << message; + } + + // Deprecated interface, compatible with older proto. + // Remove, once we're solidly on latest protobuf. + + // TODO(google/xls#1408) Once grpc/protobuf is updated, remove this method. + // NOLINTNEXTLINE(modernize-use-override) void AddError(const std::string& filename, int line, int column, - const std::string& message) override { + const std::string& message) /* override */ { LOG(ERROR) << message; } + + // TODO(google/xls#1408) Once grpc/protobuf is updated, remove this method. + // NOLINTNEXTLINE(modernize-use-override) void AddWarning(const std::string& filename, int line, int column, - const std::string& message) override { + const std::string& message) /* override */ { LOG(WARNING) << message; } }; @@ -253,15 +280,41 @@ class DbErrorCollector : public google::protobuf::compiler::MultiFileErrorCollec // Simple output logger for any errors coming from a DescriptorPool. class PoolErrorCollector : public DescriptorPool::ErrorCollector { public: + // Compatiblity: new interface is Record*(), old interface is Add*(). + // For a transition period, be compatible with both, but note, we can't + // put 'override' on any of these depending on what version we're on. + + // New interface. + // TODO(google/xls#1408) Once grpc/protobuf is updated, add 'override'. + // NOLINTNEXTLINE(modernize-use-override) + void RecordError(std::string_view filename, std::string_view element_name, + const Message* descriptor, ErrorLocation location, + std::string_view message) /* override */ { + LOG(ERROR) << message; + } + + // TODO(google/xls#1408) Once grpc/protobuf is updated, add 'override'. + // NOLINTNEXTLINE(modernize-use-override) + void RecordWarning(std::string_view filename, std::string_view element_name, + const Message* descriptor, ErrorLocation location, + std::string_view message) /* override */ { + LOG(WARNING) << message; + } + + // Deprecated interface, compatible with older proto. + // TODO(google/xls#1408) Once grpc/protobuf is updated, remove this method. + // NOLINTNEXTLINE(modernize-use-override) void AddError(const std::string& filename, const std::string& element_name, const Message* descriptor, ErrorLocation location, - const std::string& message) override { + const std::string& message) /* override */ { LOG(ERROR) << message; } + // TODO(google/xls#1408) Once grpc/protobuf is updated, remove this method. + // NOLINTNEXTLINE(modernize-use-override) void AddWarning(const std::string& filename, const std::string& element_name, const Message* descriptor, ErrorLocation location, - const std::string& message) override { + const std::string& message) /* override */ { LOG(WARNING) << message; } };