Don't assign hasbits to likely-present repeated fields with hasbits for repeated fields enabled.

ClaytonKnittel · copybara-github · commit 15c9e9a6e968 · 2025-08-07T20:41:19.000-07:00
PiperOrigin-RevId: 792437227
diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc
@@ -1098,8 +1098,11 @@ std::optional<float> GetFieldGroupPresenceProbability(
 
 HasbitMode GetFieldHasbitMode(const FieldDescriptor* field,
                               const Options& options) {
-  // TODO: Use profile data to determine the hasbit mode for fields
-  // with optional hasbits.
+  if (IsProfileDriven(options) && field->is_repeated() &&
+      IsLikelyPresent(field, options)) {
+    return HasbitMode::kNoHasbit;
+  }
+
   return internal::cpp::GetFieldHasbitModeWithoutProfile(field);
 }
 
diff --git a/src/google/protobuf/compiler/cpp/message.cc b/src/google/protobuf/compiler/cpp/message.cc
@@ -694,10 +694,13 @@ MessageGenerator::MessageGenerator(
 
 bool MessageGenerator::ShouldGenerateEnclosingIf(
     const FieldDescriptor& field) const {
-  return HasBitIndex(&field) != kNoHasbit &&
-         (field.is_repeated() ||
-          field.cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ||
-          field.cpp_type() == FieldDescriptor::CPPTYPE_STRING);
+  if (HasBitIndex(&field) == kNoHasbit) return false;
+  // Always check hasbits for repeated fields since likely repeated fields,
+  // which aren't assigned hasbits, would bail in the previous check.
+  if (field.is_repeated()) return true;
+  // Always check hasbits for message and string fields before clearing them.
+  return field.cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ||
+         field.cpp_type() == FieldDescriptor::CPPTYPE_STRING;
 }
 
 size_t MessageGenerator::HasBitsSize() const {
@@ -3304,8 +3307,9 @@ void MessageGenerator::GenerateCopyInitFields(io::Printer* p) const {
 
   auto generate_copy_fields = [&] {
     for (auto it = begin; it != end; ++it) {
-      const auto& gen = field_generators_.get(*it);
-      auto v = p->WithVars(FieldVars(*it, options_));
+      const auto* field = *it;
+      const auto& gen = field_generators_.get(field);
+      auto v = p->WithVars(FieldVars(field, options_));
 
       // Non trivial field values are copy constructed
       if (!gen.has_trivial_value() || gen.should_split()) {
@@ -3315,9 +3319,9 @@ void MessageGenerator::GenerateCopyInitFields(io::Printer* p) const {
 
       if (gen.is_message()) {
         emit_pending_copy_fields(it, false);
-        emit_copy_message(*it);
+        emit_copy_message(field);
       } else if (first == nullptr) {
-        first = *it;
+        first = field;
       }
     }
     emit_pending_copy_fields(end, false);
diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc
@@ -3265,7 +3265,8 @@ void Reflection::ClearHasBit(Message* message,
 void Reflection::NaiveSwapHasBit(Message* message1, Message* message2,
                                  const FieldDescriptor* field) const {
   ABSL_DCHECK(!field->options().weak());
-  if (!schema_.HasHasbits()) {
+  if (!schema_.HasHasbits() ||
+      schema_.HasBitIndex(field) == static_cast<uint32_t>(kNoHasbit)) {
     return;
   }
   const Reflection* r1 = message1->GetReflection();
@@ -3621,7 +3622,9 @@ void Reflection::PopulateTcParseEntries(
       entries->has_idx = schema_.oneof_case_offset_ + 4 * oneof->index();
     } else if (schema_.HasHasbits()) {
       entries->has_idx =
-          static_cast<int>(8 * schema_.HasBitsOffset() + entry.hasbit_idx);
+          entry.hasbit_idx >= 0
+              ? static_cast<int>(8 * schema_.HasBitsOffset() + entry.hasbit_idx)
+              : kNoHasbit;
     } else {
       entries->has_idx = 0;
     }
diff --git a/src/google/protobuf/generated_message_tctable_lite.cc b/src/google/protobuf/generated_message_tctable_lite.cc
@@ -114,7 +114,8 @@ absl::Status TcParser::VerifyHasBitConsistency(const MessageLite* msg,
                           msg->GetTypeName(), FieldNumber(table, &entry)));
     };
     const auto cardinality = entry.type_card & fl::kFcMask;
-    if (cardinality == fl::kFcSingular || cardinality == fl::kFcOneof) {
+    if (cardinality == fl::kFcSingular || cardinality == fl::kFcOneof ||
+        entry.has_idx == kNoHasbit) {
       continue;
     }
     if (!EnableExperimentalHintHasBitsForRepeatedFields() &&
