diff --git a/rust/test/shared/accessors_proto3_test.rs b/rust/test/shared/accessors_proto3_test.rs index 9e6047b405..35d32ec0ed 100644 --- a/rust/test/shared/accessors_proto3_test.rs +++ b/rust/test/shared/accessors_proto3_test.rs @@ -269,7 +269,7 @@ fn test_oneof_accessors() { assert_that!(msg.oneof_uint32_opt(), eq(Optional::Set(7))); assert_that!(msg.oneof_field(), matches_pattern!(OneofUint32(eq(7)))); - msg.set_oneof_uint32_opt(None); + msg.clear_oneof_uint32(); assert_that!(msg.oneof_uint32_opt(), eq(Optional::Unset(0))); assert_that!(msg.oneof_field(), matches_pattern!(not_set(_))); @@ -334,10 +334,18 @@ fn test_oneof_mut_accessors() { matches_pattern!(TestAllTypes_::OneofField::OneofUint32(eq(8))) ); - msg.set_oneof_uint32_opt(None); + // Clearing a different field in the same oneof doesn't affect the other, set + // field. + msg.clear_oneof_bytes(); + assert_that!( + msg.oneof_field(), + matches_pattern!(TestAllTypes_::OneofField::OneofUint32(eq(8))) + ); + + msg.clear_oneof_uint32(); assert_that!(msg.oneof_field_mut(), matches_pattern!(not_set(_))); - msg.set_oneof_uint32_opt(Some(7)); + msg.set_oneof_uint32(7); msg.set_oneof_bytes(b"123"); assert_that!(msg.oneof_field_mut(), matches_pattern!(OneofBytes(_))); } @@ -357,3 +365,16 @@ fn test_oneof_mut_enum_accessors() { assert_that!(msg.foo_enum_opt(), eq(Optional::Set(NestedEnum::Bar))); assert_that!(msg.foo_mut(), matches_pattern!(FooMut::FooEnum(_))); } + +#[test] +fn test_submsg_setter() { + use TestAllTypes_::*; + + let mut nested = NestedMessage::new(); + nested.set_bb(7); + + let mut parent = TestAllTypes::new(); + parent.set_optional_nested_message(nested); + + assert_that!(parent.optional_nested_message().bb(), eq(7)); +} diff --git a/rust/test/shared/accessors_test.rs b/rust/test/shared/accessors_test.rs index 4c82c85fec..b9a5565b8b 100644 --- a/rust/test/shared/accessors_test.rs +++ b/rust/test/shared/accessors_test.rs @@ -55,11 +55,7 @@ fn test_optional_fixed32_accessors() { assert_that!(msg.optional_fixed32_opt(), eq(Optional::Set(7))); assert_that!(msg.optional_fixed32(), eq(7)); - msg.set_optional_fixed32_opt(Some(8)); - assert_that!(msg.optional_fixed32_opt(), eq(Optional::Set(8))); - assert_that!(msg.optional_fixed32(), eq(8)); - - msg.set_optional_fixed32_opt(None); + msg.clear_optional_fixed32(); assert_that!(msg.optional_fixed32_opt(), eq(Optional::Unset(0))); assert_that!(msg.optional_fixed32(), eq(0)); } @@ -176,13 +172,13 @@ fn test_default_int32_accessors() { assert_that!(msg.default_int32_mut().is_set(), eq(false)); assert_that!(msg.default_int32_opt(), eq(Optional::Unset(41))); - msg.set_default_int32_opt(Some(41)); + msg.set_default_int32(41); assert_that!(msg.default_int32(), eq(41)); assert_that!(msg.default_int32_mut().get(), eq(41)); assert_that!(msg.default_int32_mut().is_set(), eq(true)); assert_that!(msg.default_int32_opt(), eq(Optional::Set(41))); - msg.set_default_int32_opt(None); + msg.clear_default_int32(); assert_that!(msg.default_int32(), eq(41)); assert_that!(msg.default_int32_mut().get(), eq(41)); assert_that!(msg.default_int32_mut().is_set(), eq(false)); @@ -624,7 +620,7 @@ fn test_nonempty_default_bytes_accessors() { assert_that!(msg.default_bytes(), eq(b"world")); assert_that!(msg.default_bytes_opt(), eq(Optional::Set(&b"world"[..]))); - msg.set_default_bytes_opt(None::<&[u8]>); + msg.clear_default_bytes(); assert_that!(msg.default_bytes_opt(), eq(Optional::Unset(&b"world"[..]))); assert_that!(msg.default_bytes_mut(), is_unset()); @@ -669,7 +665,7 @@ fn test_optional_string_accessors() { assert_that!(msg.optional_string(), eq("")); assert_that!(msg.optional_string_opt(), eq(Optional::Set("".into()))); - msg.set_optional_string_opt(None::<&str>); + msg.clear_optional_string(); msg.optional_string_mut().or_default(); assert_that!(msg.optional_string(), eq("")); assert_that!(msg.optional_string_opt(), eq(Optional::Set("".into()))); @@ -1121,3 +1117,16 @@ fn test_group() { assert_that!(m.optionalgroup_opt().is_set(), eq(true)); assert_that!(m.optionalgroup().a(), eq(7)); } + +#[test] +fn test_submsg_setter() { + use TestAllTypes_::*; + + let mut nested = NestedMessage::new(); + nested.set_bb(7); + + let mut parent = TestAllTypes::new(); + parent.set_optional_nested_message(nested); + + assert_that!(parent.optional_nested_message().bb(), eq(7)); +} diff --git a/src/google/protobuf/compiler/rust/accessors/singular_message.cc b/src/google/protobuf/compiler/rust/accessors/singular_message.cc index 18adf8d935..c8e44b99af 100644 --- a/src/google/protobuf/compiler/rust/accessors/singular_message.cc +++ b/src/google/protobuf/compiler/rust/accessors/singular_message.cc @@ -26,6 +26,7 @@ void SingularMessage::InMsgImpl(Context& ctx, const FieldDescriptor& field, std::string msg_type = RsTypePath(ctx, field); ctx.Emit({{"msg_type", msg_type}, {"field", RsSafeName(field.name())}, + {"raw_field_name", field.name()}, {"view_lifetime", ViewLifetime(accessor_case)}, {"view_self", ViewReceiver(accessor_case)}, {"getter_thunk", ThunkName(ctx, field, "get")}, @@ -72,7 +73,7 @@ void SingularMessage::InMsgImpl(Context& ctx, const FieldDescriptor& field, return; } ctx.Emit({}, R"rs( - pub fn $field$_mut(&mut self) + pub fn $raw_field_name$_mut(&mut self) -> $pb$::FieldEntry<'_, $msg_type$> { static VTABLE: $pbr$::MessageVTable = $pbr$::MessageVTable::new($pbi$::Private, @@ -92,7 +93,7 @@ void SingularMessage::InMsgImpl(Context& ctx, const FieldDescriptor& field, {"getter_opt", [&] { ctx.Emit({}, R"rs( - pub fn $field$_opt($view_self$) -> + pub fn $raw_field_name$_opt($view_self$) -> $pb$::Optional<$msg_type$View<$view_lifetime$>> { let view = self.$field$(); $pb$::Optional::new(view, unsafe { @@ -101,13 +102,22 @@ void SingularMessage::InMsgImpl(Context& ctx, const FieldDescriptor& field, } )rs"); }}, + {"setter", + [&] { + if (accessor_case == AccessorCase::VIEW) return; + ctx.Emit(R"rs( + pub fn set_$raw_field_name$(&mut self, val: impl $pb$::SettableValue<$msg_type$>) { + //~ TODO: Optimize this to not go through the + //~ FieldEntry. + self.$raw_field_name$_mut().set(val); + } + )rs"); + }}, {"clearer", [&] { - if (accessor_case == AccessorCase::VIEW) { - return; - } + if (accessor_case == AccessorCase::VIEW) return; ctx.Emit({}, R"rs( - pub fn $field$_clear(&mut self) { + pub fn clear_$raw_field_name$(&mut self) { unsafe { $clearer_thunk$(self.raw_msg()) } })rs"); }}}, @@ -115,6 +125,7 @@ void SingularMessage::InMsgImpl(Context& ctx, const FieldDescriptor& field, $getter$ $getter_mut$ $getter_opt$ + $setter$ $clearer$ )rs"); } diff --git a/src/google/protobuf/compiler/rust/accessors/singular_scalar.cc b/src/google/protobuf/compiler/rust/accessors/singular_scalar.cc index b04cca8926..3d552ed222 100644 --- a/src/google/protobuf/compiler/rust/accessors/singular_scalar.cc +++ b/src/google/protobuf/compiler/rust/accessors/singular_scalar.cc @@ -55,26 +55,21 @@ void SingularScalar::InMsgImpl(Context& ctx, const FieldDescriptor& field, }}, {"setter", [&] { + if (accessor_case == AccessorCase::VIEW) return; ctx.Emit({}, R"rs( pub fn set_$raw_field_name$(&mut self, val: $Scalar$) { unsafe { $setter_thunk$(self.raw_msg(), val) } } )rs"); }}, - {"setter_opt", + {"clearer", [&] { - if (field.has_presence()) { - ctx.Emit({}, R"rs( - pub fn set_$raw_field_name$_opt(&mut self, val: Option<$Scalar$>) { - unsafe { - match val { - Some(val) => $setter_thunk$(self.raw_msg(), val), - None => $clearer_thunk$(self.raw_msg()), - } - } - } - )rs"); - } + if (accessor_case == AccessorCase::VIEW) return; + if (!field.has_presence()) return; + ctx.Emit({}, R"rs( + pub fn clear_$raw_field_name$(&mut self) { + unsafe { $clearer_thunk$(self.raw_msg()) } + })rs"); }}, {"getter_thunk", ThunkName(ctx, field, "get")}, {"setter_thunk", ThunkName(ctx, field, "set")}, @@ -153,7 +148,7 @@ void SingularScalar::InMsgImpl(Context& ctx, const FieldDescriptor& field, $getter$ $getter_opt$ $setter$ - $setter_opt$ + $clearer$ $vtable$ $getter_mut$ )rs"); @@ -176,8 +171,7 @@ void SingularScalar::InExternC(Context& ctx, {"with_presence_fields_thunks", [&] { if (field.has_presence()) { - ctx.Emit( - R"rs( + ctx.Emit(R"rs( fn $hazzer_thunk$(raw_msg: $pbi$::RawMessage) -> bool; fn $clearer_thunk$(raw_msg: $pbi$::RawMessage); )rs"); @@ -214,14 +208,13 @@ void SingularScalar::InThunkCc(Context& ctx, {"clearer_thunk", ThunkName(ctx, field, "clear")}, {"with_presence_fields_thunks", [&] { - if (field.has_presence()) { - ctx.Emit(R"cc( - bool $hazzer_thunk$($QualifiedMsg$* msg) { - return msg->has_$field$(); - } - void $clearer_thunk$($QualifiedMsg$* msg) { msg->clear_$field$(); } - )cc"); - } + if (!field.has_presence()) return; + ctx.Emit(R"cc( + bool $hazzer_thunk$($QualifiedMsg$* msg) { + return msg->has_$field$(); + } + void $clearer_thunk$($QualifiedMsg$* msg) { msg->clear_$field$(); } + )cc"); }}}, R"cc( $with_presence_fields_thunks$; diff --git a/src/google/protobuf/compiler/rust/accessors/singular_string.cc b/src/google/protobuf/compiler/rust/accessors/singular_string.cc index c4363448ca..b654edb604 100644 --- a/src/google/protobuf/compiler/rust/accessors/singular_string.cc +++ b/src/google/protobuf/compiler/rust/accessors/singular_string.cc @@ -81,27 +81,21 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field, [&] { if (accessor_case == AccessorCase::VIEW) return; ctx.Emit(R"rs( - pub fn set_$raw_field_name$(&mut self, val: impl $pb$::SettableValue<$proxied_type$>) { - //~ TODO: Optimize this to not go through the - //~ FieldEntry. - self.$raw_field_name$_mut().set(val); - } - )rs"); + pub fn set_$raw_field_name$(&mut self, val: impl $pb$::SettableValue<$proxied_type$>) { + //~ TODO: Optimize this to not go through the + //~ FieldEntry. + self.$raw_field_name$_mut().set(val); + } + )rs"); }}, - {"setter_opt", + {"clearer", [&] { if (accessor_case == AccessorCase::VIEW) return; if (!field.has_presence()) return; - ctx.Emit(R"rs( - pub fn set_$raw_field_name$_opt(&mut self, val: Option>) { - //~ TODO: Optimize this to not go through the - //~ FieldEntry. - match val { - Some(val) => self.$raw_field_name$_mut().set(val), - None => self.$raw_field_name$_mut().clear(), - } - } - )rs"); + ctx.Emit({}, R"rs( + pub fn clear_$raw_field_name$(&mut self) { + unsafe { $clearer_thunk$(self.raw_msg()) } + })rs"); }}, {"vtable_name", VTableName(field)}, {"vtable", @@ -176,7 +170,7 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field, $getter$ $getter_opt$ $setter$ - $setter_opt$ + $clearer$ $vtable$ $field_mutator_getter$ )rs");