Skip to content

Commit e978bcc

Browse files
ClaytonKnittelcopybara-github
authored andcommitted
Make RepeatedFieldProxy non-assignable.
PiperOrigin-RevId: 883438319
1 parent d5548f9 commit e978bcc

File tree

2 files changed

+49
-4
lines changed

2 files changed

+49
-4
lines changed

src/google/protobuf/repeated_field_proxy.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,9 @@ class PROTOBUF_DECLSPEC_EMPTY_BASES RepeatedFieldProxy final
437437

438438
public:
439439
RepeatedFieldProxy(const RepeatedFieldProxy& other) = default;
440-
RepeatedFieldProxy& operator=(const RepeatedFieldProxy&) = default;
440+
// Mutable proxies are not assignable. This is intentional to avoid confusion
441+
// with the `assign` method, which reassigns the underlying repeated field.
442+
RepeatedFieldProxy& operator=(const RepeatedFieldProxy&) = delete;
441443

442444
// Returns a type which references the element at the given index. Performs
443445
// bounds checking in accordance with `bounds_check_mode_*`.
@@ -560,7 +562,7 @@ class PROTOBUF_DECLSPEC_EMPTY_BASES RepeatedFieldProxy final
560562

561563
Arena* arena() const { return arena_; }
562564

563-
Arena* arena_;
565+
Arena* const arena_;
564566
};
565567

566568
template <typename ElementType>

src/google/protobuf/repeated_field_proxy_test.cc

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1640,13 +1640,56 @@ TYPED_TEST(RepeatedNumericFieldProxyTest, Rebind) {
16401640
auto field2 = this->MakeRepeatedFieldContainer();
16411641
field2->Add(2);
16421642

1643-
auto proxy = field1.MakeProxy();
1644-
proxy = field2.MakeProxy();
1643+
auto proxy = field1.MakeConstProxy();
1644+
proxy = field2.MakeConstProxy();
16451645

16461646
// Proxy should be rebound to field2 without having modified either field.
16471647
EXPECT_THAT(proxy, ElementsAre(2));
16481648
EXPECT_THAT(*field1, ElementsAre(1));
16491649
EXPECT_THAT(*field2, ElementsAre(2));
1650+
1651+
// Verify that mutable proxies can't be rebound.
1652+
static_assert(!std::is_copy_assignable_v<decltype(field1.MakeProxy())>);
1653+
}
1654+
1655+
TYPED_TEST(RepeatedStringFieldProxyTest, Rebind) {
1656+
auto field1 = this->MakeRepeatedFieldContainer();
1657+
this->Add(field1, "1");
1658+
1659+
auto field2 = this->MakeRepeatedFieldContainer();
1660+
this->Add(field2, "2");
1661+
1662+
auto proxy = field1.MakeConstProxy();
1663+
proxy = field2.MakeConstProxy();
1664+
1665+
// Proxy should be rebound to field2 without having modified either field.
1666+
EXPECT_THAT(proxy, ElementsAre(StringEq("2")));
1667+
EXPECT_THAT(*field1, ElementsAre(StringEq("1")));
1668+
EXPECT_THAT(*field2, ElementsAre(StringEq("2")));
1669+
1670+
// Verify that mutable proxies can't be rebound.
1671+
static_assert(!std::is_copy_assignable_v<decltype(field1.MakeProxy())>);
1672+
}
1673+
1674+
TEST_P(RepeatedFieldProxyTest, RebindMessage) {
1675+
auto field1 =
1676+
this->MakeRepeatedFieldContainer<RepeatedFieldProxyTestSimpleMessage>();
1677+
field1->Add()->set_value(1);
1678+
1679+
auto field2 =
1680+
this->MakeRepeatedFieldContainer<RepeatedFieldProxyTestSimpleMessage>();
1681+
field2->Add()->set_value(2);
1682+
1683+
auto proxy = field1.MakeConstProxy();
1684+
proxy = field2.MakeConstProxy();
1685+
1686+
// Proxy should be rebound to field2 without having modified either field.
1687+
EXPECT_THAT(proxy, ElementsAre(EqualsProto(R"pb(value: 2)pb")));
1688+
EXPECT_THAT(*field1, ElementsAre(EqualsProto(R"pb(value: 1)pb")));
1689+
EXPECT_THAT(*field2, ElementsAre(EqualsProto(R"pb(value: 2)pb")));
1690+
1691+
// Verify that mutable proxies can't be rebound.
1692+
static_assert(!std::is_copy_assignable_v<decltype(field1.MakeProxy())>);
16501693
}
16511694

16521695
TEST_P(RepeatedFieldProxyTest, ExplicitConversionToLegacyRepeatedField) {

0 commit comments

Comments
 (0)