Add a macro to make Map(Arena*) constructor private in a future release.

ClaytonKnittel · mkruskal-google · commit 543a17f6553a · 2025-10-07T11:38:27.000-07:00
PiperOrigin-RevId: 815140361
diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h
@@ -1036,10 +1036,6 @@ class Map : private internal::KeyMapBase<internal::KeyForBase<Key>> {
   Map(const Map& other) : Map(nullptr, other) {}
 
   // Internal Arena constructors: do not use!
-  // TODO: remove non internal ctors
-  explicit Map(Arena* arena) : Base(arena, GetTypeInfo()) {
-    StaticValidityCheck();
-  }
   Map(internal::InternalVisibility, Arena* arena) : Map(arena) {}
   Map(internal::InternalVisibility, Arena* arena, const Map& other)
       : Map(arena, other) {}
@@ -1077,7 +1073,22 @@ class Map : private internal::KeyMapBase<internal::KeyForBase<Key>> {
     this->ClearTable(false);
   }
 
+  // If PROTOBUF_FUTURE_REMOVE_MAP_FIELD_ARENA_CONSTRUCTOR is defined, make the
+  // arena-enabled constructor private.
+#ifdef PROTOBUF_FUTURE_REMOVE_MAP_FIELD_ARENA_CONSTRUCTOR
+
+ private:
+#endif
+  explicit Map(Arena* arena) : Base(arena, GetTypeInfo()) {
+    StaticValidityCheck();
+  }
+
+  // If PROTOBUF_FUTURE_REMOVE_MAP_FIELD_ARENA_CONSTRUCTOR was not defined, we
+  // need to add the private tag here.
+#ifndef PROTOBUF_FUTURE_REMOVE_MAP_FIELD_ARENA_CONSTRUCTOR
+
  private:
+#endif
   Map(Arena* arena, const Map& other) : Map(arena) {
     StaticValidityCheck();
     CopyFromImpl(other);
diff --git a/src/google/protobuf/map_test.inc b/src/google/protobuf/map_test.inc
@@ -244,21 +244,25 @@ namespace {
 TEST_F(MapImplTest, OperatorBracketRValue) {
   Arena arena;
   for (Arena* arena_to_use : {&arena, static_cast<Arena*>(nullptr)}) {
-    Map<std::string, int> map(arena_to_use);
+    auto* map = Arena::Create<Map<std::string, int32_t>>(arena_to_use);
     {
       std::string key(10000, 'a');
       const char* const data = key.data();
-      map[key] = 0;
+      (*map)[key] = 0;
       EXPECT_EQ(data, key.data());
-      EXPECT_NE(data, map.find(std::string(10000, 'a'))->first.data());
+      EXPECT_NE(data, map->find(std::string(10000, 'a'))->first.data());
     }
 
     {
       std::string key(10000, 'b');
       const char* const data = key.data();
-      map[std::move(key)] = 0;
+      (*map)[std::move(key)] = 0;
       EXPECT_NE(data, key.data());  // NOLINT
-      EXPECT_EQ(data, map.find(std::string(10000, 'b'))->first.data());
+      EXPECT_EQ(data, map->find(std::string(10000, 'b'))->first.data());
+    }
+
+    if (arena_to_use == nullptr) {
+      delete map;
     }
   }
 }
@@ -1032,8 +1036,10 @@ TEST_F(MapImplTest, SwapBasic) {
 
 TEST_F(MapImplTest, SwapArena) {
   Arena arena1, arena2;
-  Map<int32_t, int32_t> m1(&arena1);
-  Map<int32_t, int32_t> m2(&arena2);
+  auto* m1_ptr = Arena::Create<Map<int32_t, int32_t>>(&arena1);
+  auto* m2_ptr = Arena::Create<Map<int32_t, int32_t>>(&arena2);
+  auto& m1 = *m1_ptr;
+  auto& m2 = *m2_ptr;
   map_[9398] = 41999;
   m1[9398] = 41999;
   m1[8070] = 42056;
diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc
@@ -165,6 +165,10 @@ static_assert(PROTOBUF_ABSL_MIN(20230125, 3),
 // Owner: cknittel@, mkruskal@
 #define PROTOBUF_FUTURE_REMOVE_REPEATED_PTR_FIELD_ARENA_CONSTRUCTOR 1
 
+// Removes the public Map(Arena*) constructor.
+// Owner: cknittel@, mkruskal@
+#define PROTOBUF_FUTURE_REMOVE_MAP_FIELD_ARENA_CONSTRUCTOR 1
+
 #else
 
 #define PROTOBUF_FUTURE_ADD_NODISCARD
diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc
@@ -76,6 +76,7 @@
 #undef PROTOBUF_FUTURE_ADD_NODISCARD
 #undef PROTOBUF_FUTURE_NO_RECURSIVE_MESSAGE_COPY
 #undef PROTOBUF_FUTURE_REMOVE_REPEATED_PTR_FIELD_ARENA_CONSTRUCTOR
+#undef PROTOBUF_FUTURE_REMOVE_MAP_FIELD_ARENA_CONSTRUCTOR
 #endif
 
 #include "google/protobuf/os_macros_restore.inc"
