Skip to content

Commit

Permalink
Remove protobuf logging compatibility shim.
Browse files Browse the repository at this point in the history
After upgrade to latest grpc, we are solidly using the 'new'
protobuf api for error and warning collection.

The compatibility changes introduced in 37cde3a
are not needed anymore.

Fixes: #1408
PiperOrigin-RevId: 647765652
  • Loading branch information
hzeller authored and copybara-github committed Jun 28, 2024
1 parent 90f257d commit 88c10c1
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 73 deletions.
1 change: 0 additions & 1 deletion dependency_support/load_external.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ def load_external_repositories():
)

# Released 2024-06-07, current as of 2024-06-26.
# TODO(google/xls#1408) Once updated, fix related TODOs.
http_archive(
name = "com_github_grpc_grpc",
urls = ["https://github.com/grpc/grpc/archive/v1.64.2.tar.gz"],
Expand Down
17 changes: 1 addition & 16 deletions xls/common/file/filesystem.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,8 @@ class ParseTextProtoFileErrorCollector : public google::protobuf::io::ErrorColle
const std::filesystem::path& file_name, const google::protobuf::Message& proto)
: file_name_(file_name), proto_(proto) {}

// 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 */ {
std::string_view message) final {
status_.Update(absl::Status(
absl::StatusCode::kFailedPrecondition,
absl::StrCat("Failed to parse ", proto_.GetDescriptor()->name(),
Expand All @@ -79,14 +72,6 @@ class ParseTextProtoFileErrorCollector : public google::protobuf::io::ErrorColle
"'. Proto parser error:\n", 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(int line, int column,
const std::string& message) /* override */ {
RecordError(line, column, message);
}

absl::Status status() const { return status_; }

private:
Expand Down
60 changes: 4 additions & 56 deletions xls/tools/proto_to_dslx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -241,81 +241,29 @@ 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 */ {
std::string_view message) final {
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 */ {
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 */ {
std::string_view message) final {
LOG(WARNING) << message;
}
};

// 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 */ {
std::string_view message) final {
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 */ {
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 */ {
std::string_view message) final {
LOG(WARNING) << message;
}
};
Expand Down

0 comments on commit 88c10c1

Please sign in to comment.