Initial skeleton towards oneof's Mut accessor, and clarify oneof the View accessor to use View<> instead of the raw primitive type. PiperOrigin-RevId: 565156563
diff --git a/rust/test/shared/accessors_proto3_test.rs b/rust/test/shared/accessors_proto3_test.rs index a73d666..84d58a9 100644 --- a/rust/test/shared/accessors_proto3_test.rs +++ b/rust/test/shared/accessors_proto3_test.rs
@@ -184,21 +184,23 @@ #[test] fn test_oneof_accessors() { + use TestAllTypes_::OneofField::*; + let mut msg = TestAllTypes::new(); - assert_eq!(msg.oneof_field(), TestAllTypes_::OneofField::not_set); + assert!(matches!(msg.oneof_field(), not_set(_))); msg.oneof_uint32_set(Some(7)); assert_eq!(msg.oneof_uint32_opt(), Optional::Set(7)); - assert_eq!(msg.oneof_field(), TestAllTypes_::OneofField::OneofUint32(7)); + assert!(matches!(msg.oneof_field(), OneofUint32(7))); msg.oneof_uint32_set(None); assert_eq!(msg.oneof_uint32_opt(), Optional::Unset(0)); - assert_eq!(msg.oneof_field(), TestAllTypes_::OneofField::not_set); + assert!(matches!(msg.oneof_field(), not_set(_))); msg.oneof_uint32_set(Some(7)); msg.oneof_bytes_mut().set(b""); assert_eq!(msg.oneof_uint32_opt(), Optional::Unset(0)); // This should show it set to the OneofBytes but its not supported yet. - assert_eq!(msg.oneof_field(), TestAllTypes_::OneofField::not_set); + assert!(matches!(msg.oneof_field(), not_set(_))); }
diff --git a/rust/test/shared/accessors_test.rs b/rust/test/shared/accessors_test.rs index 3eda882..2221370 100644 --- a/rust/test/shared/accessors_test.rs +++ b/rust/test/shared/accessors_test.rs
@@ -373,20 +373,22 @@ #[test] fn test_oneof_accessors() { + use TestAllTypes_::OneofField::*; + let mut msg = TestAllTypes::new(); - assert_eq!(msg.oneof_field(), TestAllTypes_::OneofField::not_set); + assert!(matches!(msg.oneof_field(), not_set(_))); msg.oneof_uint32_set(Some(7)); assert_eq!(msg.oneof_uint32_opt(), Optional::Set(7)); - assert_eq!(msg.oneof_field(), TestAllTypes_::OneofField::OneofUint32(7)); + assert!(matches!(msg.oneof_field(), OneofUint32(7))); msg.oneof_uint32_set(None); assert_eq!(msg.oneof_uint32_opt(), Optional::Unset(0)); - assert_eq!(msg.oneof_field(), TestAllTypes_::OneofField::not_set); + assert!(matches!(msg.oneof_field(), not_set(_))); msg.oneof_uint32_set(Some(7)); msg.oneof_bytes_mut().set(b""); assert_eq!(msg.oneof_uint32_opt(), Optional::Unset(0)); // This should show it set to the OneofBytes but its not supported yet. - assert_eq!(msg.oneof_field(), TestAllTypes_::OneofField::not_set); + assert!(matches!(msg.oneof_field(), not_set(_))); }
diff --git a/src/google/protobuf/compiler/rust/oneof.cc b/src/google/protobuf/compiler/rust/oneof.cc index 74dd691..e779215 100644 --- a/src/google/protobuf/compiler/rust/oneof.cc +++ b/src/google/protobuf/compiler/rust/oneof.cc
@@ -44,20 +44,26 @@ // This will emit as the exposed API: // pub mod SomeMsg_ { // // The 'view' struct (no suffix on the name) -// pub enum SomeOneof { -// FieldA(i32) = 7, -// FieldB(u32) = 9, +// pub enum SomeOneof<'msg> { +// FieldA(View<'msg, i32>) = 7, +// FieldB(View<'msg, u32>) = 9, +// not_set = 0 +// } +// pub enum SomeOneofMut<'msg> { +// FieldA(Mut<'msg, i32>) = 7, +// FieldB(Mut<'msg, u32>) = 9, // not_set = 0 // } // } // impl SomeMsg { // pub fn some_oneof() -> SomeOneof {...} +// pub fn some_oneof_mut() -> SomeOneofMut {...} + // } // // An additional "Case" enum which just reflects the corresponding slot numbers // is emitted for usage with the FFI (exactly matching the Case struct that both -// cpp and upb generate). This enum is pub(super) which makes it private to the -// codegen since its inside an inner mod: +// cpp and upb generate). // // #[repr(C)] pub(super) enum SomeOneofCase { // FieldA = 7, @@ -74,10 +80,14 @@ return ToCamelCase(desc.name()); } +std::string oneofMutEnumRsName(const OneofDescriptor& desc) { + return ToCamelCase(desc.name()) + "Mut"; +} + std::string oneofCaseEnumName(const OneofDescriptor& desc) { // Note: This is the name used for the cpp Case enum, we use it for both // the Rust Case enum as well as for the cpp case enum in the cpp thunk. - return oneofViewEnumRsName(desc) + "Case"; + return ToCamelCase(desc.name()) + "Case"; } // TODO(b/285309334): Promote up to naming.h once all types can be spelled. @@ -94,6 +104,18 @@ } } +std::string RsTypeNameView(const FieldDescriptor& desc) { + std::string type = RsTypeName(desc); + if (type.empty()) return ""; + return "View<'msg, " + type + ">"; +} + +std::string RsTypeNameMut(const FieldDescriptor& desc) { + std::string type = RsTypeName(desc); + if (type.empty()) return ""; + return "Mut<'msg, " + type + ">"; +} + } // namespace void GenerateOneofDefinition(Context<OneofDescriptor> oneof) { @@ -101,31 +123,62 @@ oneof.Emit( {{"view_enum_name", oneofViewEnumRsName(desc)}, + {"mut_enum_name", oneofMutEnumRsName(desc)}, {"view_fields", [&] { for (int i = 0; i < desc.field_count(); ++i) { const auto& field = desc.field(i); - std::string rs_type = RsTypeName(*field); + std::string rs_type = RsTypeNameView(*field); if (rs_type.empty()) { continue; } oneof.Emit({{"name", ToCamelCase(field->name())}, {"type", rs_type}, {"number", std::to_string(field->number())}}, - R"rs($name$($type$) = $number$, + R"rs($name$($pb$::$type$) = $number$, + )rs"); + } + }}, + {"mut_fields", + [&] { + for (int i = 0; i < desc.field_count(); ++i) { + const auto& field = desc.field(i); + std::string rs_type = RsTypeNameMut(*field); + if (rs_type.empty()) { + continue; + } + oneof.Emit({{"name", ToCamelCase(field->name())}, + {"type", rs_type}, + {"number", std::to_string(field->number())}}, + R"rs($name$($pb$::$type$) = $number$, )rs"); } }}}, // TODO(b/297342638): Revisit if isize is the optimal repr for this enum. + // TODO(b/285309334): not_set currently has phantom data just to avoid the + // lifetime on the enum breaking compilation if there are zero supported + // fields on it (e.g. if the oneof only has Messages inside). R"rs( #[non_exhaustive] - #[derive(Debug, PartialEq)] + #[derive(Debug)] + #[allow(dead_code)] #[repr(isize)] - pub enum $view_enum_name$ { + pub enum $view_enum_name$<'msg> { $view_fields$ #[allow(non_camel_case_types)] - not_set = 0 + not_set(std::marker::PhantomData<&'msg ()>) = 0 + } + + #[non_exhaustive] + #[derive(Debug)] + #[allow(dead_code)] + #[repr(isize)] + pub enum $mut_enum_name$<'msg> { + $mut_fields$ + + #[allow(non_camel_case_types)] + not_set(std::marker::PhantomData<&'msg ()>) = 0 } )rs"); @@ -145,7 +198,7 @@ }}}, R"rs( #[repr(C)] - #[derive(Debug, Copy, Clone, PartialEq)] + #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(super) enum $case_enum_name$ { $cases$ @@ -162,12 +215,13 @@ oneof.Emit( {{"oneof_name", desc.name()}, {"view_enum_name", oneofViewEnumRsName(desc)}, + {"mut_enum_name", oneofMutEnumRsName(desc)}, {"case_enum_name", oneofCaseEnumName(desc)}, - {"cases", + {"view_cases", [&] { for (int i = 0; i < desc.field_count(); ++i) { const auto& field = desc.field(i); - std::string rs_type = RsTypeName(*field); + std::string rs_type = RsTypeNameView(*field); if (rs_type.empty()) { continue; } @@ -181,12 +235,39 @@ )rs"); } }}, + {"mut_cases", + [&] { + for (int i = 0; i < desc.field_count(); ++i) { + const auto& field = desc.field(i); + std::string rs_type = RsTypeNameMut(*field); + if (rs_type.empty()) { + continue; + } + // TODO(b/285309334, 285309449): Uncomment this to allow mut once + // _mut() on singular fields with presence is implemented. + /*oneof.Emit({ + {"case", ToCamelCase(field->name())}, + {"rs_getter", field->name() + "_mut"}, + {"type", rs_type}, + }, + R"rs($Msg$_::$case_enum_name$::$case$ => + $Msg$_::$mut_enum_name$::$case$(self.$rs_getter$()), )rs"); + */ + } + }}, {"case_thunk", Thunk(oneof, "case")}}, R"rs( pub fn r#$oneof_name$(&self) -> $Msg$_::$view_enum_name$ { match unsafe { $case_thunk$(self.inner.msg) } { - $cases$ - _ => $Msg$_::$view_enum_name$::not_set + $view_cases$ + _ => $Msg$_::$view_enum_name$::not_set(std::marker::PhantomData) + } + } + + pub fn r#$oneof_name$_mut(&mut self) -> $Msg$_::$mut_enum_name$ { + match unsafe { $case_thunk$(self.inner.msg) } { + $mut_cases$ + _ => $Msg$_::$mut_enum_name$::not_set(std::marker::PhantomData) } }