Add tests for older gcc versions we still support (#20463)

PiperOrigin-RevId: 723691910 Co-authored-by: Mike Kruskal <mkruskal@google.com>
protocolbuffers · Feb 25, 2025 · 0778473 · 0778473
1 parent 503abcc
commit 0778473
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 43 deletions.
diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml
@@ -74,7 +74,7 @@ jobs:
     strategy:
       fail-fast: false   # Don't cancel all jobs if one fails.
       matrix:
-        version: ['9.5', '13.1']
+        version: ['7.5', '9.1', '9.5', '13.1']
     name: Linux GCC ${{ matrix.version }}
     runs-on: ubuntu-latest
     steps:
@@ -85,7 +85,7 @@ jobs:
       - name: Run tests
         uses: protocolbuffers/protobuf-ci/bazel-docker@v4
         with:
-          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-${{ matrix.version }}-d9624f2aa83cba3eaf906f751d75b36aacb9aa82
+          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-${{ matrix.version }}-e78301df86b3e4c46ec9ac4d98be00e19305d8f3
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: cpp_linux/gcc-${{ matrix.version }}
           bazel: test //pkg/... //src/... //third_party/utf8_range/... //conformance:conformance_framework_tests
@@ -352,7 +352,7 @@ jobs:
         if: ${{ !matrix.continuous-only || inputs.continuous-run }}
         uses: protocolbuffers/protobuf-ci/docker@v4
         with:
-          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-12.2-d9624f2aa83cba3eaf906f751d75b36aacb9aa82
+          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-12.2-e78301df86b3e4c46ec9ac4d98be00e19305d8f3
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           entrypoint: bash
           command: >-

diff --git a/.github/workflows/test_upb.yml b/.github/workflows/test_upb.yml
@@ -69,7 +69,7 @@ jobs:
       - name: Run tests
         uses: protocolbuffers/protobuf-ci/bazel-docker@v4
         with:
-          image: "us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-12.2-d9624f2aa83cba3eaf906f751d75b36aacb9aa82"
+          image: "us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-12.2-e78301df86b3e4c46ec9ac4d98be00e19305d8f3"
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: "upb-bazel-gcc"
           bazel: >-

diff --git a/src/google/protobuf/generated_message_tctable_lite.cc b/src/google/protobuf/generated_message_tctable_lite.cc
@@ -2717,11 +2717,11 @@ const char* TcParser::ParseOneMapEntry(
     void* obj;
     if (inner_tag == key_tag) {
       type_card = map_info.key_type_card;
-      type_kind = map.type_info().key_type;
+      type_kind = map.type_info().key_type_kind();
       obj = node->GetVoidKey();
     } else {
       type_card = map_info.value_type_card;
-      type_kind = map.type_info().value_type;
+      type_kind = map.type_info().value_type_kind();
       obj = map.GetVoidValue(node);
     }
 
@@ -2870,7 +2870,7 @@ PROTOBUF_NOINLINE const char* TcParser::MpMap(PROTOBUF_TC_PARAM_DECL) {
       WriteMapEntryAsUnknown(msg, table, map, saved_tag, node, map_info);
     } else {
       // Done parsing the node, insert it.
-      switch (map.type_info().key_type) {
+      switch (map.type_info().key_type_kind()) {
         case UntypedMapBase::TypeKind::kBool:
           static_cast<KeyMapBase<bool>&>(map).InsertOrReplaceNode(
               static_cast<KeyMapBase<bool>::KeyNode*>(node));

diff --git a/src/google/protobuf/map.cc b/src/google/protobuf/map.cc
@@ -133,9 +133,9 @@ void UntypedMapBase::ClearTableImpl(bool reset) {
     };
 
     const auto dispatch_key = [&](auto value_handler) {
-      if (type_info_.key_type < TypeKind::kString) {
+      if (type_info_.key_type_kind() < TypeKind::kString) {
         loop(value_handler);
-      } else if (type_info_.key_type == TypeKind::kString) {
+      } else if (type_info_.key_type_kind() == TypeKind::kString) {
         loop([=](NodeBase* node) {
           static_cast<std::string*>(node->GetVoidKey())->~basic_string();
           value_handler(node);
@@ -145,13 +145,13 @@ void UntypedMapBase::ClearTableImpl(bool reset) {
       }
     };
 
-    if (type_info_.value_type < TypeKind::kString) {
+    if (type_info_.value_type_kind() < TypeKind::kString) {
       dispatch_key([](NodeBase*) {});
-    } else if (type_info_.value_type == TypeKind::kString) {
+    } else if (type_info_.value_type_kind() == TypeKind::kString) {
       dispatch_key([&](NodeBase* node) {
         GetValue<std::string>(node)->~basic_string();
       });
-    } else if (type_info_.value_type == TypeKind::kMessage) {
+    } else if (type_info_.value_type_kind() == TypeKind::kMessage) {
       dispatch_key([&](NodeBase* node) {
         GetValue<MessageLite>(node)->DestroyInstance();
       });
@@ -251,7 +251,8 @@ UntypedMapBase::TypeInfo UntypedMapBase::GetTypeInfoDynamic(
       key_offsets.end, value_type, value_prototype_if_message, max_align);
   return TypeInfo{
       Narrow<uint16_t>(AlignTo(value_offsets.end, max_align, max_align)),
-      Narrow<uint8_t>(value_offsets.start), key_type, value_type};
+      Narrow<uint8_t>(value_offsets.start), static_cast<uint8_t>(key_type),
+      static_cast<uint8_t>(value_type)};
 }
 
 }  // namespace internal

diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h
@@ -294,8 +294,13 @@ class PROTOBUF_EXPORT UntypedMapBase {
     uint16_t node_size;
     // Equivalent to `offsetof(Node, kv.second)` in the derived type.
     uint8_t value_offset;
-    TypeKind key_type : 4;
-    TypeKind value_type : 4;
+    uint8_t key_type : 4;
+    uint8_t value_type : 4;
+
+    TypeKind key_type_kind() const { return static_cast<TypeKind>(key_type); }
+    TypeKind value_type_kind() const {
+      return static_cast<TypeKind>(value_type);
+    }
   };
   static_assert(sizeof(TypeInfo) == 4);
 
@@ -474,7 +479,7 @@ class PROTOBUF_EXPORT UntypedMapBase {
 
 template <typename F>
 auto UntypedMapBase::VisitKeyType(F f) const {
-  switch (type_info_.key_type) {
+  switch (type_info_.key_type_kind()) {
     case TypeKind::kBool:
       return f(std::enable_if<true, bool>{});
     case TypeKind::kU32:
@@ -494,7 +499,7 @@ auto UntypedMapBase::VisitKeyType(F f) const {
 
 template <typename F>
 auto UntypedMapBase::VisitValueType(F f) const {
-  switch (type_info_.value_type) {
+  switch (type_info_.value_type_kind()) {
     case TypeKind::kBool:
       return f(std::enable_if<true, bool>{});
     case TypeKind::kU32:
@@ -1446,8 +1451,8 @@ class Map : private internal::KeyMapBase<internal::KeyForBase<Key>> {
     return internal::UntypedMapBase::TypeInfo{
         sizeof(Node),
         PROTOBUF_FIELD_OFFSET(Node, kv.second),
-        internal::UntypedMapBase::StaticTypeKind<Key>(),
-        internal::UntypedMapBase::StaticTypeKind<T>(),
+        static_cast<uint8_t>(internal::UntypedMapBase::StaticTypeKind<Key>()),
+        static_cast<uint8_t>(internal::UntypedMapBase::StaticTypeKind<T>()),
     };
   }
 

diff --git a/src/google/protobuf/map_test.cc b/src/google/protobuf/map_test.cc
@@ -309,6 +309,32 @@ TEST(MapTest, SizeTypeIsSizeT) {
   (void)x;
 }
 
+using KeyTypes = void (*)(bool, int32_t, uint32_t, int64_t, uint64_t,
+                          std::string);
+// Some arbitrary proto enum.
+using SomeEnum = proto2_unittest::TestAllTypes::NestedEnum;
+using ValueTypes = void (*)(bool, int32_t, uint32_t, int64_t, uint64_t, float,
+                            double, std::string, SomeEnum,
+                            proto2_unittest::TestEmptyMessage,
+                            proto2_unittest::TestAllTypes);
+
+TEST(MapTest, StaticTypeKindWorks) {
+  using UMB = UntypedMapBase;
+  EXPECT_EQ(UMB::TypeKind::kBool, UMB::StaticTypeKind<bool>());
+  EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<int32_t>());
+  EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<uint32_t>());
+  EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<SomeEnum>());
+  EXPECT_EQ(UMB::TypeKind::kU64, UMB::StaticTypeKind<int64_t>());
+  EXPECT_EQ(UMB::TypeKind::kU64, UMB::StaticTypeKind<uint64_t>());
+  EXPECT_EQ(UMB::TypeKind::kString, UMB::StaticTypeKind<std::string>());
+  EXPECT_EQ(UMB::TypeKind::kMessage,
+            UMB::StaticTypeKind<proto2_unittest::TestAllTypes>());
+}
+
+#if !defined(__GNUC__) || defined(__clang__) || PROTOBUF_GNUC_MIN(9, 4)
+// Parameter pack expansion bug before GCC 8.2:
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85305
+
 template <typename F, typename... Key, typename... Value>
 void TestAllKeyValueTypes(void (*)(Key...), void (*)(Value...), F f) {
   (
@@ -319,15 +345,6 @@ void TestAllKeyValueTypes(void (*)(Key...), void (*)(Value...), F f) {
       ...);
 }
 
-using KeyTypes = void (*)(bool, int32_t, uint32_t, int64_t, uint64_t,
-                          std::string);
-// Some arbitrary proto enum.
-using SomeEnum = proto2_unittest::TestAllTypes::NestedEnum;
-using ValueTypes = void (*)(bool, int32_t, uint32_t, int64_t, uint64_t, float,
-                            double, std::string, SomeEnum,
-                            proto2_unittest::TestEmptyMessage,
-                            proto2_unittest::TestAllTypes);
-
 TEST(MapTest, StaticTypeInfoMatchesDynamicOne) {
   TestAllKeyValueTypes(KeyTypes(), ValueTypes(), [](auto key, auto value) {
     using Key = decltype(key);
@@ -338,27 +355,15 @@ TEST(MapTest, StaticTypeInfoMatchesDynamicOne) {
     }
     const auto type_info = MapTestPeer::GetTypeInfo<Map<Key, Value>>();
     const auto dyn_type_info = internal::UntypedMapBase::GetTypeInfoDynamic(
-        type_info.key_type, type_info.value_type, value_prototype);
+        type_info.key_type_kind(), type_info.value_type_kind(),
+        value_prototype);
     EXPECT_EQ(dyn_type_info.node_size, type_info.node_size);
     EXPECT_EQ(dyn_type_info.value_offset, type_info.value_offset);
     EXPECT_EQ(dyn_type_info.key_type, type_info.key_type);
     EXPECT_EQ(dyn_type_info.value_type, type_info.value_type);
   });
 }
 
-TEST(MapTest, StaticTypeKindWorks) {
-  using UMB = UntypedMapBase;
-  EXPECT_EQ(UMB::TypeKind::kBool, UMB::StaticTypeKind<bool>());
-  EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<int32_t>());
-  EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<uint32_t>());
-  EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<SomeEnum>());
-  EXPECT_EQ(UMB::TypeKind::kU64, UMB::StaticTypeKind<int64_t>());
-  EXPECT_EQ(UMB::TypeKind::kU64, UMB::StaticTypeKind<uint64_t>());
-  EXPECT_EQ(UMB::TypeKind::kString, UMB::StaticTypeKind<std::string>());
-  EXPECT_EQ(UMB::TypeKind::kMessage,
-            UMB::StaticTypeKind<proto2_unittest::TestAllTypes>());
-}
-
 template <typename LHS, typename RHS>
 void TestEqPtr(LHS* lhs, RHS* rhs) {
   // To silence some false positive compiler errors in gcc 9.5
@@ -428,6 +433,8 @@ TEST(MapTest, VisitAllNodesUsesTheRightTypesOnAllNodes) {
   });
 }
 
+#endif
+
 TEST(MapTest, IteratorNodeFieldIsNullPtrAtEnd) {
   Map<int, int> map;
   EXPECT_EQ(internal::UntypedMapIterator::FromTyped(map.cbegin()).node_,

diff --git a/src/google/protobuf/map_test.inc b/src/google/protobuf/map_test.inc
@@ -2389,7 +2389,10 @@ class MyMapEntry
   constexpr MyMapEntry()
       : MyMapEntry::MapEntry(static_cast<ClassData*>(nullptr)) {}
   MyMapEntry(Arena* arena) : MyMapEntry::MapEntry(arena, nullptr) {}
-  const ClassData* GetClassData() const { ABSL_CHECK(false); }
+  const ClassData* GetClassData() const {
+    ABSL_CHECK(false);
+    return nullptr;
+  }
   static bool ValidateKey(void*) { return true; }
   static bool ValidateValue(void*) { return true; }
 };