Merge pull request #23899 from protocolbuffers/33rc2-cp

Backport fixes to 33.x

Author: mkruskal-google

java/core/src/main/java/com/google/protobuf/Descriptors.java

@@ -495,6 +495,45 @@ private static FileDescriptor[] findDescriptors(
       return descriptors.toArray(new FileDescriptor[0]);
     }
 
+    /**
+     * This method is for backward compatibility with generated code which passed an
+     * InternalDescriptorAssigner.
+     *
+     * @deprecated Any gencode which is using this method is far out of support and should be
+     *     regenerated with a newer version of the protobuf compiler.
+     */
+    @Deprecated
+    public static void internalBuildGeneratedFileFrom(
+        final String[] descriptorDataParts,
+        final FileDescriptor[] dependencies,
+        final InternalDescriptorAssigner descriptorAssigner) {
+      final byte[] descriptorBytes = latin1Cat(descriptorDataParts);
+
+      FileDescriptorProto proto;
+      try {
+        proto = FileDescriptorProto.parseFrom(descriptorBytes);
+      } catch (InvalidProtocolBufferException e) {
+        throw new IllegalArgumentException(
+            "Failed to parse protocol buffer descriptor for generated code.", e);
+      }
+
+      final FileDescriptor result;
+      try {
+        // When building descriptors for generated code, we allow unknown
+        // dependencies by default.
+        result = buildFrom(proto, dependencies, true);
+      } catch (DescriptorValidationException e) {
+        throw new IllegalArgumentException(
+            "Invalid embedded descriptor for \"" + proto.getName() + "\".", e);
+      }
+
+      final ExtensionRegistry registry = descriptorAssigner.assignDescriptors(result);
+
+      if (registry != null) {
+        throw new RuntimeException("assignDescriptors must return null");
+      }
+    }
+
     /**
      * This method is to be called by generated code only. It is equivalent to {@code buildFrom}
      * except that the {@code FileDescriptorProto} is encoded in protocol buffer wire format.

src/google/protobuf/arena.h

@@ -124,6 +124,16 @@ constexpr bool FieldHasArenaOffset() {
   return !std::is_same_v<T, ArenaRepT>;
 }
 
+// TODO - Some types have a deprecated arena-enabled constructor,
+// as we plan to remove it in favor of using arena offsets, but for now Arena
+// needs to call it. While the arena constructor exists, we will call the
+// `InternalVisibility` override to silence the warning.
+template <typename T>
+constexpr bool HasDeprecatedArenaConstructor() {
+  return std::is_base_of_v<internal::RepeatedPtrFieldBase, T> &&
+         !std::is_same_v<T, internal::RepeatedPtrFieldBase>;
+}
+
 template <typename T>
 void arena_delete_object(void* PROTOBUF_NONNULL object) {
   delete reinterpret_cast<T*>(object);
@@ -478,16 +488,12 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
     static T* PROTOBUF_NONNULL ConstructOnArena(void* PROTOBUF_NONNULL ptr,
                                                 Arena& arena, Args&&... args) {
       // TODO - ClangTidy gives warnings for calling the deprecated
-      // `RepeatedPtrField(Arena*)` constructor here, but this is the correct
-      // way to call it as it will allow us to silently switch to a different
-      // constructor once arena pointers are removed from RepeatedPtrFields.
-      // While this constructor exists, we will call the `InternalVisibility`
-      // override to silence the warning.
-      //
-      // Note: RepeatedPtrFieldBase is sometimes constructed internally, and it
-      // doesn't have `InternalVisibility` constructors.
-      if constexpr (std::is_base_of_v<internal::RepeatedPtrFieldBase, T> &&
-                    !std::is_same_v<T, internal::RepeatedPtrFieldBase>) {
+      // constructors here, which leads to log spam. It is correct to invoke
+      // these constructors through the Arena class as it will allow us to
+      // silently switch to a different constructor once arena pointers are
+      // removed. While these constructors exists, we will call the
+      // `InternalVisibility` overrides to silence the warning.
+      if constexpr (internal::HasDeprecatedArenaConstructor<T>()) {
         return new (ptr) T(internal::InternalVisibility(), &arena,
                            static_cast<Args&&>(args)...);
       } else {
@@ -510,7 +516,8 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
       // Fields which use arena offsets don't have constructors that take an
       // arena pointer. Since the arena is nullptr, it is safe to default
       // construct the object.
-      if constexpr (internal::FieldHasArenaOffset<T>()) {
+      if constexpr (internal::FieldHasArenaOffset<T>() ||
+                    internal::HasDeprecatedArenaConstructor<T>()) {
         return new T();
       } else {
         return new T(nullptr);
@@ -590,7 +597,8 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
     static_assert(is_arena_constructable<T>::value,
                   "Can only construct types that are ArenaConstructable");
     if (ABSL_PREDICT_FALSE(arena == nullptr)) {
-      if constexpr (internal::FieldHasArenaOffset<T>()) {
+      if constexpr (internal::FieldHasArenaOffset<T>() ||
+                    internal::HasDeprecatedArenaConstructor<T>()) {
         return new T(static_cast<Args&&>(args)...);
       } else {
         return new T(nullptr, static_cast<Args&&>(args)...);
@@ -744,7 +752,12 @@ Arena::DefaultConstruct(Arena* PROTOBUF_NULLABLE arena) {
   static_assert(is_destructor_skippable<T>::value, "");
   void* mem = arena != nullptr ? arena->AllocateAligned(sizeof(T))
                                : ::operator new(sizeof(T));
-  return new (mem) T(arena);
+
+  if constexpr (internal::HasDeprecatedArenaConstructor<T>()) {
+    return new (mem) T(internal::InternalVisibility(), arena);
+  } else {
+    return new (mem) T(arena);
+  }
 }
 
 template <typename T>

src/google/protobuf/repeated_ptr_field.h

@@ -1022,7 +1022,7 @@ class ABSL_ATTRIBUTE_WARN_UNUSED RepeatedPtrField final
 
   // Arena enabled constructors: for internal use only.
   RepeatedPtrField(internal::InternalVisibility, Arena* arena)
-      : RepeatedPtrField(arena) {}
+      : RepeatedPtrFieldBase(arena) {}
   RepeatedPtrField(internal::InternalVisibility, Arena* arena,
                    const RepeatedPtrField& rhs)
       : RepeatedPtrField(arena, rhs) {}