Skip to content

Commit

Permalink
Fix various unsigned to signed comparison warnings. (#17212)
Browse files Browse the repository at this point in the history 
GCC aggressively emit warnings when comparing unsigned and signed integer types, which causes failures under protobuf's -Werror default. We can fix these often by switching to iterators, but sometimes it's easiest to add a cast or switch a variable type.

Closes #17212

COPYBARA_INTEGRATE_REVIEW=#17212 from benjaminp:unsigned-size-comparison-warnings 4b3c9c2
FUTURE_COPYBARA_INTEGRATE_REVIEW=#17212 from benjaminp:unsigned-size-comparison-warnings 4b3c9c2
PiperOrigin-RevId: 680638328
  • Loading branch information
@benjaminp @copybara-github
benjaminp authored and copybara-github committed Oct 1, 2024
1 parent 1db4fdc commit 1cf1990
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 52 deletions.
1 change: 1 addition & 0 deletions src/google/protobuf/compiler/java/full/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ cc_library(
":service",
"//src/google/protobuf",
"//src/google/protobuf:port",
"//src/google/protobuf:protobuf_lite",
"//src/google/protobuf/compiler/java:generator_common",
"//src/google/protobuf/compiler/java:helpers",
"//src/google/protobuf/compiler/java:message_serialization",
Expand Down
33 changes: 16 additions & 17 deletions src/google/protobuf/compiler/java/full/enum.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "absl/container/flat_hash_map.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/compiler/java/context.h"
#include "google/protobuf/compiler/java/doc_comment.h"
#include "google/protobuf/compiler/java/helpers.h"
Expand Down Expand Up @@ -82,22 +83,21 @@ void EnumNonLiteGenerator::Generate(io::Printer* printer) {
}
}

for (int i = 0; i < canonical_values_.size(); i++) {
for (const EnumValueDescriptor* value : canonical_values_) {
absl::flat_hash_map<absl::string_view, std::string> vars;
vars["name"] = canonical_values_[i]->name();
vars["index"] = absl::StrCat(canonical_values_[i]->index());
vars["number"] = absl::StrCat(canonical_values_[i]->number());
WriteEnumValueDocComment(printer, canonical_values_[i],
context_->options());
if (canonical_values_[i]->options().deprecated()) {
vars["name"] = value->name();
vars["index"] = absl::StrCat(value->index());
vars["number"] = absl::StrCat(value->number());
WriteEnumValueDocComment(printer, value, context_->options());
if (value->options().deprecated()) {
printer->Print("@java.lang.Deprecated\n");
}
if (ordinal_is_index) {
printer->Print(vars, "$name$($number$),\n");
} else {
printer->Print(vars, "$name$($index$, $number$),\n");
}
printer->Annotate("name", canonical_values_[i]);
printer->Annotate("name", value);
}

if (!descriptor_->is_closed()) {
Expand All @@ -122,15 +122,15 @@ void EnumNonLiteGenerator::Generate(io::Printer* printer) {
printer->Outdent();
printer->Print("}\n");

for (int i = 0; i < aliases_.size(); i++) {
for (const Alias& alias : aliases_) {
absl::flat_hash_map<absl::string_view, std::string> vars;
vars["classname"] = descriptor_->name();
vars["name"] = aliases_[i].value->name();
vars["canonical_name"] = aliases_[i].canonical_value->name();
WriteEnumValueDocComment(printer, aliases_[i].value, context_->options());
vars["name"] = alias.value->name();
vars["canonical_name"] = alias.canonical_value->name();
WriteEnumValueDocComment(printer, alias.value, context_->options());
printer->Print(
vars, "public static final $classname$ $name$ = $canonical_name$;\n");
printer->Annotate("name", aliases_[i].value);
printer->Annotate("name", alias.value);
}

for (int i = 0; i < descriptor_->value_count(); i++) {
Expand Down Expand Up @@ -206,10 +206,9 @@ void EnumNonLiteGenerator::Generate(io::Printer* printer) {
printer->Indent();
printer->Indent();

for (int i = 0; i < canonical_values_.size(); i++) {
printer->Print("case $number$: return $name$;\n", "name",
canonical_values_[i]->name(), "number",
absl::StrCat(canonical_values_[i]->number()));
for (const EnumValueDescriptor* value : canonical_values_) {
printer->Print("case $number$: return $name$;\n", "name", value->name(),
"number", absl::StrCat(value->number()));
}

printer->Outdent();
Expand Down
20 changes: 9 additions & 11 deletions src/google/protobuf/compiler/java/full/message.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <cstdint>
#include <memory>
#include <string>
#include <utility>
#include <vector>

#include "absl/container/flat_hash_map.h"
Expand All @@ -27,6 +28,7 @@
#include "google/protobuf/compiler/java/doc_comment.h"
#include "google/protobuf/compiler/java/field_common.h"
#include "google/protobuf/compiler/java/generator_common.h"
#include "google/protobuf/compiler/java/generator_factory.h"
#include "google/protobuf/compiler/java/helpers.h"
#include "google/protobuf/compiler/java/full/enum.h"
#include "google/protobuf/compiler/java/full/extension.h"
Expand All @@ -48,9 +50,6 @@ namespace protobuf {
namespace compiler {
namespace java {

using internal::WireFormat;
using internal::WireFormatLite;

namespace {
std::string MapValueImmutableClassdName(const Descriptor* descriptor,
ClassNameResolver* name_resolver) {
Expand All @@ -72,7 +71,7 @@ MessageGenerator::MessageGenerator(const Descriptor* descriptor)
}
}

MessageGenerator::~MessageGenerator() {}
MessageGenerator::~MessageGenerator() = default;

// ===================================================================
ImmutableMessageGenerator::ImmutableMessageGenerator(
Expand All @@ -86,7 +85,7 @@ ImmutableMessageGenerator::ImmutableMessageGenerator(
"generate lite messages.";
}

ImmutableMessageGenerator::~ImmutableMessageGenerator() {}
ImmutableMessageGenerator::~ImmutableMessageGenerator() = default;

void ImmutableMessageGenerator::GenerateStaticVariables(
io::Printer* printer, int* bytecode_estimate) {
Expand Down Expand Up @@ -273,7 +272,7 @@ void ImmutableMessageGenerator::GenerateInterface(io::Printer* printer) {
field_generators_.get(descriptor_->field(i))
.GenerateInterfaceMembers(printer);
}
for (auto& kv : oneofs_) {
for (const auto& kv : oneofs_) {
printer->Print(
"\n"
"$classname$.$oneof_capitalized_name$Case "
Expand Down Expand Up @@ -394,7 +393,7 @@ void ImmutableMessageGenerator::Generate(io::Printer* printer) {

// oneof
absl::flat_hash_map<absl::string_view, std::string> vars;
for (auto& kv : oneofs_) {
for (const auto& kv : oneofs_) {
const OneofDescriptor* oneof = kv.second;
vars["oneof_name"] = context_->GetOneofGeneratorInfo(oneof)->name;
vars["oneof_capitalized_name"] =
Expand Down Expand Up @@ -804,8 +803,7 @@ void ImmutableMessageGenerator::GenerateDescriptorMethods(
" switch (number) {\n");
printer->Indent();
printer->Indent();
for (int i = 0; i < map_fields.size(); ++i) {
const FieldDescriptor* field = map_fields[i];
for (const FieldDescriptor* field : map_fields) {
const FieldGeneratorInfo* info = context_->GetFieldGeneratorInfo(field);
printer->Print(
"case $number$:\n"
Expand Down Expand Up @@ -994,7 +992,7 @@ void ImmutableMessageGenerator::GenerateEqualsAndHashCode(
}

// Compare oneofs.
for (auto& kv : oneofs_) {
for (const auto& kv : oneofs_) {
const OneofDescriptor* oneof = kv.second;
printer->Print(
"if (!get$oneof_capitalized_name$Case().equals("
Expand Down Expand Up @@ -1074,7 +1072,7 @@ void ImmutableMessageGenerator::GenerateEqualsAndHashCode(
}

// hashCode oneofs.
for (auto& kv : oneofs_) {
for (const auto& kv : oneofs_) {
const OneofDescriptor* oneof = kv.second;
printer->Print("switch ($oneof_name$Case_) {\n", "oneof_name",
context_->GetOneofGeneratorInfo(oneof)->name);
Expand Down
9 changes: 4 additions & 5 deletions src/google/protobuf/compiler/java/full/message_builder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "google/protobuf/descriptor.pb.h"
#include "google/protobuf/io/printer.h"
#include "google/protobuf/wire_format.h"
#include "google/protobuf/wire_format_lite.h"

// Must be last.
#include "google/protobuf/port_def.inc"
Expand Down Expand Up @@ -92,7 +93,7 @@ MessageBuilderGenerator::MessageBuilderGenerator(const Descriptor* descriptor,
}
}

MessageBuilderGenerator::~MessageBuilderGenerator() {}
MessageBuilderGenerator::~MessageBuilderGenerator() = default;

void MessageBuilderGenerator::Generate(io::Printer* printer) {
WriteMessageDocComment(printer, descriptor_, context_->options());
Expand Down Expand Up @@ -211,8 +212,7 @@ void MessageBuilderGenerator::GenerateDescriptorMethods(io::Printer* printer) {
" switch (number) {\n");
printer->Indent();
printer->Indent();
for (int i = 0; i < map_fields.size(); ++i) {
const FieldDescriptor* field = map_fields[i];
for (const FieldDescriptor* field : map_fields) {
const FieldGeneratorInfo* info = context_->GetFieldGeneratorInfo(field);
printer->Print(
"case $number$:\n"
Expand All @@ -237,8 +237,7 @@ void MessageBuilderGenerator::GenerateDescriptorMethods(io::Printer* printer) {
" switch (number) {\n");
printer->Indent();
printer->Indent();
for (int i = 0; i < map_fields.size(); ++i) {
const FieldDescriptor* field = map_fields[i];
for (const FieldDescriptor* field : map_fields) {
const FieldGeneratorInfo* info = context_->GetFieldGeneratorInfo(field);
printer->Print(
"case $number$:\n"
Expand Down
32 changes: 16 additions & 16 deletions src/google/protobuf/compiler/java/lite/enum.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

#include "absl/container/flat_hash_map.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/compiler/java/context.h"
#include "google/protobuf/compiler/java/doc_comment.h"
#include "google/protobuf/compiler/java/helpers.h"
#include "google/protobuf/compiler/java/internal_helpers.h"
#include "google/protobuf/compiler/java/name_resolver.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/descriptor.pb.h"
#include "google/protobuf/io/printer.h"

Expand Down Expand Up @@ -67,17 +69,16 @@ void EnumLiteGenerator::Generate(io::Printer* printer) {
printer->Annotate("classname", descriptor_);
printer->Indent();

for (int i = 0; i < canonical_values_.size(); i++) {
for (const EnumValueDescriptor* value : canonical_values_) {
absl::flat_hash_map<absl::string_view, std::string> vars;
vars["name"] = canonical_values_[i]->name();
vars["number"] = absl::StrCat(canonical_values_[i]->number());
WriteEnumValueDocComment(printer, canonical_values_[i],
context_->options());
if (canonical_values_[i]->options().deprecated()) {
vars["name"] = value->name();
vars["number"] = absl::StrCat(value->number());
WriteEnumValueDocComment(printer, value, context_->options());
if (value->options().deprecated()) {
printer->Print("@java.lang.Deprecated\n");
}
printer->Print(vars, "$name$($number$),\n");
printer->Annotate("name", canonical_values_[i]);
printer->Annotate("name", value);
}

if (!descriptor_->is_closed()) {
Expand All @@ -91,15 +92,15 @@ void EnumLiteGenerator::Generate(io::Printer* printer) {

// -----------------------------------------------------------------

for (int i = 0; i < aliases_.size(); i++) {
for (const Alias& alias : aliases_) {
absl::flat_hash_map<absl::string_view, std::string> vars;
vars["classname"] = descriptor_->name();
vars["name"] = aliases_[i].value->name();
vars["canonical_name"] = aliases_[i].canonical_value->name();
WriteEnumValueDocComment(printer, aliases_[i].value, context_->options());
vars["name"] = alias.value->name();
vars["canonical_name"] = alias.canonical_value->name();
WriteEnumValueDocComment(printer, alias.value, context_->options());
printer->Print(
vars, "public static final $classname$ $name$ = $canonical_name$;\n");
printer->Annotate("name", aliases_[i].value);
printer->Annotate("name", alias.value);
}

for (int i = 0; i < descriptor_->value_count(); i++) {
Expand Down Expand Up @@ -162,10 +163,9 @@ void EnumLiteGenerator::Generate(io::Printer* printer) {
printer->Indent();
printer->Indent();

for (int i = 0; i < canonical_values_.size(); i++) {
printer->Print("case $number$: return $name$;\n", "name",
canonical_values_[i]->name(), "number",
absl::StrCat(canonical_values_[i]->number()));
for (const EnumValueDescriptor* value : canonical_values_) {
printer->Print("case $number$: return $name$;\n", "name", value->name(),
"number", absl::StrCat(value->number()));
}

printer->Outdent();
Expand Down
4 changes: 3 additions & 1 deletion src/google/protobuf/compiler/rust/relative_path.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

#include "google/protobuf/compiler/rust/relative_path.h"

#include <cstddef>
#include <string>
#include <vector>

#include "absl/algorithm/container.h"
#include "absl/log/absl_check.h"
#include "absl/strings/match.h"
#include "absl/strings/str_join.h"
#include "absl/strings/str_split.h"
Expand Down Expand Up @@ -62,7 +64,7 @@ std::string RelativePath::Relative(const RelativePath& dest) const {
result.push_back(segment);
}
// Push `..` from the common ancestor to the current path.
for (int i = 0; i < current_segments.size(); ++i) {
for (size_t i = 0; i < current_segments.size(); ++i) {
result.push_back("..");
}
absl::c_reverse(result);
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/io/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ cc_library(
copts = COPTS,
strip_include_prefix = "/src",
deps = [
":io",
":zero_copy_sink",
"//src/google/protobuf:port",
"//src/google/protobuf/stubs",
Expand Down
6 changes: 4 additions & 2 deletions src/google/protobuf/io/printer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@
#include "absl/strings/str_format.h"
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "absl/types/span.h"
#include "absl/types/variant.h"
#include "google/protobuf/io/zero_copy_sink.h"
#include "google/protobuf/io/zero_copy_stream.h"


// Must be included last.
Expand Down Expand Up @@ -124,8 +126,8 @@ class AnnotationProtoCollector : public AnnotationCollector {
const std::string& file_path, const std::vector<int>& path,
absl::optional<Semantic> semantic) override {
auto* annotation = annotation_proto_->add_annotation();
for (int i = 0; i < path.size(); ++i) {
annotation->add_path(path[i]);
for (const int segment : path) {
annotation->add_path(segment);
}
annotation->set_source_file(file_path);
annotation->set_begin(begin_offset);
Expand Down

0 comments on commit 1cf1990

Please sign in to comment.