Skip to content

Commit

Permalink
Breaking change: Remove deprecated std::string error collector overrides
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 590740727
  • Loading branch information
@mkruskal-google
mkruskal-google authored and Copybara-Service committed Dec 14, 2023
1 parent 4b2a30c commit 543fbcd
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 72 deletions.
22 changes: 2 additions & 20 deletions src/google/protobuf/compiler/importer.h
View file
Expand Up @@ -181,30 +181,12 @@ class PROTOBUF_EXPORT MultiFileErrorCollector {
// Line and column numbers are zero-based. A line number of -1 indicates
// an error with the entire file (e.g. "not found").
virtual void RecordError(absl::string_view filename, int line, int column,
absl::string_view message) {
PROTOBUF_IGNORE_DEPRECATION_START
AddError(std::string(filename), line, column, std::string(message));
PROTOBUF_IGNORE_DEPRECATION_STOP
}
absl::string_view message)
= 0;
virtual void RecordWarning(absl::string_view filename, int line, int column,
absl::string_view message) {
PROTOBUF_IGNORE_DEPRECATION_START
AddWarning(std::string(filename), line, column, std::string(message));
PROTOBUF_IGNORE_DEPRECATION_STOP
}

private:
// These should never be called directly, but if a legacy class overrides
// them they'll get routed to by the Record* methods.
ABSL_DEPRECATED("Use RecordError")
virtual void AddError(const std::string& filename, int line, int column,
const std::string& message) {
ABSL_LOG(FATAL) << "AddError or RecordError must be implemented.";
}

ABSL_DEPRECATED("Use RecordWarning")
virtual void AddWarning(const std::string& filename, int line, int column,
const std::string& message) {}
};

// Abstract interface which represents a directory tree containing proto files.
Expand Down
27 changes: 2 additions & 25 deletions src/google/protobuf/descriptor.h
View file
Expand Up @@ -2156,12 +2156,8 @@ class PROTOBUF_EXPORT DescriptorPool {
virtual void RecordError(absl::string_view filename,
absl::string_view element_name,
const Message* descriptor, ErrorLocation location,
absl::string_view message) {
PROTOBUF_IGNORE_DEPRECATION_START
AddError(std::string(filename), std::string(element_name), descriptor,
location, std::string(message));
PROTOBUF_IGNORE_DEPRECATION_STOP
}
absl::string_view message)
= 0;

// Reports a warning in the FileDescriptorProto. Use this function if the
// problem occurred should NOT interrupt building the FileDescriptorProto.
Expand All @@ -2176,27 +2172,8 @@ class PROTOBUF_EXPORT DescriptorPool {
const Message* descriptor,
ErrorLocation location,
absl::string_view message) {
PROTOBUF_IGNORE_DEPRECATION_START
AddWarning(std::string(filename), std::string(element_name), descriptor,
location, std::string(message));
PROTOBUF_IGNORE_DEPRECATION_STOP
}

private:
// These should never be called directly, but if a legacy class overrides
// them they'll get routed to by the Record* methods.
ABSL_DEPRECATED("Use RecordError")
virtual void AddError(const std::string& filename,
const std::string& element_name,
const Message* descriptor, ErrorLocation location,
const std::string& message) {
ABSL_LOG(FATAL) << "AddError or RecordError must be implemented.";
}
ABSL_DEPRECATED("Use RecordWarning")
virtual void AddWarning(const std::string& filename,
const std::string& element_name,
const Message* descriptor, ErrorLocation location,
const std::string& message) {}
};

// Convert the FileDescriptorProto to real descriptors and place them in
Expand Down
21 changes: 2 additions & 19 deletions src/google/protobuf/io/tokenizer.h
View file
Expand Up @@ -55,33 +55,16 @@ class PROTOBUF_EXPORT ErrorCollector {
// column numbers. The numbers are zero-based, so you may want to add
// 1 to each before printing them.
virtual void RecordError(int line, ColumnNumber column,
absl::string_view message) {
PROTOBUF_IGNORE_DEPRECATION_START
AddError(line, column, std::string(message));
PROTOBUF_IGNORE_DEPRECATION_STOP
}
absl::string_view message)
= 0;

// Indicates that there was a warning in the input at the given line and
// column numbers. The numbers are zero-based, so you may want to add
// 1 to each before printing them.
virtual void RecordWarning(int line, ColumnNumber column,
absl::string_view message) {
PROTOBUF_IGNORE_DEPRECATION_START
AddWarning(line, column, std::string(message));
PROTOBUF_IGNORE_DEPRECATION_STOP
}

private:
// These should never be called directly, but if a legacy class overrides
// them they'll get routed to by the Record* methods.
ABSL_DEPRECATED("Use RecordError")
virtual void AddError(int line, ColumnNumber column,
const std::string& message) {
ABSL_LOG(FATAL) << "AddError or RecordError must be implemented.";
}
ABSL_DEPRECATED("Use RecordWarning")
virtual void AddWarning(int line, ColumnNumber column,
const std::string& message) {}
};

// This class converts a stream of raw text into a stream of tokens for
Expand Down
17 changes: 12 additions & 5 deletions src/google/protobuf/retention_test.cc
View file
Expand Up @@ -13,6 +13,7 @@

#include "google/protobuf/descriptor.pb.h"
#include <gtest/gtest.h>
#include "absl/strings/string_view.h"
#include "absl/strings/substitute.h"
#include "google/protobuf/compiler/parser.h"
#include "google/protobuf/dynamic_message.h"
Expand Down Expand Up @@ -160,6 +161,12 @@ TEST(RetentionTest, Method) {
.GetExtension(protobuf_unittest::method_option));
}

class SimpleErrorCollector : public io::ErrorCollector {
public:
SimpleErrorCollector() = default;
void RecordError(int line, io::ColumnNumber column,
absl::string_view message) override{};
};

TEST(RetentionTest, StripSourceRetentionOptionsWithSourceCodeInfo) {
// The tests above make assertions against the generated code, but this test
Expand Down Expand Up @@ -202,7 +209,7 @@ TEST(RetentionTest, StripSourceRetentionOptionsWithSourceCodeInfo) {
FileDescriptorSet::descriptor()->file()->name());
io::ArrayInputStream input_stream(proto_file.data(),
static_cast<int>(proto_file.size()));
io::ErrorCollector error_collector;
SimpleErrorCollector error_collector;
io::Tokenizer tokenizer(&input_stream, &error_collector);
compiler::Parser parser;
FileDescriptorProto file_descriptor;
Expand Down Expand Up @@ -242,7 +249,7 @@ TEST(RetentionTest, RemoveEmptyOptions) {
FileDescriptorSet::descriptor()->file()->name());
io::ArrayInputStream input_stream(proto_file.data(),
static_cast<int>(proto_file.size()));
io::ErrorCollector error_collector;
SimpleErrorCollector error_collector;
io::Tokenizer tokenizer(&input_stream, &error_collector);
compiler::Parser parser;
FileDescriptorProto file_descriptor;
Expand Down Expand Up @@ -282,7 +289,7 @@ TEST(RetentionTest, InvalidDescriptor) {
FileDescriptorSet::descriptor()->file()->name());
io::ArrayInputStream input_stream(proto_file.data(),
static_cast<int>(proto_file.size()));
io::ErrorCollector error_collector;
SimpleErrorCollector error_collector;
io::Tokenizer tokenizer(&input_stream, &error_collector);
compiler::Parser parser;
FileDescriptorProto file_descriptor_proto;
Expand Down Expand Up @@ -331,7 +338,7 @@ TEST(RetentionTest, MissingRequiredField) {
FileDescriptorSet::descriptor()->file()->name());
io::ArrayInputStream input_stream(proto_file.data(),
static_cast<int>(proto_file.size()));
io::ErrorCollector error_collector;
SimpleErrorCollector error_collector;
io::Tokenizer tokenizer(&input_stream, &error_collector);
compiler::Parser parser;
FileDescriptorProto file_descriptor_proto;
Expand Down Expand Up @@ -387,7 +394,7 @@ TEST(RetentionTest, InvalidRecursionDepth) {
FileDescriptorSet::descriptor()->file()->name());
io::ArrayInputStream input_stream(proto_file.data(),
static_cast<int>(proto_file.size()));
io::ErrorCollector error_collector;
SimpleErrorCollector error_collector;
io::Tokenizer tokenizer(&input_stream, &error_collector);
compiler::Parser parser;
FileDescriptorProto file_descriptor_proto;
Expand Down
6 changes: 3 additions & 3 deletions upb/util/def_to_proto_test.h
View file
Expand Up @@ -46,9 +46,9 @@ MATCHER_P(EqualsProtoTreatNansAsEqual, proto,
}

class NullErrorCollector : public google::protobuf::DescriptorPool::ErrorCollector {
void AddError(const std::string& filename, const std::string& element_name,
const google::protobuf::Message* descriptor, ErrorLocation location,
const std::string& message) override {}
void RecordError(absl::string_view filename, absl::string_view element_name,
const google::protobuf::Message* descriptor, ErrorLocation location,
absl::string_view message) override {}
void RecordWarning(absl::string_view filename, absl::string_view element_name,
const google::protobuf::Message* descriptor, ErrorLocation location,
absl::string_view message) override {}
Expand Down

0 comments on commit 543fbcd

Please sign in to comment.