Skip to content

Change upb C generated map iteration function to not hand out MapEntry pointers. #20462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zhangskz merged 1 commit into from
Feb 25, 2025
Merged

Change upb C generated map iteration function to not hand out MapEntry pointers. #20462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 41 additions & 47 deletions upb/message/copy_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,55 +191,49 @@ TEST(GeneratedCode, DeepCloneMessageMapField) {
protobuf_test_messages_proto2_TestAllTypesProto2_NestedMessage_set_a(nested,
0);
upb_Arena_Free(source_arena);
size_t iter = kUpb_Map_Begin;
// Test map<int32, int32>.
const protobuf_test_messages_proto2_TestAllTypesProto2_MapInt32DoubleEntry*
int32_double_entry =
protobuf_test_messages_proto2_TestAllTypesProto2_map_int32_double_next(
clone, &iter);
ASSERT_NE(int32_double_entry, nullptr);
EXPECT_EQ(
protobuf_test_messages_proto2_TestAllTypesProto2_MapInt32DoubleEntry_key(
int32_double_entry),
12);
EXPECT_EQ(
protobuf_test_messages_proto2_TestAllTypesProto2_MapInt32DoubleEntry_value(
int32_double_entry),
1200.5);
// Test map<int32, double>.
{
int32_t key;
double value;
size_t iter = kUpb_Map_Begin;

ASSERT_TRUE(
protobuf_test_messages_proto2_TestAllTypesProto2_map_int32_double_next(
clone, &key, &value, &iter));
EXPECT_EQ(key, 12);
EXPECT_EQ(value, 1200.5);
}

// Test map<string, string>.
iter = kUpb_Map_Begin;
const protobuf_test_messages_proto2_TestAllTypesProto2_MapStringStringEntry*
string_string_entry =
protobuf_test_messages_proto2_TestAllTypesProto2_map_string_string_next(
clone, &iter);
ASSERT_NE(string_string_entry, nullptr);
EXPECT_TRUE(upb_StringView_IsEqual(
protobuf_test_messages_proto2_TestAllTypesProto2_MapStringStringEntry_key(
string_string_entry),
upb_StringView_FromString("key1")));
EXPECT_TRUE(upb_StringView_IsEqual(
protobuf_test_messages_proto2_TestAllTypesProto2_MapStringStringEntry_value(
string_string_entry),
upb_StringView_FromString("value1")));
{
upb_StringView key;
upb_StringView value;
size_t iter = kUpb_Map_Begin;

ASSERT_TRUE(
protobuf_test_messages_proto2_TestAllTypesProto2_map_string_string_next(
clone, &key, &value, &iter));
EXPECT_TRUE(upb_StringView_IsEqual(key, upb_StringView_FromString("key1")));
EXPECT_TRUE(
upb_StringView_IsEqual(value, upb_StringView_FromString("value1")));
}

// Test map<string, NestedMessage>.
iter = kUpb_Map_Begin;
const protobuf_test_messages_proto2_TestAllTypesProto2_MapStringNestedMessageEntry*
nested_message_entry =
protobuf_test_messages_proto2_TestAllTypesProto2_map_string_nested_message_next(
clone, &iter);
ASSERT_NE(nested_message_entry, nullptr);
EXPECT_TRUE(upb_StringView_IsEqual(
protobuf_test_messages_proto2_TestAllTypesProto2_MapStringNestedMessageEntry_key(
nested_message_entry),
upb_StringView_FromString("nestedkey1")));
const protobuf_test_messages_proto2_TestAllTypesProto2_NestedMessage*
cloned_nested =
protobuf_test_messages_proto2_TestAllTypesProto2_MapStringNestedMessageEntry_value(
nested_message_entry);
ASSERT_NE(cloned_nested, nullptr);
EXPECT_EQ(protobuf_test_messages_proto2_TestAllTypesProto2_NestedMessage_a(
cloned_nested),
kTestNestedInt32);
{
upb_StringView key;
const protobuf_test_messages_proto2_TestAllTypesProto2_NestedMessage* value;
size_t iter = kUpb_Map_Begin;
ASSERT_TRUE(
protobuf_test_messages_proto2_TestAllTypesProto2_map_string_nested_message_next(
clone, &key, &value, &iter));
EXPECT_TRUE(
upb_StringView_IsEqual(key, upb_StringView_FromString("nestedkey1")));
ASSERT_NE(value, nullptr);
EXPECT_EQ(
protobuf_test_messages_proto2_TestAllTypesProto2_NestedMessage_a(value),
kTestNestedInt32);
}

upb_Arena_Free(arena);
}

Expand Down
95 changes: 31 additions & 64 deletions upb/test/test_generated_code.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,15 +505,16 @@ static void check_string_map_empty(
0,
protobuf_test_messages_proto3_TestAllTypesProto3_map_string_string_size(
msg));

upb_StringView key;
upb_StringView val;
EXPECT_FALSE(
protobuf_test_messages_proto3_TestAllTypesProto3_map_string_string_next(
msg, &iter));
msg, &key, &val, &iter));
}

static void check_string_map_one_entry(
protobuf_test_messages_proto3_TestAllTypesProto3* msg) {
const protobuf_test_messages_proto3_TestAllTypesProto3_MapStringStringEntry*
const_ent;
size_t iter;
upb_StringView str;

Expand All @@ -532,23 +533,15 @@ static void check_string_map_one_entry(

/* Test that iteration reveals a single k/v pair in the map. */
iter = kUpb_Map_Begin;
const_ent =
protobuf_test_messages_proto3_TestAllTypesProto3_map_string_string_next(
msg, &iter);
ASSERT_NE(nullptr, const_ent);
EXPECT_TRUE(upb_StringView_IsEqual(
test_str_view,
protobuf_test_messages_proto3_TestAllTypesProto3_MapStringStringEntry_key(
const_ent)));
EXPECT_TRUE(upb_StringView_IsEqual(
test_str_view2,
protobuf_test_messages_proto3_TestAllTypesProto3_MapStringStringEntry_value(
const_ent)));

const_ent =
upb_StringView key;
upb_StringView val;
protobuf_test_messages_proto3_TestAllTypesProto3_map_string_string_next(
msg, &key, &val, &iter);
EXPECT_TRUE(upb_StringView_IsEqual(test_str_view, key));
EXPECT_TRUE(upb_StringView_IsEqual(test_str_view2, val));
EXPECT_FALSE(
protobuf_test_messages_proto3_TestAllTypesProto3_map_string_string_next(
msg, &iter);
EXPECT_EQ(nullptr, const_ent);
msg, &key, &val, &iter));
}

TEST(GeneratedCode, StringDoubleMap) {
Expand Down Expand Up @@ -580,8 +573,6 @@ TEST(GeneratedCode, StringMap) {
upb_Arena* arena = upb_Arena_New();
protobuf_test_messages_proto3_TestAllTypesProto3* msg =
protobuf_test_messages_proto3_TestAllTypesProto3_new(arena);
const protobuf_test_messages_proto3_TestAllTypesProto3_MapStringStringEntry*
const_ent;
size_t iter, count;

check_string_map_empty(msg);
Expand Down Expand Up @@ -615,18 +606,11 @@ TEST(GeneratedCode, StringMap) {
/* Test iteration */
iter = kUpb_Map_Begin;
count = 0;

upb_StringView key;
upb_StringView val;
while (
(const_ent =
protobuf_test_messages_proto3_TestAllTypesProto3_map_string_string_next(
msg, &iter)) != nullptr) {
upb_StringView key =
protobuf_test_messages_proto3_TestAllTypesProto3_MapStringStringEntry_key(
const_ent);
upb_StringView val =
protobuf_test_messages_proto3_TestAllTypesProto3_MapStringStringEntry_value(
const_ent);

protobuf_test_messages_proto3_TestAllTypesProto3_map_string_string_next(
msg, &key, &val, &iter)) {
count++;
if (upb_StringView_IsEqual(key, test_str_view)) {
EXPECT_TRUE(upb_StringView_IsEqual(val, test_str_view2));
Expand All @@ -652,16 +636,18 @@ static void check_int32_map_empty(
EXPECT_EQ(
0, protobuf_test_messages_proto3_TestAllTypesProto3_map_int32_int32_size(
msg));

int32_t key;
int32_t val;
EXPECT_FALSE(
protobuf_test_messages_proto3_TestAllTypesProto3_map_int32_int32_next(
msg, &iter));
msg, &key, &val, &iter));
}

static void check_int32_map_one_entry(
protobuf_test_messages_proto3_TestAllTypesProto3* msg) {
const protobuf_test_messages_proto3_TestAllTypesProto3_MapInt32Int32Entry*
const_ent;
size_t iter;
int32_t key;
int32_t val;

EXPECT_EQ(
Expand All @@ -678,31 +664,20 @@ static void check_int32_map_one_entry(

/* Test that iteration reveals a single k/v pair in the map. */
iter = kUpb_Map_Begin;
const_ent =
EXPECT_TRUE(
protobuf_test_messages_proto3_TestAllTypesProto3_map_int32_int32_next(
msg, &iter);
ASSERT_NE(nullptr, const_ent);
EXPECT_EQ(
test_int32,
protobuf_test_messages_proto3_TestAllTypesProto3_MapInt32Int32Entry_key(
const_ent));
EXPECT_EQ(
test_int32_2,
protobuf_test_messages_proto3_TestAllTypesProto3_MapInt32Int32Entry_value(
const_ent));

const_ent =
msg, &key, &val, &iter));
EXPECT_EQ(test_int32, key);
EXPECT_EQ(test_int32_2, val);
EXPECT_FALSE(
protobuf_test_messages_proto3_TestAllTypesProto3_map_int32_int32_next(
msg, &iter);
EXPECT_EQ(nullptr, const_ent);
msg, &key, &val, &iter));
}

TEST(GeneratedCode, Int32Map) {
upb_Arena* arena = upb_Arena_New();
protobuf_test_messages_proto3_TestAllTypesProto3* msg =
protobuf_test_messages_proto3_TestAllTypesProto3_new(arena);
const protobuf_test_messages_proto3_TestAllTypesProto3_MapInt32Int32Entry*
const_ent;
size_t iter, count;

check_int32_map_empty(msg);
Expand Down Expand Up @@ -751,18 +726,10 @@ TEST(GeneratedCode, Int32Map) {
/* Test iteration */
iter = kUpb_Map_Begin;
count = 0;

while (
(const_ent =
protobuf_test_messages_proto3_TestAllTypesProto3_map_int32_int32_next(
msg, &iter)) != nullptr) {
int32_t key =
protobuf_test_messages_proto3_TestAllTypesProto3_MapInt32Int32Entry_key(
const_ent);
int32_t val =
protobuf_test_messages_proto3_TestAllTypesProto3_MapInt32Int32Entry_value(
const_ent);

int32_t key;
int32_t val;
while (protobuf_test_messages_proto3_TestAllTypesProto3_map_int32_int32_next(
msg, &key, &val, &iter)) {
count++;
if (key == test_int32) {
EXPECT_EQ(val, test_int32_2);
Expand Down
20 changes: 15 additions & 5 deletions upb_generator/c/generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ std::string MapValueCType(upb::FieldDefPtr map_field) {
return CType(map_field.message_type().map_value());
}

std::string MapValueCTypeConst(upb::FieldDefPtr map_field) {
return CTypeConst(map_field.message_type().map_value());
}

std::string MapKeyValueSize(upb_CType ctype, absl::string_view expr) {
return ctype == kUpb_CType_String || ctype == kUpb_CType_Bytes
? "0"
Expand Down Expand Up @@ -481,14 +485,20 @@ void GenerateMapGetters(upb::FieldDefPtr field, const DefPoolPair& pools,
MapValueSize(field, "*val"));
output(
R"cc(
UPB_INLINE $0 $1_$2_next(const $1* msg, size_t* iter) {
const upb_MiniTableField field = $3;
UPB_INLINE bool $0_$1_next(const $0* msg, $2* key, $3* val,
size_t* iter) {
const upb_MiniTableField field = $4;
const upb_Map* map = upb_Message_GetMap(UPB_UPCAST(msg), &field);
if (!map) return NULL;
return ($0)_upb_map_next(map, iter);
if (!map) return false;
upb_MessageValue k;
upb_MessageValue v;
if (!upb_Map_Next(map, &k, &v, iter)) return false;
memcpy(key, &k, sizeof(*key));
memcpy(val, &v, sizeof(*val));
return true;
}
)cc",
CTypeConst(field), msg_name, resolved_name,
msg_name, resolved_name, MapKeyCType(field), MapValueCTypeConst(field),
FieldInitializerStrong(pools, field, options));
// Generate private getter returning a upb_Map or NULL for immutable and
// a upb_Map for mutable.
Expand Down
Loading