Improve MergeFrom behavior for oneof fields:

- Avoid redundant calls to `GetArena()`.
 - Only do a single call to `xxx_clear()` if needed.
 - Set the oneof_case once, if needed.
 - Use CopyConstruct for new objects, like we do for non-oneof Merge.
 - Avoid the _Internal::mutable_xxx functions, as they are not needed anymore.

PiperOrigin-RevId: 585705944

src/google/protobuf/compiler/cpp/field_generators/cord_field.cc

@@ -119,8 +119,10 @@ class CordOneofFieldGenerator : public CordFieldGenerator {
   void GenerateInlineAccessorDefinitions(io::Printer* printer) const override;
   void GenerateNonInlineAccessorDefinitions(
       io::Printer* printer) const override;
+  bool RequiresArena(GeneratorFunction func) const override;
   void GenerateClearingCode(io::Printer* printer) const override;
   void GenerateSwappingCode(io::Printer* printer) const override;
+  void GenerateMergingCode(io::Printer* printer) const override;
   void GenerateConstructorCode(io::Printer* printer) const override {}
   void GenerateArenaDestructorCode(io::Printer* printer) const override;
   // Overrides CordFieldGenerator behavior.
@@ -347,7 +349,7 @@ void CordOneofFieldGenerator::GenerateInlineAccessorDefinitions(
     }
   )cc");
   printer->Emit(R"cc(
-    inline void $classname$::_internal_set_$name$(const ::absl::Cord& value) {
+    inline void $classname$::set_$name$(const ::absl::Cord& value) {
       if ($not_has_field$) {
         clear_$oneof_name$();
         set_has_$name$();
@@ -358,11 +360,6 @@ void CordOneofFieldGenerator::GenerateInlineAccessorDefinitions(
         }
       }
       *$field$ = value;
-    }
-  )cc");
-  printer->Emit(R"cc(
-    inline void $classname$::set_$name$(const ::absl::Cord& value) {
-      _internal_set_$name$(value);
       $annotate_set$;
       // @@protoc_insertion_point(field_set:$full_name$)
     }
@@ -411,6 +408,14 @@ void CordOneofFieldGenerator::GenerateNonInlineAccessorDefinitions(
   }
 }
 
+bool CordOneofFieldGenerator::RequiresArena(GeneratorFunction func) const {
+  switch (func) {
+    case GeneratorFunction::kMergeFrom:
+      return true;
+  }
+  return false;
+}
+
 void CordOneofFieldGenerator::GenerateClearingCode(io::Printer* printer) const {
   Formatter format(printer, variables_);
   format(
@@ -429,6 +434,15 @@ void CordOneofFieldGenerator::GenerateArenaDestructorCode(
   // default behavior here.
 }
 
+void CordOneofFieldGenerator::GenerateMergingCode(io::Printer* printer) const {
+  printer->Emit(R"cc(
+    if (oneof_needs_init) {
+      _this->$field$ = ::$proto_ns$::Arena::Create<absl::Cord>(arena);
+    }
+    *_this->$field$ = *from.$field$;
+  )cc");
+}
+
 // ===================================================================
 }  // namespace
 

src/google/protobuf/compiler/cpp/field_generators/enum_field.cc

@@ -74,7 +74,7 @@ class SingularEnum : public FieldGeneratorBase {
 
   void GenerateMergingCode(io::Printer* p) const override {
     p->Emit(R"cc(
-      _this->_internal_set_$name$(from._internal_$name$());
+      _this->$field_$ = from.$field_$;
     )cc");
   }
 
@@ -175,7 +175,12 @@ void SingularEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
     p->Emit(R"cc(
       inline void $Msg$::set_$name$($Enum$ value) {
         $PrepareSplitMessageForWrite$;
-        _internal_set_$name$(value);
+        $assert_valid$;
+        if ($not_has_field$) {
+          clear_$oneof_name$();
+          set_has_$name$();
+        }
+        $field_$ = value;
         $annotate_set$;
         // @@protoc_insertion_point(field_set:$pkg.Msg.field$)
       }
@@ -185,14 +190,6 @@ void SingularEnum::GenerateInlineAccessorDefinitions(io::Printer* p) const {
         }
         return static_cast<$Enum$>($kDefault$);
       }
-      inline void $Msg$::_internal_set_$name$($Enum$ value) {
-        $assert_valid$;
-        if ($not_has_field$) {
-          clear_$oneof_name$();
-          set_has_$name$();
-        }
-        $field_$ = value;
-      }
     )cc");
   } else {
     p->Emit(R"cc(

src/google/protobuf/compiler/cpp/field_generators/message_field.cc

@@ -322,7 +322,7 @@ void SingularMessage::GenerateInternalAccessorDefinitions(
   // practice, the linker is then not able to throw them out making implicit
   // weak dependencies not work at all.
 
-  if (!is_weak()) return;
+  if (!is_weak() || is_oneof()) return;
 
   // These private accessors are used by MergeFrom and
   // MergePartialFromCodedStream, and their purpose is to provide access to
@@ -336,28 +336,11 @@ void SingularMessage::GenerateInternalAccessorDefinitions(
                msg->$set_hasbit$;
              )cc");
            }},
-          {"is_already_set",
-           [&] {
-             if (!is_oneof()) {
-               p->Emit("msg->$field_$ == nullptr");
-             } else {
-               p->Emit("msg->$not_has_field$");
-             }
-           }},
-          {"clear_oneof",
-           [&] {
-             if (!is_oneof()) return;
-             p->Emit(R"cc(
-               msg->clear_$oneof_name$();
-               msg->set_has_$name$();
-             )cc");
-           }},
       },
       R"cc(
         $pb$::MessageLite* $Msg$::_Internal::mutable_$name$($Msg$* msg) {
           $update_hasbit$;
-          if ($is_already_set$) {
+          if (msg->$field_$ == nullptr) {
-            $clear_oneof$;
             msg->$field_$ = $kDefaultPtr$->New(msg->GetArena());
           }
           return msg->$field_$;
@@ -398,7 +381,7 @@ void SingularMessage::GenerateMessageClearingCode(io::Printer* p) const {
 bool SingularMessage::RequiresArena(GeneratorFunction function) const {
   switch (function) {
     case GeneratorFunction::kMergeFrom:
-      return !(is_weak() || is_oneof() || should_split());
+      return !(is_weak() || should_split());
   }
   return false;
 }
@@ -408,7 +391,7 @@ void SingularMessage::GenerateMergingCode(io::Printer* p) const {
     p->Emit(
         "_Internal::mutable_$name$(_this)->CheckTypeAndMergeFrom(\n"
         "    *from.$field_$);\n");
-  } else if (is_oneof() || should_split()) {
+  } else if (should_split()) {
     p->Emit(
         "_this->_internal_mutable_$name$()->$Submsg$::MergeFrom(\n"
         "    from._internal_$name$());\n");
@@ -548,6 +531,8 @@ class OneofMessage : public SingularMessage {
   void GenerateDestructorCode(io::Printer* p) const override;
   void GenerateConstructorCode(io::Printer* p) const override;
   void GenerateIsInitialized(io::Printer* p) const override;
+  void GenerateMergingCode(io::Printer* p) const override;
+  bool RequiresArena(GeneratorFunction func) const override;
 };
 
 void OneofMessage::GenerateNonInlineAccessorDefinitions(io::Printer* p) const {
@@ -689,6 +674,34 @@ void OneofMessage::GenerateIsInitialized(io::Printer* p) const {
   )cc");
 }
 
+void OneofMessage::GenerateMergingCode(io::Printer* p) const {
+  if (is_weak()) {
+    p->Emit(R"cc(
+      if (oneof_needs_init) {
+        _this->$field_$ = from.$field_$->New(arena);
+      }
+      _this->$field_$->CheckTypeAndMergeFrom(*from.$field_$);
+    )cc");
+  } else {
+    p->Emit(R"cc(
+      if (oneof_needs_init) {
+        _this->$field_$ =
+            $superclass$::CopyConstruct<$Submsg$>(arena, *from.$field_$);
+      } else {
+        _this->$field_$->MergeFrom(from._internal_$name$());
+      }
+    )cc");
+  }
+}
+
+bool OneofMessage::RequiresArena(GeneratorFunction func) const {
+  switch (func) {
+    case GeneratorFunction::kMergeFrom:
+      return true;
+  }
+  return false;
+}
+
 class RepeatedMessage : public FieldGeneratorBase {
  public:
   RepeatedMessage(const FieldDescriptor* field, const Options& opts,

src/google/protobuf/compiler/cpp/field_generators/primitive_field.cc

@@ -108,7 +108,7 @@ class SingularPrimitive final : public FieldGeneratorBase {
 
   void GenerateMergingCode(io::Printer* p) const override {
     p->Emit(R"cc(
-      _this->_internal_set_$name$(from._internal_$name$());
+      _this->$field_$ = from.$field_$;
     )cc");
   }
 
@@ -198,7 +198,11 @@ void SingularPrimitive::GenerateInlineAccessorDefinitions(
     p->Emit(R"cc(
       inline void $Msg$::set_$name$($Type$ value) {
         $PrepareSplitMessageForWrite$;
-        _internal_set_$name$(value);
+        if ($not_has_field$) {
+          clear_$oneof_name$();
+          set_has_$name$();
+        }
+        $field_$ = value;
         $annotate_set$;
         // @@protoc_insertion_point(field_set:$pkg.Msg.field$)
       }
@@ -208,13 +212,6 @@ void SingularPrimitive::GenerateInlineAccessorDefinitions(
         }
         return $kDefault$;
       }
-      inline void $Msg$::_internal_set_$name$($Type$ value) {
-        if ($not_has_field$) {
-          clear_$oneof_name$();
-          set_has_$name$();
-        }
-        $field_$ = value;
-      }
     )cc");
   } else {
     p->Emit(R"cc(

src/google/protobuf/compiler/cpp/field_generators/string_field.cc

@@ -88,11 +88,28 @@ class SingularString : public FieldGeneratorBase {
             )cc");
   }
 
+  bool RequiresArena(GeneratorFunction function) const override {
+    switch (function) {
+      case GeneratorFunction::kMergeFrom:
+        return is_oneof();
+    }
+    return false;
+  }
+
   void GenerateMergingCode(io::Printer* p) const override {
+    if (is_oneof()) {
+      p->Emit(R"cc(
+        if (oneof_needs_init) {
+          _this->$field_$.InitDefault();
+        }
+        _this->$field_$.Set(from._internal_$name$(), arena);
+      )cc");
+    } else {
       p->Emit(R"cc(
         _this->_internal_set_$name$(from._internal_$name$());
       )cc");
     }
+  }
 
   void GenerateArenaDestructorCode(io::Printer* p) const override {
     if (!is_inlined()) return;

src/google/protobuf/compiler/cpp/message.cc

@@ -3771,23 +3771,44 @@ void MessageGenerator::GenerateClassSpecificMergeImpl(io::Printer* p) {
 
   // Merge oneof fields. Oneof field requires oneof case check.
   for (auto oneof : OneOfRange(descriptor_)) {
-    format("switch (from.$1$_case()) {\n", oneof->name());
+    p->Emit({{"name", oneof->name()},
-    format.Indent();
+             {"NAME", absl::AsciiStrToUpper(oneof->name())},
-    for (auto field : FieldRange(oneof)) {
+             {"index", oneof->index()},
-      format("case k$1$: {\n", UnderscoresToCamelCase(field->name(), true));
+             {"cases",
-      format.Indent();
+              [&] {
+                for (const auto* field : FieldRange(oneof)) {
+                  p->Emit(
+                      {{"Label", UnderscoresToCamelCase(field->name(), true)},
+                       {"body",
+                        [&] {
                           field_generators_.get(field).GenerateMergingCode(p);
-      format("break;\n");
+                        }}},
-      format.Outdent();
+                      R"cc(
-      format("}\n");
+                        case k$Label$: {
+                          $body$;
+                          break;
                         }
-    format(
+                      )cc");
-        "case $1$_NOT_SET: {\n"
+                }
-        "  break;\n"
+              }}},
-        "}\n",
+            R"cc(
-        absl::AsciiStrToUpper(oneof->name()));
+              if (const uint32_t oneof_from_case = from.$oneof_case$[$index$]) {
-    format.Outdent();
+                const uint32_t oneof_to_case = _this->$oneof_case$[$index$];
-    format("}\n");
+                const bool oneof_needs_init = oneof_to_case != oneof_from_case;
+                if (oneof_needs_init) {
+                  if (oneof_to_case != 0) {
+                    _this->clear_$name$();
+                  }
+                  _this->$oneof_case$[$index$] = oneof_from_case;
+                }
+
+                switch (oneof_from_case) {
+                  $cases$;
+                  case $NAME$_NOT_SET:
+                    break;
+                }
+              }
+            )cc");
   }
   if (num_weak_fields_) {
     format(

src/google/protobuf/cpp_features.pb.cc

@@ -270,7 +270,7 @@ void CppFeatures::MergeImpl(::google::protobuf::MessageLite& to_msg, const ::goo
 
   cached_has_bits = from._impl_._has_bits_[0];
   if (cached_has_bits & 0x00000001u) {
-    _this->_internal_set_legacy_closed_enum(from._internal_legacy_closed_enum());
+    _this->_impl_.legacy_closed_enum_ = from._impl_.legacy_closed_enum_;
   }
   _this->_impl_._has_bits_[0] |= cached_has_bits;
   _this->_internal_metadata_.MergeFrom<::google::protobuf::UnknownFieldSet>(from._internal_metadata_);