Skip to content

Commit

Permalink
Get ready for latest protobuf version.
Browse files Browse the repository at this point in the history
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: #1408
PiperOrigin-RevId: 634099367
  • Loading branch information
hzeller authored and copybara-github committed May 15, 2024
1 parent 5cf4feb commit 37cde3a
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 6 deletions.
1 change: 1 addition & 0 deletions dependency_support/load_external.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
20 changes: 18 additions & 2 deletions xls/common/file/filesystem.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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_;
Expand Down
61 changes: 57 additions & 4 deletions xls/tools/proto_to_dslx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,28 +240,81 @@ 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;
}
};

// 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;
}
};
Expand Down

0 comments on commit 37cde3a

Please sign in to comment.