From d538808032d66366364e410200246c236e275e0f Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Thu, 19 Jan 2023 12:20:16 -0800 Subject: [PATCH] Set up string_view migration for error collector classes. We will continue to support the deprecated versions for the time being, but users should migrate to the string_view alternatives. PiperOrigin-RevId: 503233401 --- src/google/protobuf/compiler/importer.cc | 11 ++-- src/google/protobuf/compiler/importer.h | 27 +++++++- src/google/protobuf/compiler/parser.cc | 4 +- .../protobuf/compiler/parser_unittest.cc | 2 +- src/google/protobuf/descriptor.cc | 8 +-- src/google/protobuf/descriptor.h | 63 ++++++++++++++----- src/google/protobuf/io/tokenizer.cc | 8 +-- src/google/protobuf/io/tokenizer.h | 32 ++++++++-- src/google/protobuf/port_def.inc | 17 +++++ src/google/protobuf/port_undef.inc | 2 + src/google/protobuf/text_format.cc | 6 +- 11 files changed, 137 insertions(+), 43 deletions(-) diff --git a/src/google/protobuf/compiler/importer.cc b/src/google/protobuf/compiler/importer.cc index f56892ce07..465945daf0 100644 --- a/src/google/protobuf/compiler/importer.cc +++ b/src/google/protobuf/compiler/importer.cc @@ -107,7 +107,8 @@ class SourceTreeDescriptorDatabase::SingleFileErrorCollector // implements ErrorCollector --------------------------------------- void AddError(int line, int column, const std::string& message) override { if (multi_file_error_collector_ != nullptr) { - multi_file_error_collector_->AddError(filename_, line, column, message); + multi_file_error_collector_->RecordError(filename_, line, column, + message); } had_errors_ = true; } @@ -147,8 +148,8 @@ bool SourceTreeDescriptorDatabase::FindFileByName(const std::string& filename, return true; } if (error_collector_ != nullptr) { - error_collector_->AddError(filename, -1, 0, - source_tree_->GetLastErrorMessage()); + error_collector_->RecordError(filename, -1, 0, + source_tree_->GetLastErrorMessage()); } return false; } @@ -203,7 +204,7 @@ void SourceTreeDescriptorDatabase::ValidationErrorCollector::AddError( } else { owner_->source_locations_.Find(descriptor, location, &line, &column); } - owner_->error_collector_->AddError(filename, line, column, message); + owner_->error_collector_->RecordError(filename, line, column, message); } void SourceTreeDescriptorDatabase::ValidationErrorCollector::AddWarning( @@ -219,7 +220,7 @@ void SourceTreeDescriptorDatabase::ValidationErrorCollector::AddWarning( } else { owner_->source_locations_.Find(descriptor, location, &line, &column); } - owner_->error_collector_->AddWarning(filename, line, column, message); + owner_->error_collector_->RecordWarning(filename, line, column, message); } // =================================================================== diff --git a/src/google/protobuf/compiler/importer.h b/src/google/protobuf/compiler/importer.h index 3912f2dcb2..83f05048e6 100644 --- a/src/google/protobuf/compiler/importer.h +++ b/src/google/protobuf/compiler/importer.h @@ -41,6 +41,7 @@ #include #include +#include "absl/strings/string_view.h" #include "google/protobuf/compiler/parser.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor_database.h" @@ -202,11 +203,31 @@ 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 + } + 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) = 0; + const std::string& message) { + GOOGLE_ABSL_LOG(FATAL) << "AddError or RecordError must be implemented."; + } - virtual void AddWarning(const std::string& /* filename */, int /* line */, - int /* column */, const std::string& /* message */) {} + 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. diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index 77f42a943a..14bdf3eb29 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -400,7 +400,7 @@ bool Parser::ConsumeEndOfDeclaration(absl::string_view text, void Parser::AddError(int line, int column, const std::string& error) { if (error_collector_ != nullptr) { - error_collector_->AddError(line, column, error); + error_collector_->RecordError(line, column, error); } had_errors_ = true; } @@ -411,7 +411,7 @@ void Parser::AddError(const std::string& error) { void Parser::AddWarning(int line, int column, const std::string& warning) { if (error_collector_ != nullptr) { - error_collector_->AddWarning(line, column, warning); + error_collector_->RecordWarning(line, column, warning); } } diff --git a/src/google/protobuf/compiler/parser_unittest.cc b/src/google/protobuf/compiler/parser_unittest.cc index 44889f99d0..b7b710300e 100644 --- a/src/google/protobuf/compiler/parser_unittest.cc +++ b/src/google/protobuf/compiler/parser_unittest.cc @@ -99,7 +99,7 @@ class MockValidationErrorCollector : public DescriptorPool::ErrorCollector { } else { source_locations_.Find(descriptor, location, &line, &column); } - wrapped_collector_->AddError(line, column, message); + wrapped_collector_->RecordError(line, column, message); } private: diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index fd49431159..f4fa637474 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -4200,8 +4200,8 @@ PROTOBUF_NOINLINE void DescriptorBuilder::AddError( } GOOGLE_ABSL_LOG(ERROR) << " " << element_name << ": " << error; } else { - error_collector_->AddError(filename_, element_name, &descriptor, location, - error); + error_collector_->RecordError(filename_, element_name, &descriptor, + location, error); } had_errors_ = true; } @@ -4252,8 +4252,8 @@ PROTOBUF_NOINLINE void DescriptorBuilder::AddWarning( if (error_collector_ == nullptr) { GOOGLE_ABSL_LOG(WARNING) << filename_ << " " << element_name << ": " << error; } else { - error_collector_->AddWarning(filename_, element_name, &descriptor, location, - error); + error_collector_->RecordWarning(filename_, element_name, &descriptor, + location, error); } } diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 04727ffb15..de6d960a2a 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -1906,25 +1906,56 @@ class PROTOBUF_EXPORT DescriptorPool { // Reports an error in the FileDescriptorProto. Use this function if the // problem occurred should interrupt building the FileDescriptorProto. - virtual void AddError( - const std::string& filename, // File name in which the error occurred. - const std::string& element_name, // Full name of the erroneous element. - const Message* descriptor, // Descriptor of the erroneous element. - ErrorLocation location, // One of the location constants, above. - const std::string& message // Human-readable error message. - ) = 0; + // Provided the following arguments: + // filename - File name in which the error occurred. + // element_name - Full name of the erroneous element. + // descriptor - Descriptor of the erroneous element. + // location - One of the location constants, above. + // message - Human-readable error message. + 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 + } // Reports a warning in the FileDescriptorProto. Use this function if the // problem occurred should NOT interrupt building the FileDescriptorProto. - virtual void AddWarning( - const std::string& /*filename*/, // File name in which the error - // occurred. - const std::string& /*element_name*/, // Full name of the erroneous - // element. - const Message* /*descriptor*/, // Descriptor of the erroneous element. - ErrorLocation /*location*/, // One of the location constants, above. - const std::string& /*message*/ // Human-readable error message. - ) {} + // Provided the following arguments: + // filename - File name in which the error occurred. + // element_name - Full name of the erroneous element. + // descriptor - Descriptor of the erroneous element. + // location - One of the location constants, above. + // message - Human-readable error message. + virtual void RecordWarning(absl::string_view filename, + absl::string_view element_name, + 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) { + GOOGLE_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 diff --git a/src/google/protobuf/io/tokenizer.cc b/src/google/protobuf/io/tokenizer.cc index dd124af061..dd6f51de1d 100644 --- a/src/google/protobuf/io/tokenizer.cc +++ b/src/google/protobuf/io/tokenizer.cc @@ -568,8 +568,8 @@ void Tokenizer::ConsumeBlockComment(std::string* content) { "\"/*\" inside block comment. Block comments cannot be nested."); } else if (current_char_ == '\0') { AddError("End-of-file inside block comment."); - error_collector_->AddError(start_line, start_column, - " Comment started here."); + error_collector_->RecordError(start_line, start_column, + " Comment started here."); if (content != NULL) StopRecording(); break; } @@ -687,7 +687,7 @@ bool Tokenizer::Next() { current_.line == previous_.line && current_.column == previous_.end_column) { // We don't accept syntax like "blah.123". - error_collector_->AddError( + error_collector_->RecordError( line_, column_ - 2, "Need space between identifier and decimal point."); } @@ -706,7 +706,7 @@ bool Tokenizer::Next() { } else { // Check if the high order bit is set. if (current_char_ & 0x80) { - error_collector_->AddError( + error_collector_->RecordError( line_, column_, absl::StrFormat("Interpreting non ascii codepoint %d.", static_cast(current_char_))); diff --git a/src/google/protobuf/io/tokenizer.h b/src/google/protobuf/io/tokenizer.h index 3df6854d6a..cc60e38393 100644 --- a/src/google/protobuf/io/tokenizer.h +++ b/src/google/protobuf/io/tokenizer.h @@ -41,6 +41,8 @@ #include #include "google/protobuf/stubs/common.h" +#include "google/protobuf/stubs/logging.h" +#include "absl/strings/string_view.h" #include "google/protobuf/port.h" // Must be included last. @@ -75,14 +77,34 @@ class PROTOBUF_EXPORT ErrorCollector { // Indicates that there was an error 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 AddError(int line, ColumnNumber column, - const std::string& message) = 0; + 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 + } // 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 AddWarning(int /* line */, ColumnNumber /* column */, - const std::string& /* message */) {} + 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) { + GOOGLE_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 @@ -338,7 +360,7 @@ class PROTOBUF_EXPORT Tokenizer { // Convenience method to add an error at the current line and column. void AddError(const std::string& message) { - error_collector_->AddError(line_, column_, message); + error_collector_->RecordError(line_, column_, message); } // ----------------------------------------------------------------- diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 7bdd72bde6..57efc9147f 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -410,6 +410,23 @@ static_assert(PROTOBUF_CPLUSPLUS_MIN(201402L), "Protobuf only supports C++14 and # define PROTOBUF_DEPRECATED_ENUM #endif +#if defined(__clang__) +#define PROTOBUF_IGNORE_DEPRECATION_START \ + _Pragma("clang diagnostic push") \ + _Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"") +#define PROTOBUF_IGNORE_DEPRECATION_STOP \ + _Pragma("clang diagnostic pop") +#elif defined(__GNUC__) +#define PROTOBUF_IGNORE_DEPRECATION_START \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"") +#define PROTOBUF_IGNORE_DEPRECATION_STOP \ + _Pragma("GCC diagnostic pop") +#else +#define PROTOBUF_IGNORE_DEPRECATION_START +#define PROTOBUF_IGNORE_DEPRECATION_STOP +#endif + #ifdef PROTOBUF_RETURNS_NONNULL #error PROTOBUF_RETURNS_NONNULL was previously defined #endif diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc index 32b499a9aa..6e93c8f01b 100644 --- a/src/google/protobuf/port_undef.inc +++ b/src/google/protobuf/port_undef.inc @@ -57,6 +57,8 @@ #undef PROTOBUF_DEPRECATED #undef PROTOBUF_DEPRECATED_ENUM #undef PROTOBUF_DEPRECATED_MSG +#undef PROTOBUF_IGNORE_DEPRECATION_START +#undef PROTOBUF_IGNORE_DEPRECATION_STOP #undef PROTOBUF_RETURNS_NONNULL #undef PROTOBUF_ATTRIBUTE_REINITIALIZES #undef PROTOBUF_RTTI diff --git a/src/google/protobuf/text_format.cc b/src/google/protobuf/text_format.cc index e8c64e9d4a..3a42d11e1c 100644 --- a/src/google/protobuf/text_format.cc +++ b/src/google/protobuf/text_format.cc @@ -388,7 +388,7 @@ class TextFormat::Parser::ParserImpl { << root_message_type_->full_name() << ": " << message; } } else { - error_collector_->AddError(line, col, message); + error_collector_->RecordError(line, col, message); } } @@ -403,7 +403,7 @@ class TextFormat::Parser::ParserImpl { << root_message_type_->full_name() << ": " << message; } } else { - error_collector_->AddWarning(line, col, message); + error_collector_->RecordWarning(line, col, message); } } @@ -1662,7 +1662,7 @@ namespace { bool CheckParseInputSize(absl::string_view input, io::ErrorCollector* error_collector) { if (input.size() > INT_MAX) { - error_collector->AddError( + error_collector->RecordError( -1, 0, absl::StrCat( "Input size too large: ", static_cast(input.size()),