Skip to content

Commit

Permalink
Remove separate setters for singular scalars
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 590609323
  • Loading branch information
Necior authored and copybara-github committed Dec 13, 2023
1 parent 5076e62 commit b19deb9
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 68 deletions.
8 changes: 4 additions & 4 deletions rust/test/cpp/interop/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@ fn mutate_message_in_cpp() {
}

let mut msg2 = TestAllTypes::new();
msg2.optional_int64_set(Some(42));
msg2.optional_int64_mut().set(42);
msg2.optional_bytes_mut().set(b"something mysterious");
msg2.optional_bool_set(Some(false));
msg2.optional_bool_mut().set(false);

proto_assert_eq!(msg1, msg2);
}

#[test]
fn deserialize_in_rust() {
let mut msg1 = TestAllTypes::new();
msg1.optional_int64_set(Some(-1));
msg1.optional_int64_mut().set(-1);
msg1.optional_bytes_mut().set(b"some cool data I guess");
let serialized =
unsafe { SerializeTestAllTypes(msg1.__unstable_cpp_repr_grant_permission_to_break()) };
Expand All @@ -63,7 +63,7 @@ fn deserialize_in_rust() {
#[test]
fn deserialize_in_cpp() {
let mut msg1 = TestAllTypes::new();
msg1.optional_int64_set(Some(-1));
msg1.optional_int64_mut().set(-1);
msg1.optional_bytes_mut().set(b"some cool data I guess");
let data = msg1.serialize();

Expand Down
20 changes: 6 additions & 14 deletions rust/test/shared/accessors_proto3_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ fn test_fixed32_accessors() {
msg.optional_fixed32_mut().set(u32::default());
assert_that!(msg.optional_fixed32(), eq(0));
assert_that!(msg.optional_fixed32_mut().get(), eq(0));

msg.optional_fixed32_set(43);
assert_that!(msg.optional_fixed32(), eq(43));
assert_that!(msg.optional_fixed32_mut().get(), eq(43));
}

#[test]
Expand All @@ -45,10 +41,6 @@ fn test_bool_accessors() {
msg.optional_bool_mut().set(bool::default());
assert_that!(msg.optional_bool(), eq(false));
assert_that!(msg.optional_bool_mut().get(), eq(false));

msg.optional_bool_set(true);
assert_that!(msg.optional_bool(), eq(true));
assert_that!(msg.optional_bool_mut().get(), eq(true));
}

#[test]
Expand Down Expand Up @@ -199,11 +191,11 @@ fn test_oneof_accessors() {
let mut msg = TestAllTypes::new();
assert_that!(msg.oneof_field(), matches_pattern!(not_set(_)));

msg.oneof_uint32_set(Some(7));
msg.oneof_uint32_mut().set(7);
assert_that!(msg.oneof_uint32_opt(), eq(Optional::Set(7)));
assert_that!(msg.oneof_field(), matches_pattern!(OneofUint32(eq(7))));

msg.oneof_uint32_set(None);
msg.oneof_uint32_mut().clear();
assert_that!(msg.oneof_uint32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.oneof_field(), matches_pattern!(not_set(_)));

Expand All @@ -217,7 +209,7 @@ fn test_oneof_accessors() {
// eq(Optional::Unset(_))); assert_that!(msg.oneof_field(),
// matches_pattern!(OneofNestedMessage(_)));

msg.oneof_uint32_set(Some(7));
msg.oneof_uint32_mut().set(7);
msg.oneof_bytes_mut().set(b"123");
assert_that!(msg.oneof_uint32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.oneof_field(), matches_pattern!(OneofBytes(eq(b"123"))));
Expand All @@ -233,7 +225,7 @@ fn test_oneof_mut_accessors() {
let mut msg = TestAllTypes::new();
assert_that!(msg.oneof_field_mut(), matches_pattern!(not_set(_)));

msg.oneof_uint32_set(Some(7));
msg.oneof_uint32_mut().set(7);

match msg.oneof_field_mut() {
OneofUint32(mut v) => {
Expand All @@ -252,10 +244,10 @@ fn test_oneof_mut_accessors() {
matches_pattern!(TestAllTypes_::OneofField::OneofUint32(eq(8)))
);

msg.oneof_uint32_set(None);
msg.oneof_uint32_mut().clear();
assert_that!(msg.oneof_field_mut(), matches_pattern!(not_set(_)));

msg.oneof_uint32_set(Some(7));
msg.oneof_uint32_mut().set(7);
msg.oneof_bytes_mut().set(b"123");
assert_that!(msg.oneof_field_mut(), matches_pattern!(OneofBytes(_)));
}
56 changes: 28 additions & 28 deletions rust/test/shared/accessors_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ fn test_optional_fixed32_accessors() {
assert_that!(msg.optional_fixed32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_fixed32(), eq(0));

msg.optional_fixed32_set(Some(99));
msg.optional_fixed32_mut().set(99);
assert_that!(msg.optional_fixed32_opt(), eq(Optional::Set(99)));
assert_that!(msg.optional_fixed32(), eq(99));

msg.optional_fixed32_set(None);
msg.optional_fixed32_mut().clear();
assert_that!(msg.optional_fixed32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_fixed32(), eq(0));
}
Expand Down Expand Up @@ -85,11 +85,11 @@ fn test_optional_fixed64_accessors() {
assert_that!(msg.optional_fixed64_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_fixed64(), eq(0));

msg.optional_fixed64_set(Some(2000));
msg.optional_fixed64_mut().set(2000);
assert_that!(msg.optional_fixed64_opt(), eq(Optional::Set(2000)));
assert_that!(msg.optional_fixed64(), eq(2000));

msg.optional_fixed64_set(None);
msg.optional_fixed64_mut().clear();
assert_that!(msg.optional_fixed64_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_fixed64(), eq(0));
}
Expand Down Expand Up @@ -127,11 +127,11 @@ fn test_optional_int32_accessors() {
assert_that!(msg.optional_int32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_int32(), eq(0));

msg.optional_int32_set(Some(1));
msg.optional_int32_mut().set(1);
assert_that!(msg.optional_int32_opt(), eq(Optional::Set(1)));
assert_that!(msg.optional_int32(), eq(1));

msg.optional_int32_set(None);
msg.optional_int32_mut().clear();
assert_that!(msg.optional_int32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_int32(), eq(0));
}
Expand Down Expand Up @@ -169,11 +169,11 @@ fn test_optional_int64_accessors() {
assert_that!(msg.optional_int64_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_int64(), eq(0));

msg.optional_int64_set(Some(42));
msg.optional_int64_mut().set(42);
assert_that!(msg.optional_int64_opt(), eq(Optional::Set(42)));
assert_that!(msg.optional_int64(), eq(42));

msg.optional_int64_set(None);
msg.optional_int64_mut().clear();
assert_that!(msg.optional_int64_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_int64(), eq(0));
}
Expand Down Expand Up @@ -211,11 +211,11 @@ fn test_optional_sint32_accessors() {
assert_that!(msg.optional_sint32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_sint32(), eq(0));

msg.optional_sint32_set(Some(-22));
msg.optional_sint32_mut().set(-22);
assert_that!(msg.optional_sint32_opt(), eq(Optional::Set(-22)));
assert_that!(msg.optional_sint32(), eq(-22));

msg.optional_sint32_set(None);
msg.optional_sint32_mut().clear();
assert_that!(msg.optional_sint32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_sint32(), eq(0));
}
Expand Down Expand Up @@ -253,11 +253,11 @@ fn test_optional_sint64_accessors() {
assert_that!(msg.optional_sint64_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_sint64(), eq(0));

msg.optional_sint64_set(Some(7000));
msg.optional_sint64_mut().set(7000);
assert_that!(msg.optional_sint64_opt(), eq(Optional::Set(7000)));
assert_that!(msg.optional_sint64(), eq(7000));

msg.optional_sint64_set(None);
msg.optional_sint64_mut().clear();
assert_that!(msg.optional_sint64_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_sint64(), eq(0));
}
Expand Down Expand Up @@ -295,11 +295,11 @@ fn test_optional_uint32_accessors() {
assert_that!(msg.optional_uint32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_uint32(), eq(0));

msg.optional_uint32_set(Some(9001));
msg.optional_uint32_mut().set(9001);
assert_that!(msg.optional_uint32_opt(), eq(Optional::Set(9001)));
assert_that!(msg.optional_uint32(), eq(9001));

msg.optional_uint32_set(None);
msg.optional_uint32_mut().clear();
assert_that!(msg.optional_uint32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_uint32(), eq(0));
}
Expand Down Expand Up @@ -337,11 +337,11 @@ fn test_optional_uint64_accessors() {
assert_that!(msg.optional_uint64_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_uint64(), eq(0));

msg.optional_uint64_set(Some(42));
msg.optional_uint64_mut().set(42);
assert_that!(msg.optional_uint64_opt(), eq(Optional::Set(42)));
assert_that!(msg.optional_uint64(), eq(42));

msg.optional_uint64_set(None);
msg.optional_uint64_mut().clear();
assert_that!(msg.optional_uint64_opt(), eq(Optional::Unset(0)));
assert_that!(msg.optional_uint64(), eq(0));
}
Expand Down Expand Up @@ -379,11 +379,11 @@ fn test_optional_float_accessors() {
assert_that!(msg.optional_float_opt(), eq(Optional::Unset(0.0)));
assert_that!(msg.optional_float(), eq(0.0));

msg.optional_float_set(Some(std::f32::consts::PI));
msg.optional_float_mut().set(std::f32::consts::PI);
assert_that!(msg.optional_float_opt(), eq(Optional::Set(std::f32::consts::PI)));
assert_that!(msg.optional_float(), eq(std::f32::consts::PI));

msg.optional_float_set(None);
msg.optional_float_mut().clear();
assert_that!(msg.optional_float_opt(), eq(Optional::Unset(0.0)));
assert_that!(msg.optional_float(), eq(0.0));
}
Expand Down Expand Up @@ -421,11 +421,11 @@ fn test_optional_double_accessors() {
assert_that!(msg.optional_double_opt(), eq(Optional::Unset(0.0)));
assert_that!(msg.optional_double(), eq(0.0));

msg.optional_double_set(Some(-10.99));
msg.optional_double_mut().set(-10.99);
assert_that!(msg.optional_double_opt(), eq(Optional::Set(-10.99)));
assert_that!(msg.optional_double(), eq(-10.99));

msg.optional_double_set(None);
msg.optional_double_mut().clear();
assert_that!(msg.optional_double_opt(), eq(Optional::Unset(0.0)));
assert_that!(msg.optional_double(), eq(0.0));
}
Expand Down Expand Up @@ -462,10 +462,10 @@ fn test_optional_bool_accessors() {
let mut msg = TestAllTypes::new();
assert_that!(msg.optional_bool_opt(), eq(Optional::Unset(false)));

msg.optional_bool_set(Some(true));
msg.optional_bool_mut().set(true);
assert_that!(msg.optional_bool_opt(), eq(Optional::Set(true)));

msg.optional_bool_set(None);
msg.optional_bool_mut().clear();
assert_that!(msg.optional_bool_opt(), eq(Optional::Unset(false)));
}

Expand Down Expand Up @@ -687,15 +687,15 @@ fn test_oneof_accessors() {
let mut msg = TestAllTypes::new();
assert_that!(msg.oneof_field(), matches_pattern!(not_set(_)));

msg.oneof_uint32_set(Some(7));
msg.oneof_uint32_mut().set(7);
assert_that!(msg.oneof_uint32_opt(), eq(Optional::Set(7)));
assert_that!(msg.oneof_field(), matches_pattern!(OneofUint32(eq(7))));

msg.oneof_uint32_set(None);
msg.oneof_uint32_mut().clear();
assert_that!(msg.oneof_uint32_opt(), eq(Optional::Unset(0)));
assert_that!(msg.oneof_field(), matches_pattern!(not_set(_)));

msg.oneof_uint32_set(Some(7));
msg.oneof_uint32_mut().set(7);
msg.oneof_bytes_mut().set(b"123");
assert_that!(msg.oneof_uint32_opt(), eq(Optional::Unset(0)));

Expand All @@ -709,7 +709,7 @@ fn test_oneof_mut_accessors() {
let mut msg = TestAllTypes::new();
assert_that!(msg.oneof_field_mut(), matches_pattern!(not_set(_)));

msg.oneof_uint32_set(Some(7));
msg.oneof_uint32_mut().set(7);

match msg.oneof_field_mut() {
OneofUint32(mut v) => {
Expand All @@ -728,10 +728,10 @@ fn test_oneof_mut_accessors() {
matches_pattern!(TestAllTypes_::OneofField::OneofUint32(eq(8)))
);

msg.oneof_uint32_set(None);
msg.oneof_uint32_mut().clear();
assert_that!(msg.oneof_field_mut(), matches_pattern!(not_set(_)));

msg.oneof_uint32_set(Some(7));
msg.oneof_uint32_mut().set(7);
msg.oneof_bytes_mut().set(b"123");
assert_that!(msg.oneof_field_mut(), matches_pattern!(OneofBytes(_)));
}
4 changes: 2 additions & 2 deletions rust/test/shared/serialization_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use unittest_proto::proto2_unittest::TestAllTypes;
#[test]
fn serialize_deserialize_message() {
let mut msg = TestAllTypes::new();
msg.optional_int64_set(Some(42));
msg.optional_bool_set(Some(true));
msg.optional_int64_mut().set(42);
msg.optional_bool_mut().set(true);
msg.optional_bytes_mut().set(b"serialize deserialize test");

let serialized = msg.serialize();
Expand Down
20 changes: 0 additions & 20 deletions src/google/protobuf/compiler/rust/accessors/singular_scalar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,6 @@ void SingularScalar::InMsgImpl(Context<FieldDescriptor> field) const {
{"getter_thunk", Thunk(field, "get")},
{"setter_thunk", Thunk(field, "set")},
{"clearer_thunk", Thunk(field, "clear")},
{"field_setter",
[&] {
if (field.desc().has_presence()) {
field.Emit({}, R"rs(
pub fn r#$field$_set(&mut self, val: Option<$Scalar$>) {
match val {
Some(val) => unsafe { $setter_thunk$(self.inner.msg, val) },
None => unsafe { $clearer_thunk$(self.inner.msg) },
}
}
)rs");
} else {
field.Emit({}, R"rs(
pub fn r#$field$_set(&mut self, val: $Scalar$) {
unsafe { $setter_thunk$(self.inner.msg, val) }
}
)rs");
}
}},
{"field_mutator_getter",
[&] {
if (field.desc().has_presence()) {
Expand Down Expand Up @@ -124,7 +105,6 @@ void SingularScalar::InMsgImpl(Context<FieldDescriptor> field) const {
R"rs(
$getter$
$getter_opt$
$field_setter$
$field_mutator_getter$
)rs");
}
Expand Down

0 comments on commit b19deb9

Please sign in to comment.