From f5a225597b04743164f87777110ec64f4f8bdaed Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 5 Feb 2024 16:44:49 -0800 Subject: [PATCH] Minor cleanup: IWYU, shoud_split() instead of ShouldSplit(...). PiperOrigin-RevId: 604471605 --- cmake/abseil-cpp.cmake | 13 +-- src/google/protobuf/compiler/BUILD.bazel | 4 + .../protobuf/compiler/code_generator.cc | 10 +- src/google/protobuf/compiler/code_generator.h | 4 +- .../compiler/command_line_interface.cc | 97 +++++++++++-------- .../command_line_interface_unittest.cc | 26 +++-- src/google/protobuf/compiler/cpp/BUILD.bazel | 1 + src/google/protobuf/compiler/cpp/field.cc | 14 +-- .../cpp/field_generators/generators.h | 2 + .../cpp/field_generators/string_field.cc | 12 +-- 10 files changed, 106 insertions(+), 77 deletions(-) diff --git a/cmake/abseil-cpp.cmake b/cmake/abseil-cpp.cmake index a4e9d2295e..061d2dbb5b 100644 --- a/cmake/abseil-cpp.cmake +++ b/cmake/abseil-cpp.cmake @@ -39,12 +39,12 @@ set(_protobuf_FIND_ABSL "if(NOT TARGET absl::strings)\n find_package(absl CONFI if (BUILD_SHARED_LIBS AND MSVC) # On MSVC Abseil is bundled into a single DLL. - # This condition is necessary as of abseil 20230125.3 when abseil is consumed via add_subdirectory, - # the abseil_dll target is named abseil_dll, while if abseil is consumed via find_package, the target - # is called absl::abseil_dll - # Once https://github.com/abseil/abseil-cpp/pull/1466 is merged and released in the minimum version of - # abseil required by protobuf, it is possible to always link absl::abseil_dll and absl::abseil_test_dll - # and remove the if + # This condition is necessary as of abseil 20230125.3 when abseil is consumed + # via add_subdirectory, the abseil_dll target is named abseil_dll, while if + # abseil is consumed via find_package, the target is called absl::abseil_dll + # Once https://github.com/abseil/abseil-cpp/pull/1466 is merged and released + # in the minimum version of abseil required by protobuf, it is possible to + # always link absl::abseil_dll and absl::abseil_test_dll and remove the if if(protobuf_ABSL_PROVIDER STREQUAL "package") set(protobuf_ABSL_USED_TARGETS absl::abseil_dll) set(protobuf_ABSL_USED_TEST_TARGETS absl::abseil_test_dll) @@ -75,6 +75,7 @@ else() absl::if_constexpr absl::layout absl::log_initialize + absl::log_globals absl::log_severity absl::memory absl::node_hash_map diff --git a/src/google/protobuf/compiler/BUILD.bazel b/src/google/protobuf/compiler/BUILD.bazel index a564703626..fb5822837d 100644 --- a/src/google/protobuf/compiler/BUILD.bazel +++ b/src/google/protobuf/compiler/BUILD.bazel @@ -159,11 +159,14 @@ cc_library( "//src/google/protobuf/stubs", "@com_google_absl//absl/algorithm", "@com_google_absl//absl/algorithm:container", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/base:log_severity", "@com_google_absl//absl/container:btree", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/log:absl_log", + "@com_google_absl//absl/log:globals", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", @@ -421,6 +424,7 @@ cc_test( "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", + "@com_google_absl//absl/types:span", "@com_google_googletest//:gtest", "@com_google_googletest//:gtest_main", ], diff --git a/src/google/protobuf/compiler/code_generator.cc b/src/google/protobuf/compiler/code_generator.cc index fd1f45f3e8..cf4c1e7c06 100644 --- a/src/google/protobuf/compiler/code_generator.cc +++ b/src/google/protobuf/compiler/code_generator.cc @@ -11,10 +11,14 @@ #include "google/protobuf/compiler/code_generator.h" +#include +#include #include +#include #include "absl/log/absl_log.h" #include "absl/status/statusor.h" +#include "absl/strings/match.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" #include "absl/strings/string_view.h" @@ -31,7 +35,7 @@ namespace google { namespace protobuf { namespace compiler { -CodeGenerator::~CodeGenerator() {} +CodeGenerator::~CodeGenerator() = default; bool CodeGenerator::GenerateAll(const std::vector& files, const std::string& parameter, @@ -40,7 +44,7 @@ bool CodeGenerator::GenerateAll(const std::vector& files, // Default implementation is just to call the per file method, and prefix any // error string with the file to provide context. bool succeeded = true; - for (int i = 0; i < files.size(); i++) { + for (size_t i = 0; i < files.size(); i++) { const FileDescriptor* file = files[i]; succeeded = Generate(file, parameter, generator_context, error); if (!succeeded && error && error->empty()) { @@ -74,7 +78,7 @@ absl::StatusOr CodeGenerator::BuildFeatureSetDefaults() GetMaximumEdition()); } -GeneratorContext::~GeneratorContext() {} +GeneratorContext::~GeneratorContext() = default; io::ZeroCopyOutputStream* GeneratorContext::OpenForAppend( const std::string& filename) { diff --git a/src/google/protobuf/compiler/code_generator.h b/src/google/protobuf/compiler/code_generator.h index 8111161c1b..cabad0c4d1 100644 --- a/src/google/protobuf/compiler/code_generator.h +++ b/src/google/protobuf/compiler/code_generator.h @@ -22,10 +22,8 @@ #include "absl/status/statusor.h" #include "absl/strings/string_view.h" -#include "google/protobuf/compiler/retention.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" -#include "google/protobuf/port.h" // Must be included last. #include "google/protobuf/port_def.inc" @@ -53,7 +51,7 @@ class GeneratorContext; // be registered with CommandLineInterface to support various languages. class PROTOC_EXPORT CodeGenerator { public: - CodeGenerator() {} + CodeGenerator() = default; CodeGenerator(const CodeGenerator&) = delete; CodeGenerator& operator=(const CodeGenerator&) = delete; virtual ~CodeGenerator(); diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 6b4bad04c5..60cc517a05 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -13,18 +13,25 @@ #include #include +#include #include "absl/algorithm/container.h" +#include "absl/base/attributes.h" +#include "absl/base/log_severity.h" +#include "absl/container/btree_map.h" #include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" +#include "absl/log/globals.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/types/span.h" #include "google/protobuf/compiler/allowlists/allowlists.h" #include "google/protobuf/compiler/versions.h" +#include "google/protobuf/descriptor_database.h" #include "google/protobuf/descriptor_visitor.h" #include "google/protobuf/feature_resolver.h" +#include "google/protobuf/io/zero_copy_stream_impl_lite.h" #include "google/protobuf/stubs/platform_macros.h" @@ -42,7 +49,6 @@ #ifndef _MSC_VER #include #endif -#include #include #include @@ -61,7 +67,6 @@ #include #endif -#include "google/protobuf/stubs/common.h" #include "absl/log/absl_check.h" #include "absl/log/absl_log.h" #include "absl/container/flat_hash_set.h" @@ -81,12 +86,15 @@ #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/dynamic_message.h" #include "google/protobuf/io/coded_stream.h" -#include "google/protobuf/io/io_win32.h" #include "google/protobuf/io/printer.h" #include "google/protobuf/io/zero_copy_stream_impl.h" #include "google/protobuf/text_format.h" +#ifdef _WIN32 +#include "google/protobuf/io/io_win32.h" +#endif + // Must be included last. #include "google/protobuf/port_def.inc" @@ -178,7 +186,7 @@ bool TryCreateParentDirectory(const std::string& prefix, std::vector parts = absl::StrSplit(filename, absl::ByAnyChar("/\\"), absl::SkipEmpty()); std::string path_so_far = prefix; - for (int i = 0; i < parts.size() - 1; i++) { + for (size_t i = 0; i < parts.size() - 1; ++i) { path_so_far += parts[i]; if (mkdir(path_so_far.c_str(), 0777) != 0) { if (errno != EEXIST) { @@ -310,7 +318,7 @@ void CommandLineInterface::GetTransitiveDependencies( } // Add all dependencies. - for (int i = 0; i < file->dependency_count(); i++) { + for (int i = 0; i < file->dependency_count(); ++i) { GetTransitiveDependencies(file->dependency(i), already_seen, output, options); } @@ -335,12 +343,12 @@ class CommandLineInterface::ErrorPrinter public io::ErrorCollector, public DescriptorPool::ErrorCollector { public: - ErrorPrinter(ErrorFormat format, DiskSourceTree* tree = nullptr) + explicit ErrorPrinter(ErrorFormat format, DiskSourceTree* tree = nullptr) : format_(format), tree_(tree), found_errors_(false), found_warnings_(false) {} - ~ErrorPrinter() override {} + ~ErrorPrinter() override = default; // implements MultiFileErrorCollector ------------------------------ void RecordError(absl::string_view filename, int line, int column, @@ -431,7 +439,8 @@ class CommandLineInterface::ErrorPrinter // them all to disk on demand. class CommandLineInterface::GeneratorContextImpl : public GeneratorContext { public: - GeneratorContextImpl(const std::vector& parsed_files); + explicit GeneratorContextImpl( + const std::vector& parsed_files); // Write all files in the directory to disk at the given output location, // which must end in a '/'. @@ -734,10 +743,11 @@ void CommandLineInterface::MemoryOutputStream::InsertShiftedInfo( int inner_indent = 0; // insertion_content is guaranteed to end in an endline. This last endline // has no effect on indentation. - for (; pos < source_annotation.end() && pos < insertion_content.size() - 1; + for (; pos < static_cast(source_annotation.end()) && + pos < insertion_content.size() - 1; ++pos) { if (insertion_content[pos] == '\n') { - if (pos >= source_annotation.begin()) { + if (pos >= static_cast(source_annotation.begin())) { // The beginning of the annotation is at insertion_offset, but the end // can still move further in the target file. inner_indent += indent_length; @@ -797,7 +807,8 @@ void CommandLineInterface::MemoryOutputStream::UpdateMetadata( // insert the new metadata from info_to_insert_. Shift all annotations // after the new metadata by the length of the text that was inserted // (including any additional indent length). - if (source_annotation.begin() >= insertion_offset && !crossed_offset) { + if (static_cast(source_annotation.begin()) >= insertion_offset && + !crossed_offset) { crossed_offset = true; InsertShiftedInfo(insertion_content, insertion_offset, indent_length, new_metadata); @@ -905,7 +916,7 @@ CommandLineInterface::MemoryOutputStream::~MemoryOutputStream() { } // Calculate how much space we need. int indent_size = 0; - for (int i = 0; i < data_.size(); i++) { + for (size_t i = 0; i < data_.size(); ++i) { if (data_[i] == '\n') indent_size += indent_.size(); } @@ -947,7 +958,7 @@ CommandLineInterface::CommandLineInterface() : direct_dependencies_violation_msg_( kDefaultDirectDependenciesViolationMsg) {} -CommandLineInterface::~CommandLineInterface() {} +CommandLineInterface::~CommandLineInterface() = default; void CommandLineInterface::RegisterGenerator(const std::string& flag_name, CodeGenerator* generator, @@ -978,13 +989,13 @@ void CommandLineInterface::AllowPlugins(const std::string& exe_name_prefix) { namespace { bool ContainsProto3Optional(const Descriptor* desc) { - for (int i = 0; i < desc->field_count(); i++) { + for (int i = 0; i < desc->field_count(); ++i) { if (desc->field(i)->real_containing_oneof() == nullptr && desc->field(i)->containing_oneof() != nullptr) { return true; } } - for (int i = 0; i < desc->nested_type_count(); i++) { + for (int i = 0; i < desc->nested_type_count(); ++i) { if (ContainsProto3Optional(desc->nested_type(i))) { return true; } @@ -994,7 +1005,7 @@ bool ContainsProto3Optional(const Descriptor* desc) { bool ContainsProto3Optional(Edition edition, const FileDescriptor* file) { if (edition == Edition::EDITION_PROTO3) { - for (int i = 0; i < file->message_type_count(); i++) { + for (int i = 0; i < file->message_type_count(); ++i) { if (ContainsProto3Optional(file->message_type(i))) { return true; } @@ -1207,8 +1218,8 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { databases_per_descriptor_set) { raw_databases_per_descriptor_set.push_back(db.get()); } - descriptor_set_in_database.reset( - new MergedDescriptorDatabase(raw_databases_per_descriptor_set)); + descriptor_set_in_database = std::make_unique( + raw_databases_per_descriptor_set); } if (proto_path_.empty()) { @@ -1219,26 +1230,26 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { return 1; } - error_collector.reset(new ErrorPrinter(error_format_)); - descriptor_pool.reset(new DescriptorPool(descriptor_set_in_database.get(), - error_collector.get())); + error_collector = std::make_unique(error_format_); + descriptor_pool = std::make_unique( + descriptor_set_in_database.get(), error_collector.get()); } else { - disk_source_tree.reset(new DiskSourceTree()); + disk_source_tree = std::make_unique(); if (!InitializeDiskSourceTree(disk_source_tree.get(), descriptor_set_in_database.get())) { return 1; } - error_collector.reset( - new ErrorPrinter(error_format_, disk_source_tree.get())); + error_collector = + std::make_unique(error_format_, disk_source_tree.get()); - source_tree_database.reset(new SourceTreeDescriptorDatabase( - disk_source_tree.get(), descriptor_set_in_database.get())); + source_tree_database = std::make_unique( + disk_source_tree.get(), descriptor_set_in_database.get()); source_tree_database->RecordErrorsTo(error_collector.get()); - descriptor_pool.reset(new DescriptorPool( + descriptor_pool = std::make_unique( source_tree_database.get(), - source_tree_database->GetValidationErrorCollector())); + source_tree_database->GetValidationErrorCollector()); } descriptor_pool->EnforceWeakDependencies(true); @@ -1318,7 +1329,7 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { // Generate output. if (mode_ == MODE_COMPILE) { - for (int i = 0; i < output_directives_.size(); i++) { + for (size_t i = 0; i < output_directives_.size(); ++i) { std::string output_location = output_directives_[i].output_location; if (!absl::EndsWith(output_location, ".zip") && !absl::EndsWith(output_location, ".jar") && @@ -1405,7 +1416,7 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { if (mode_ == MODE_PRINT) { switch (print_mode_) { case PRINT_FREE_FIELDS: - for (int i = 0; i < parsed_files.size(); ++i) { + for (size_t i = 0; i < parsed_files.size(); ++i) { const FileDescriptor* fd = parsed_files[i]; for (int j = 0; j < fd->message_type_count(); ++j) { PrintFreeFieldNumbers(fd->message_type(j)); @@ -1429,7 +1440,7 @@ bool CommandLineInterface::InitializeDiskSourceTree( AddDefaultProtoPaths(&proto_path_); // Set up the source tree. - for (int i = 0; i < proto_path_.size(); i++) { + for (size_t i = 0; i < proto_path_.size(); ++i) { source_tree->MapPath(proto_path_[i].first, proto_path_[i].second); } @@ -1631,7 +1642,7 @@ bool CommandLineInterface::ParseInputFiles( // Enforce --direct_dependencies if (direct_dependencies_explicitly_set_) { bool indirect_imports = false; - for (int i = 0; i < parsed_file->dependency_count(); i++) { + for (int i = 0; i < parsed_file->dependency_count(); ++i) { if (direct_dependencies_.find(parsed_file->dependency(i)->name()) == direct_dependencies_.end()) { indirect_imports = true; @@ -1821,7 +1832,7 @@ CommandLineInterface::ParseArgumentStatus CommandLineInterface::ParseArguments( } // Iterate through all arguments and parse them. - for (int i = 0; i < arguments.size(); ++i) { + for (size_t i = 0; i < arguments.size(); ++i) { std::string name, value; if (ParseArgument(arguments[i].c_str(), &name, &value)) { @@ -2067,7 +2078,7 @@ CommandLineInterface::InterpretArgument(const std::string& name, value, absl::ByAnyChar(CommandLineInterface::kPathSeparator), absl::SkipEmpty()); - for (int i = 0; i < parts.size(); i++) { + for (size_t i = 0; i < parts.size(); ++i) { std::string virtual_path; std::string disk_path; @@ -2119,7 +2130,7 @@ CommandLineInterface::InterpretArgument(const std::string& name, direct_dependencies_explicitly_set_ = true; std::vector direct = - absl::StrSplit(value, ":", absl::SkipEmpty()); + absl::StrSplit(value, ':', absl::SkipEmpty()); ABSL_DCHECK(direct_dependencies_.empty()); direct_dependencies_.insert(direct.begin(), direct.end()); @@ -2686,7 +2697,7 @@ bool CommandLineInterface::GenerateDependencyManifestFile( FileDescriptorSet file_set; absl::flat_hash_set already_seen; - for (int i = 0; i < parsed_files.size(); i++) { + for (size_t i = 0; i < parsed_files.size(); ++i) { GetTransitiveDependencies(parsed_files[i], &already_seen, file_set.mutable_file()); } @@ -2697,7 +2708,7 @@ bool CommandLineInterface::GenerateDependencyManifestFile( GeneratorContextImpl* directory = pair.second.get(); std::vector relative_output_filenames; directory->GetOutputFilenames(&relative_output_filenames); - for (int i = 0; i < relative_output_filenames.size(); i++) { + for (size_t i = 0; i < relative_output_filenames.size(); ++i) { std::string output_filename = location + relative_output_filenames[i]; if (output_filename.compare(0, 2, "./") == 0) { output_filename = output_filename.substr(2); @@ -2732,7 +2743,7 @@ bool CommandLineInterface::GenerateDependencyManifestFile( io::FileOutputStream out(fd); io::Printer printer(&out, '$'); - for (size_t i = 0; i < output_filenames.size(); i++) { + for (size_t i = 0; i < output_filenames.size(); ++i) { printer.Print(output_filenames[i]); if (i == output_filenames.size() - 1) { printer.Print(":"); @@ -2741,7 +2752,7 @@ bool CommandLineInterface::GenerateDependencyManifestFile( } } - for (int i = 0; i < file_set.file_size(); i++) { + for (int i = 0; i < file_set.file_size(); ++i) { const FileDescriptorProto& file = file_set.file(i); const std::string& virtual_file = file.name(); std::string disk_file; @@ -2830,7 +2841,7 @@ bool CommandLineInterface::GeneratePluginOutput( // Write the files. We do this even if there was a generator error in order // to match the behavior of a compiled-in generator. std::unique_ptr current_output; - for (int i = 0; i < response.file_size(); i++) { + for (int i = 0; i < response.file_size(); ++i) { const CodeGeneratorResponse::File& output_file = response.file(i); if (!output_file.insertion_point().empty()) { @@ -2960,7 +2971,7 @@ bool CommandLineInterface::WriteDescriptorSet( // in GetTransitiveDependencies. absl::flat_hash_set to_output; to_output.insert(parsed_files.begin(), parsed_files.end()); - for (int i = 0; i < parsed_files.size(); i++) { + for (size_t i = 0; i < parsed_files.size(); ++i) { const FileDescriptor* file = parsed_files[i]; for (int j = 0; j < file->dependency_count(); j++) { const FileDescriptor* dependency = file->dependency(j); @@ -2975,7 +2986,7 @@ bool CommandLineInterface::WriteDescriptorSet( options.include_json_name = true; options.include_source_code_info = source_info_in_descriptor_set_; options.retain_options = retain_options_in_descriptor_set_; - for (int i = 0; i < parsed_files.size(); i++) { + for (size_t i = 0; i < parsed_files.size(); ++i) { GetTransitiveDependencies(parsed_files[i], &already_seen, file_set.mutable_file(), options); } @@ -3191,7 +3202,7 @@ void CommandLineInterface::PrintFreeFieldNumbers(const Descriptor* descriptor) { std::vector nested_messages; GatherOccupiedFieldRanges(descriptor, &ranges, &nested_messages); - for (int i = 0; i < nested_messages.size(); ++i) { + for (size_t i = 0; i < nested_messages.size(); ++i) { PrintFreeFieldNumbers(nested_messages[i]); } FormatFreeFieldNumbers(descriptor->full_name(), ranges); diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 133a393df4..8c0dfe17d8 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -13,12 +13,14 @@ #include #include +#include #include #include #include "absl/log/absl_check.h" +#include "absl/strings/escaping.h" #include "absl/strings/str_cat.h" -#include "absl/strings/str_format.h" +#include "absl/types/span.h" #include "google/protobuf/compiler/command_line_interface_tester.h" #include "google/protobuf/unittest_features.pb.h" #include "google/protobuf/unittest_invalid_features.pb.h" @@ -31,6 +33,7 @@ #include #include +#include "google/protobuf/testing/file.h" #include "google/protobuf/testing/file.h" #include "google/protobuf/any.pb.h" #include "google/protobuf/descriptor.pb.h" @@ -44,8 +47,6 @@ #include "google/protobuf/compiler/cpp/names.h" #include "google/protobuf/compiler/mock_code_generator.h" #include "google/protobuf/compiler/plugin.pb.h" -#include "google/protobuf/compiler/subprocess.h" -#include "google/protobuf/io/io_win32.h" #include "google/protobuf/test_textproto.h" #include "google/protobuf/test_util2.h" #include "google/protobuf/unittest.pb.h" @@ -56,6 +57,11 @@ #include "google/protobuf/compiler/test_plugin_paths.h" #endif // GOOGLE_PROTOBUF_USE_BAZEL_GENERATED_PLUGIN_PATHS +#ifdef _WIN32 +#include "google/protobuf/compiler/subprocess.h" +#include "google/protobuf/io/io_win32.h" +#endif + // Must be included last. #include "google/protobuf/port_def.inc" @@ -106,7 +112,7 @@ std::string CreatePluginArg() { "test_plugin.exe", // Other Win32 (MSVC) "test_plugin", // Unix }; - for (int i = 0; i < ABSL_ARRAYSIZE(possible_paths); i++) { + for (int i = 0; i < ABSL_ARRAYSIZE(possible_paths); ++i) { if (access(possible_paths[i], F_OK) == 0) { plugin_path = possible_paths[i]; break; @@ -215,7 +221,7 @@ class CommandLineInterfaceTest : public CommandLineInterfaceTester { class CommandLineInterfaceTest::NullCodeGenerator : public CodeGenerator { public: NullCodeGenerator() : called_(false) {} - ~NullCodeGenerator() override {} + ~NullCodeGenerator() override = default; mutable bool called_; mutable std::string parameter_; @@ -812,7 +818,7 @@ TEST_F(CommandLineInterfaceTest, foo_file_descriptor_proto.set_name("foo.proto"); foo_file_descriptor_proto.add_message_type()->set_name("Foo"); - file_descriptor_set.add_file()->CopyFrom(foo_file_descriptor_proto); + *file_descriptor_set.add_file() = foo_file_descriptor_proto; FileDescriptorProto* file_descriptor_proto = file_descriptor_set.add_file(); file_descriptor_proto->set_name("bar.proto"); @@ -831,7 +837,7 @@ TEST_F(CommandLineInterfaceTest, WriteDescriptorSet("foo_and_bar.bin", &file_descriptor_set); file_descriptor_set.clear_file(); - file_descriptor_set.add_file()->CopyFrom(foo_file_descriptor_proto); + *file_descriptor_set.add_file() = foo_file_descriptor_proto; file_descriptor_proto = file_descriptor_set.add_file(); file_descriptor_proto->set_name("baz.proto"); @@ -3948,7 +3954,7 @@ class EncodeDecodeTest : public testing::TestWithParam { std::string StripCR(const std::string& text) { std::string result; - for (int i = 0; i < text.size(); i++) { + for (size_t i = 0; i < text.size(); ++i) { if (text[i] != '\r') { result.push_back(text[i]); } @@ -3964,7 +3970,7 @@ class EncodeDecodeTest : public testing::TestWithParam { std::vector args; args.push_back("protoc"); for (absl::string_view split_piece : - absl::StrSplit(command, " ", absl::SkipEmpty())) { + absl::StrSplit(command, ' ', absl::SkipEmpty())) { args.push_back(std::string(split_piece)); } if (specify_proto_files) { @@ -3983,7 +3989,7 @@ class EncodeDecodeTest : public testing::TestWithParam { } std::unique_ptr argv(new const char*[args.size()]); - for (int i = 0; i < args.size(); i++) { + for (size_t i = 0; i < args.size(); ++i) { argv[i] = args[i].c_str(); } diff --git a/src/google/protobuf/compiler/cpp/BUILD.bazel b/src/google/protobuf/compiler/cpp/BUILD.bazel index 9330442d4b..f4a1ffeea1 100644 --- a/src/google/protobuf/compiler/cpp/BUILD.bazel +++ b/src/google/protobuf/compiler/cpp/BUILD.bazel @@ -120,6 +120,7 @@ cc_library( "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/synchronization", "@com_google_absl//absl/types:optional", + "@com_google_absl//absl/types:span", ], ) diff --git a/src/google/protobuf/compiler/cpp/field.cc b/src/google/protobuf/compiler/cpp/field.cc index 471c1eb9d3..eedb5b64bd 100644 --- a/src/google/protobuf/compiler/cpp/field.cc +++ b/src/google/protobuf/compiler/cpp/field.cc @@ -11,6 +11,7 @@ #include "google/protobuf/compiler/cpp/field.h" +#include #include #include #include @@ -21,6 +22,7 @@ #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "absl/types/span.h" #include "google/protobuf/compiler/cpp/field_generators/generators.h" #include "google/protobuf/compiler/cpp/helpers.h" #include "google/protobuf/compiler/cpp/options.h" @@ -341,18 +343,18 @@ void FieldGeneratorTable::Build( absl::Span has_bit_indices, absl::Span inlined_string_indices) { // Construct all the FieldGenerators. - fields_.reserve(descriptor_->field_count()); + fields_.reserve(static_cast(descriptor_->field_count())); for (const auto* field : internal::FieldRange(descriptor_)) { + size_t index = static_cast(field->index()); absl::optional has_bit_index; - if (!has_bit_indices.empty() && has_bit_indices[field->index()] >= 0) { - has_bit_index = static_cast(has_bit_indices[field->index()]); + if (!has_bit_indices.empty() && has_bit_indices[index] >= 0) { + has_bit_index = static_cast(has_bit_indices[index]); } absl::optional inlined_string_index; - if (!inlined_string_indices.empty() && - inlined_string_indices[field->index()] >= 0) { + if (!inlined_string_indices.empty() && inlined_string_indices[index] >= 0) { inlined_string_index = - static_cast(inlined_string_indices[field->index()]); + static_cast(inlined_string_indices[index]); } fields_.push_back(FieldGenerator(field, options, scc, has_bit_index, diff --git a/src/google/protobuf/compiler/cpp/field_generators/generators.h b/src/google/protobuf/compiler/cpp/field_generators/generators.h index 376bc5b4b3..28948ec350 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/generators.h +++ b/src/google/protobuf/compiler/cpp/field_generators/generators.h @@ -12,6 +12,8 @@ #include "google/protobuf/compiler/cpp/field.h" #include "google/protobuf/compiler/cpp/helpers.h" +#include "google/protobuf/compiler/cpp/options.h" +#include "google/protobuf/descriptor.h" // The functions in this file construct FieldGeneratorBase objects for // generating different "codegen types" of fields. The logic for selecting the diff --git a/src/google/protobuf/compiler/cpp/field_generators/string_field.cc b/src/google/protobuf/compiler/cpp/field_generators/string_field.cc index dfd84252bf..c64967de8b 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/string_field.cc +++ b/src/google/protobuf/compiler/cpp/field_generators/string_field.cc @@ -739,7 +739,7 @@ class RepeatedString : public FieldGeneratorBase { std::vector MakeVars() const override { return Vars(field_, *opts_); } void GeneratePrivateMembers(io::Printer* p) const override { - if (ShouldSplit(descriptor_, options_)) { + if (should_split()) { p->Emit(R"cc( $pbi$::RawPtr<$pb$::RepeatedPtrField> $name$_; )cc"); @@ -766,7 +766,7 @@ class RepeatedString : public FieldGeneratorBase { _this->_internal_mutable_$name$()->MergeFrom(from._internal_$name$()); )cc"); }; - if (!ShouldSplit(descriptor_, options_)) { + if (!should_split()) { body(); } else { p->Emit({{"body", body}}, R"cc( @@ -778,14 +778,14 @@ class RepeatedString : public FieldGeneratorBase { } void GenerateSwappingCode(io::Printer* p) const override { - ABSL_CHECK(!ShouldSplit(descriptor_, options_)); + ABSL_CHECK(!should_split()); p->Emit(R"cc( $field_$.InternalSwap(&other->$field_$); )cc"); } void GenerateDestructorCode(io::Printer* p) const override { - if (ShouldSplit(descriptor_, options_)) { + if (should_split()) { p->Emit(R"cc( $field_$.DeleteIfNotDefault(); )cc"); @@ -795,7 +795,7 @@ class RepeatedString : public FieldGeneratorBase { void GenerateConstructorCode(io::Printer* p) const override {} void GenerateCopyConstructorCode(io::Printer* p) const override { - if (ShouldSplit(descriptor_, options_)) { + if (should_split()) { p->Emit(R"cc( if (!from._internal_$name$().empty()) { _internal_mutable_$name$()->MergeFrom(from._internal_$name$()); @@ -984,7 +984,7 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const { return _internal_mutable_$name_internal$(); } )cc"); - if (ShouldSplit(descriptor_, options_)) { + if (should_split()) { p->Emit(R"cc( inline const $pb$::RepeatedPtrField& $Msg$::_internal_$name_internal$() const {