Have Arena::Create support arena constructible types

Unlike Arena::CreateMessage, Arena::Create creates only the top level object
from arena even if it is arena constructalble; e.g. messages, RepeatedPtrField,
etc. This renders arenas less effective.

Instead of asking users to be aware of such nuances to use the right API for
the right type, this CL makes Arena::Create recognizes and fully supports arena
constructable types.

While extremly rare, some users try to emulate Arena::CreateMessage with
Arena::Create by passing arena parameter twice. For example,

```
auto foo = Arena::Create<Foo>(&arena, &arena);  // bad
```

This pattern is not supported and  will break after this change. The following
is recommended instead.

```
auto foo = Arena::CreateMessage<Foo>(&arena);  // recommended
auto foo = Arena::Create<Foo>(&arena);  // after this change
```

PiperOrigin-RevId: 585709990

cmake/abseil-cpp.cmake

@@ -72,6 +72,7 @@ else()
     absl::flat_hash_set
     absl::function_ref
     absl::hash
+    absl::if_constexpr
     absl::layout
     absl::log_initialize
     absl::log_severity

src/google/protobuf/BUILD.bazel

@@ -338,6 +338,7 @@ cc_library(
         "@com_google_absl//absl/log:absl_check",
         "@com_google_absl//absl/log:absl_log",
         "@com_google_absl//absl/synchronization",
+        "@com_google_absl//absl/utility:if_constexpr",
     ],
 )
 

src/google/protobuf/arena.h

@@ -10,7 +10,10 @@
 #ifndef GOOGLE_PROTOBUF_ARENA_H__
 #define GOOGLE_PROTOBUF_ARENA_H__
 
+#include <cstddef>
+#include <cstdint>
 #include <limits>
+#include <new>
 #include <string>
 #include <type_traits>
 #include <utility>
@@ -26,8 +29,11 @@ using type_info = ::type_info;
 #include <typeinfo>
 #endif
 
+#include "absl/base/attributes.h"
 #include "absl/log/absl_check.h"
+#include "absl/utility/internal/if_constexpr.h"
 #include "google/protobuf/arena_align.h"
+#include "google/protobuf/arena_allocation_policy.h"
 #include "google/protobuf/port.h"
 #include "google/protobuf/serial_arena.h"
 #include "google/protobuf/thread_safe_arena.h"
@@ -48,6 +54,9 @@ class Message;  // defined in message.h
 class MessageLite;
 template <typename Key, typename T>
 class Map;
+namespace internal {
+struct RepeatedFieldBase;
+}  // namespace internal
 
 namespace arena_metrics {
 
@@ -210,7 +219,7 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
       internal::ThreadSafeArena::kBlockHeaderSize +
       internal::ThreadSafeArena::kSerialArenaSize;
 
-  inline ~Arena() {}
+  inline ~Arena() = default;
 
   // API to create proto2 message objects on the arena. If the arena passed in
   // is nullptr, then a heap allocated object is returned. Type T must be a
@@ -243,27 +252,31 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
     return CreateMessageInternal<Type>(arena, std::forward<Args>(args)...);
   }
 
-  // API to create any objects on the arena. Note that only the object will
+  // API to create any objects on the arena.
-  // be created on the arena; the underlying ptrs (in case of a proto2 message)
+  //
-  // will be still heap allocated. Proto messages should usually be allocated
+  // If an object is arena-constructable, this API behaves like
-  // with CreateMessage<T>() instead.
+  // Arena::CreateMessage. Arena constructable types are expected to accept
+  // Arena* as the first argument and one is implicitly added by the API.
   //
-  // Note that even if T satisfies the arena message construction protocol
+  // If the object is not arena-constructable, only the object will be created
-  // (InternalArenaConstructable_ trait and optional DestructorSkippable_
+  // on the arena; the underlying ptrs will be still heap allocated.
-  // trait), as described above, this function does not follow the protocol;
-  // instead, it treats T as a black-box type, just as if it did not have these
-  // traits. Specifically, T's constructor arguments will always be only those
-  // passed to Create<T>() -- no additional arena pointer is implicitly added.
-  // Furthermore, the destructor will always be called at arena destruction time
-  // (unless the destructor is trivial). Hence, from T's point of view, it is as
-  // if the object were allocated on the heap (except that the underlying memory
-  // is obtained from the arena).
   template <typename T, typename... Args>
   PROTOBUF_NDEBUG_INLINE static T* Create(Arena* arena, Args&&... args) {
-    if (PROTOBUF_PREDICT_FALSE(arena == nullptr)) {
+    return absl::utility_internal::IfConstexprElse<
-      return new T(std::forward<Args>(args)...);
+        is_arena_constructable<T>::value>(
-    }
+        // Arena-constructable
-    return new (arena->AllocateInternal<T>()) T(std::forward<Args>(args)...);
+        [arena](auto&&... args) {
+          return CreateMessage<T>(arena, std::forward<Args>(args)...);
+        },
+        // Non arena-constructable
+        [arena](auto&&... args) {
+          if (PROTOBUF_PREDICT_FALSE(arena == nullptr)) {
+            return new T(std::forward<Args>(args)...);
+          }
+          return new (arena->AllocateInternal<T>())
+              T(std::forward<Args>(args)...);
+        },
+        std::forward<Args>(args)...);
   }
 
   // API to delete any objects not on an arena.  This can be used to safely

src/google/protobuf/arena_unittest.cc

@@ -450,6 +450,29 @@ DispatcherTestProto* Arena::CreateMessageInternal<DispatcherTestProto, int>(
   return nullptr;
 }
 
+TEST(ArenaTest, CreateArenaConstructable) {
+  TestAllTypes original;
+  TestUtil::SetAllFields(&original);
+
+  Arena arena;
+  auto copied = Arena::Create<TestAllTypes>(&arena, original);
+
+  TestUtil::ExpectAllFieldsSet(*copied);
+  EXPECT_EQ(copied->GetArena(), &arena);
+  EXPECT_EQ(copied->optional_nested_message().GetArena(), &arena);
+}
+
+TEST(ArenaTest, CreateRepeatedPtrField) {
+  Arena arena;
+  auto repeated = Arena::Create<RepeatedPtrField<TestAllTypes>>(&arena);
+  TestUtil::SetAllFields(repeated->Add());
+
+  TestUtil::ExpectAllFieldsSet(repeated->Get(0));
+  EXPECT_EQ(repeated->GetArena(), &arena);
+  EXPECT_EQ(repeated->Get(0).GetArena(), &arena);
+  EXPECT_EQ(repeated->Get(0).optional_nested_message().GetArena(), &arena);
+}
+
 TEST(ArenaTest, CreateMessageDispatchesToSpecialFunctions) {
   hook_called = "";
   Arena::CreateMessage<DispatcherTestProto>(nullptr);

src/google/protobuf/proto3_arena_unittest.cc

@@ -152,7 +152,7 @@ TEST(Proto3ArenaTest, GetArena) {
 
   // Tests message created by Arena::Create.
   auto* arena_message3 = Arena::Create<TestAllTypes>(&arena);
-  EXPECT_EQ(nullptr, arena_message3->GetArena());
+  EXPECT_EQ(&arena, arena_message3->GetArena());
 }
 
 TEST(Proto3ArenaTest, GetArenaWithUnknown) {