Skip to content

Commit 6c768ff

Browse files
mkruskal-googlecopybara-github
authored andcommitted
Add tests for older gcc versions we still support
PiperOrigin-RevId: 723691910
1 parent e0ae411 commit 6c768ff

File tree

7 files changed

+59
-43
lines changed

7 files changed

+59
-43
lines changed

.github/workflows/test_cpp.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ jobs:
7878
strategy:
7979
fail-fast: false # Don't cancel all jobs if one fails.
8080
matrix:
81-
version: ['9.5', '13.1']
81+
version: ['7.5', '9.1', '9.5', '13.1']
8282
name: Linux GCC ${{ matrix.version }}
8383
runs-on: ubuntu-latest
8484
steps:
@@ -89,7 +89,7 @@ jobs:
8989
- name: Run tests
9090
uses: protocolbuffers/protobuf-ci/bazel-docker@v4
9191
with:
92-
image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-${{ matrix.version }}-d9624f2aa83cba3eaf906f751d75b36aacb9aa82
92+
image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-${{ matrix.version }}-e78301df86b3e4c46ec9ac4d98be00e19305d8f3
9393
credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
9494
bazel-cache: cpp_linux/gcc-${{ matrix.version }}
9595
bazel: test //pkg/... //src/... //third_party/utf8_range/... //conformance:conformance_framework_tests
@@ -356,7 +356,7 @@ jobs:
356356
if: ${{ !matrix.continuous-only || inputs.continuous-run }}
357357
uses: protocolbuffers/protobuf-ci/docker@v4
358358
with:
359-
image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-12.2-d9624f2aa83cba3eaf906f751d75b36aacb9aa82
359+
image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-12.2-e78301df86b3e4c46ec9ac4d98be00e19305d8f3
360360
credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
361361
entrypoint: bash
362362
command: >-

.github/workflows/test_upb.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ jobs:
6969
- name: Run tests
7070
uses: protocolbuffers/protobuf-ci/bazel-docker@v4
7171
with:
72-
image: "us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-12.2-d9624f2aa83cba3eaf906f751d75b36aacb9aa82"
72+
image: "us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:7.1.2-12.2-e78301df86b3e4c46ec9ac4d98be00e19305d8f3"
7373
credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
7474
bazel-cache: "upb-bazel-gcc"
7575
bazel: >-

src/google/protobuf/generated_message_tctable_lite.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -2689,11 +2689,11 @@ const char* TcParser::ParseOneMapEntry(
26892689
void* obj;
26902690
if (inner_tag == key_tag) {
26912691
type_card = map_info.key_type_card;
2692-
type_kind = map.type_info().key_type;
2692+
type_kind = map.type_info().key_type_kind();
26932693
obj = node->GetVoidKey();
26942694
} else {
26952695
type_card = map_info.value_type_card;
2696-
type_kind = map.type_info().value_type;
2696+
type_kind = map.type_info().value_type_kind();
26972697
obj = map.GetVoidValue(node);
26982698
}
26992699

@@ -2842,7 +2842,7 @@ PROTOBUF_NOINLINE const char* TcParser::MpMap(PROTOBUF_TC_PARAM_DECL) {
28422842
WriteMapEntryAsUnknown(msg, table, map, saved_tag, node, map_info);
28432843
} else {
28442844
// Done parsing the node, insert it.
2845-
switch (map.type_info().key_type) {
2845+
switch (map.type_info().key_type_kind()) {
28462846
case UntypedMapBase::TypeKind::kBool:
28472847
static_cast<KeyMapBase<bool>&>(map).InsertOrReplaceNode(
28482848
static_cast<KeyMapBase<bool>::KeyNode*>(node));

src/google/protobuf/map.cc

+7-6
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,9 @@ void UntypedMapBase::ClearTableImpl(bool reset) {
133133
};
134134

135135
const auto dispatch_key = [&](auto value_handler) {
136-
if (type_info_.key_type < TypeKind::kString) {
136+
if (type_info_.key_type_kind() < TypeKind::kString) {
137137
loop(value_handler);
138-
} else if (type_info_.key_type == TypeKind::kString) {
138+
} else if (type_info_.key_type_kind() == TypeKind::kString) {
139139
loop([=](NodeBase* node) {
140140
static_cast<std::string*>(node->GetVoidKey())->~basic_string();
141141
value_handler(node);
@@ -145,13 +145,13 @@ void UntypedMapBase::ClearTableImpl(bool reset) {
145145
}
146146
};
147147

148-
if (type_info_.value_type < TypeKind::kString) {
148+
if (type_info_.value_type_kind() < TypeKind::kString) {
149149
dispatch_key([](NodeBase*) {});
150-
} else if (type_info_.value_type == TypeKind::kString) {
150+
} else if (type_info_.value_type_kind() == TypeKind::kString) {
151151
dispatch_key([&](NodeBase* node) {
152152
GetValue<std::string>(node)->~basic_string();
153153
});
154-
} else if (type_info_.value_type == TypeKind::kMessage) {
154+
} else if (type_info_.value_type_kind() == TypeKind::kMessage) {
155155
dispatch_key([&](NodeBase* node) {
156156
GetValue<MessageLite>(node)->DestroyInstance();
157157
});
@@ -251,7 +251,8 @@ UntypedMapBase::TypeInfo UntypedMapBase::GetTypeInfoDynamic(
251251
key_offsets.end, value_type, value_prototype_if_message, max_align);
252252
return TypeInfo{
253253
Narrow<uint16_t>(AlignTo(value_offsets.end, max_align, max_align)),
254-
Narrow<uint8_t>(value_offsets.start), key_type, value_type};
254+
Narrow<uint8_t>(value_offsets.start), static_cast<uint8_t>(key_type),
255+
static_cast<uint8_t>(value_type)};
255256
}
256257

257258
} // namespace internal

src/google/protobuf/map.h

+11-6
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,13 @@ class PROTOBUF_EXPORT UntypedMapBase {
294294
uint16_t node_size;
295295
// Equivalent to `offsetof(Node, kv.second)` in the derived type.
296296
uint8_t value_offset;
297-
TypeKind key_type : 4;
298-
TypeKind value_type : 4;
297+
uint8_t key_type : 4;
298+
uint8_t value_type : 4;
299+
300+
TypeKind key_type_kind() const { return static_cast<TypeKind>(key_type); }
301+
TypeKind value_type_kind() const {
302+
return static_cast<TypeKind>(value_type);
303+
}
299304
};
300305
static_assert(sizeof(TypeInfo) == 4);
301306

@@ -474,7 +479,7 @@ class PROTOBUF_EXPORT UntypedMapBase {
474479

475480
template <typename F>
476481
auto UntypedMapBase::VisitKeyType(F f) const {
477-
switch (type_info_.key_type) {
482+
switch (type_info_.key_type_kind()) {
478483
case TypeKind::kBool:
479484
return f(std::enable_if<true, bool>{});
480485
case TypeKind::kU32:
@@ -494,7 +499,7 @@ auto UntypedMapBase::VisitKeyType(F f) const {
494499

495500
template <typename F>
496501
auto UntypedMapBase::VisitValueType(F f) const {
497-
switch (type_info_.value_type) {
502+
switch (type_info_.value_type_kind()) {
498503
case TypeKind::kBool:
499504
return f(std::enable_if<true, bool>{});
500505
case TypeKind::kU32:
@@ -1446,8 +1451,8 @@ class Map : private internal::KeyMapBase<internal::KeyForBase<Key>> {
14461451
return internal::UntypedMapBase::TypeInfo{
14471452
sizeof(Node),
14481453
PROTOBUF_FIELD_OFFSET(Node, kv.second),
1449-
internal::UntypedMapBase::StaticTypeKind<Key>(),
1450-
internal::UntypedMapBase::StaticTypeKind<T>(),
1454+
static_cast<uint8_t>(internal::UntypedMapBase::StaticTypeKind<Key>()),
1455+
static_cast<uint8_t>(internal::UntypedMapBase::StaticTypeKind<T>()),
14511456
};
14521457
}
14531458

src/google/protobuf/map_test.cc

+30-23
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,32 @@ TEST(MapTest, SizeTypeIsSizeT) {
309309
(void)x;
310310
}
311311

312+
using KeyTypes = void (*)(bool, int32_t, uint32_t, int64_t, uint64_t,
313+
std::string);
314+
// Some arbitrary proto enum.
315+
using SomeEnum = proto2_unittest::TestAllTypes::NestedEnum;
316+
using ValueTypes = void (*)(bool, int32_t, uint32_t, int64_t, uint64_t, float,
317+
double, std::string, SomeEnum,
318+
proto2_unittest::TestEmptyMessage,
319+
proto2_unittest::TestAllTypes);
320+
321+
TEST(MapTest, StaticTypeKindWorks) {
322+
using UMB = UntypedMapBase;
323+
EXPECT_EQ(UMB::TypeKind::kBool, UMB::StaticTypeKind<bool>());
324+
EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<int32_t>());
325+
EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<uint32_t>());
326+
EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<SomeEnum>());
327+
EXPECT_EQ(UMB::TypeKind::kU64, UMB::StaticTypeKind<int64_t>());
328+
EXPECT_EQ(UMB::TypeKind::kU64, UMB::StaticTypeKind<uint64_t>());
329+
EXPECT_EQ(UMB::TypeKind::kString, UMB::StaticTypeKind<std::string>());
330+
EXPECT_EQ(UMB::TypeKind::kMessage,
331+
UMB::StaticTypeKind<proto2_unittest::TestAllTypes>());
332+
}
333+
334+
#if !defined(__GNUC__) || defined(__clang__) || PROTOBUF_GNUC_MIN(9, 4)
335+
// Parameter pack expansion bug before GCC 8.2:
336+
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85305
337+
312338
template <typename F, typename... Key, typename... Value>
313339
void TestAllKeyValueTypes(void (*)(Key...), void (*)(Value...), F f) {
314340
(
@@ -319,15 +345,6 @@ void TestAllKeyValueTypes(void (*)(Key...), void (*)(Value...), F f) {
319345
...);
320346
}
321347

322-
using KeyTypes = void (*)(bool, int32_t, uint32_t, int64_t, uint64_t,
323-
std::string);
324-
// Some arbitrary proto enum.
325-
using SomeEnum = proto2_unittest::TestAllTypes::NestedEnum;
326-
using ValueTypes = void (*)(bool, int32_t, uint32_t, int64_t, uint64_t, float,
327-
double, std::string, SomeEnum,
328-
proto2_unittest::TestEmptyMessage,
329-
proto2_unittest::TestAllTypes);
330-
331348
TEST(MapTest, StaticTypeInfoMatchesDynamicOne) {
332349
TestAllKeyValueTypes(KeyTypes(), ValueTypes(), [](auto key, auto value) {
333350
using Key = decltype(key);
@@ -338,27 +355,15 @@ TEST(MapTest, StaticTypeInfoMatchesDynamicOne) {
338355
}
339356
const auto type_info = MapTestPeer::GetTypeInfo<Map<Key, Value>>();
340357
const auto dyn_type_info = internal::UntypedMapBase::GetTypeInfoDynamic(
341-
type_info.key_type, type_info.value_type, value_prototype);
358+
type_info.key_type_kind(), type_info.value_type_kind(),
359+
value_prototype);
342360
EXPECT_EQ(dyn_type_info.node_size, type_info.node_size);
343361
EXPECT_EQ(dyn_type_info.value_offset, type_info.value_offset);
344362
EXPECT_EQ(dyn_type_info.key_type, type_info.key_type);
345363
EXPECT_EQ(dyn_type_info.value_type, type_info.value_type);
346364
});
347365
}
348366

349-
TEST(MapTest, StaticTypeKindWorks) {
350-
using UMB = UntypedMapBase;
351-
EXPECT_EQ(UMB::TypeKind::kBool, UMB::StaticTypeKind<bool>());
352-
EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<int32_t>());
353-
EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<uint32_t>());
354-
EXPECT_EQ(UMB::TypeKind::kU32, UMB::StaticTypeKind<SomeEnum>());
355-
EXPECT_EQ(UMB::TypeKind::kU64, UMB::StaticTypeKind<int64_t>());
356-
EXPECT_EQ(UMB::TypeKind::kU64, UMB::StaticTypeKind<uint64_t>());
357-
EXPECT_EQ(UMB::TypeKind::kString, UMB::StaticTypeKind<std::string>());
358-
EXPECT_EQ(UMB::TypeKind::kMessage,
359-
UMB::StaticTypeKind<proto2_unittest::TestAllTypes>());
360-
}
361-
362367
template <typename LHS, typename RHS>
363368
void TestEqPtr(LHS* lhs, RHS* rhs) {
364369
// To silence some false positive compiler errors in gcc 9.5
@@ -428,6 +433,8 @@ TEST(MapTest, VisitAllNodesUsesTheRightTypesOnAllNodes) {
428433
});
429434
}
430435

436+
#endif
437+
431438
TEST(MapTest, IteratorNodeFieldIsNullPtrAtEnd) {
432439
Map<int, int> map;
433440
EXPECT_EQ(internal::UntypedMapIterator::FromTyped(map.cbegin()).node_,

src/google/protobuf/map_test.inc

+4-1
Original file line numberDiff line numberDiff line change
@@ -2389,7 +2389,10 @@ class MyMapEntry
23892389
constexpr MyMapEntry()
23902390
: MyMapEntry::MapEntry(static_cast<ClassData*>(nullptr)) {}
23912391
MyMapEntry(Arena* arena) : MyMapEntry::MapEntry(arena, nullptr) {}
2392-
const ClassData* GetClassData() const { ABSL_CHECK(false); }
2392+
const ClassData* GetClassData() const {
2393+
ABSL_CHECK(false);
2394+
return nullptr;
2395+
}
23932396
static bool ValidateKey(void*) { return true; }
23942397
static bool ValidateValue(void*) { return true; }
23952398
};

0 commit comments

Comments
 (0)