Introduce preliminary submessage_mut semantics
We've had access to views for submessages for a while:
If you hit some_message.submsg().some_int(), you'll get a view for that int.
Until now, there hasn't been a way to get some_message.submsg_mut(), so we introduce the mutational pathway here.
We haven't added fully-functioning mutation, but this is a step towards that goal.
subview was inaccurate, so I've refactored and renamed: { accessor_fns_for_views, accessor_fns_for_muts }.
PiperOrigin-RevId: 581984371
diff --git a/rust/cpp.rs b/rust/cpp.rs
index 7c4998f..d1f670b 100644
--- a/rust/cpp.rs
+++ b/rust/cpp.rs
@@ -171,6 +171,14 @@
MutatorMessageRef { msg: msg.msg, _phantom: PhantomData }
}
+ pub fn from_parent(
+ _private: Private,
+ _parent_msg: &'msg mut MessageInner,
+ message_field_ptr: RawMessage,
+ ) -> Self {
+ MutatorMessageRef { msg: message_field_ptr, _phantom: PhantomData }
+ }
+
pub fn msg(&self) -> RawMessage {
self.msg
}
diff --git a/rust/test/nested.proto b/rust/test/nested.proto
index 80b0c61..6663a5a 100644
--- a/rust/test/nested.proto
+++ b/rust/test/nested.proto
@@ -11,8 +11,19 @@
message Outer {
message Inner {
- optional int32 num = 1;
- optional bool boolean = 2;
+ optional double double = 1;
+ optional float float = 2;
+ optional int32 int32 = 3;
+ optional int64 int64 = 4;
+ optional uint32 uint32 = 5;
+ optional uint64 uint64 = 6;
+ optional sint32 sint32 = 7;
+ optional sint64 sint64 = 8;
+ optional fixed32 fixed32 = 9;
+ optional fixed64 fixed64 = 10;
+ optional sfixed32 sfixed32 = 11;
+ optional sfixed64 sfixed64 = 12;
+ optional bool bool = 13;
message SuperInner {
message DuperInner {
diff --git a/rust/test/shared/BUILD b/rust/test/shared/BUILD
index b5864a3..06f5633 100644
--- a/rust/test/shared/BUILD
+++ b/rust/test/shared/BUILD
@@ -261,7 +261,10 @@
# TODO: Enable testing on arm once we support sanitizers for Rust on Arm.
"not_build:arm",
],
- deps = ["//rust/test:nested_cc_rust_proto"],
+ deps = [
+ "@crate_index//:googletest",
+ "//rust/test:nested_cc_rust_proto",
+ ],
)
rust_test(
@@ -271,7 +274,10 @@
# TODO: Enable testing on arm once we support sanitizers for Rust on Arm.
"not_build:arm",
],
- deps = ["//rust/test:nested_upb_rust_proto"],
+ deps = [
+ "@crate_index//:googletest",
+ "//rust/test:nested_upb_rust_proto",
+ ],
)
rust_test(
diff --git a/rust/test/shared/accessors_test.rs b/rust/test/shared/accessors_test.rs
index e5949ff..70c8cc0 100644
--- a/rust/test/shared/accessors_test.rs
+++ b/rust/test/shared/accessors_test.rs
@@ -667,11 +667,16 @@
#[test]
fn test_singular_msg_field() {
- use crate::TestAllTypes_::NestedMessageView;
- let msg = TestAllTypes::new();
- // TODO: fetch the inner integer `bb`
- // call should look like msg.optional_nested_message().bb()
- let _msg: NestedMessageView = msg.optional_nested_message();
+ use crate::TestAllTypes_::{NestedMessageMut, NestedMessageView};
+
+ let mut msg = TestAllTypes::new();
+ let msg_view: NestedMessageView = msg.optional_nested_message();
+ // testing reading an int inside a view
+ assert_that!(msg_view.bb(), eq(0));
+
+ let msg_mut: NestedMessageMut = msg.optional_nested_message_mut();
+ // test reading an int inside a mut
+ assert_that!(msg_mut.bb(), eq(0));
}
#[test]
diff --git a/rust/test/shared/simple_nested_test.rs b/rust/test/shared/simple_nested_test.rs
index daa654c..b5d6ba3 100644
--- a/rust/test/shared/simple_nested_test.rs
+++ b/rust/test/shared/simple_nested_test.rs
@@ -5,13 +5,49 @@
// license that can be found in the LICENSE file or at
// https://developers.google.com/open-source/licenses/bsd
+use googletest::prelude::*;
use nested_proto::nest::Outer;
+use nested_proto::nest::Outer_::InnerMut;
+use nested_proto::nest::Outer_::InnerView;
#[test]
-fn test_simple_nested_proto() {
+fn test_nested_views() {
let outer_msg = Outer::new();
- assert_eq!(outer_msg.inner().num(), 0);
- assert!(!outer_msg.inner().boolean());
+ let inner_msg: InnerView<'_> = outer_msg.inner();
+ assert_that!(inner_msg.double(), eq(0.0));
+ assert_that!(inner_msg.float(), eq(0.0));
+ assert_that!(inner_msg.int32(), eq(0));
+ assert_that!(inner_msg.int64(), eq(0));
+ assert_that!(inner_msg.uint32(), eq(0));
+ assert_that!(inner_msg.uint64(), eq(0));
+ assert_that!(inner_msg.sint32(), eq(0));
+ assert_that!(inner_msg.sint64(), eq(0));
+ assert_that!(inner_msg.fixed32(), eq(0));
+ assert_that!(inner_msg.fixed64(), eq(0));
+ assert_that!(inner_msg.sfixed32(), eq(0));
+ assert_that!(inner_msg.sfixed64(), eq(0));
+ assert_that!(inner_msg.bool(), eq(false));
+}
+
+#[test]
+fn test_nested_muts() {
+ // TODO: add actual mutation logic, this just peeks at InnerMut at
+ // the moment
+ let mut outer_msg = Outer::new();
+ let inner_msg: InnerMut<'_> = outer_msg.inner_mut();
+ assert_that!(inner_msg.double(), eq(0.0));
+ assert_that!(inner_msg.float(), eq(0.0));
+ assert_that!(inner_msg.int32(), eq(0));
+ assert_that!(inner_msg.int64(), eq(0));
+ assert_that!(inner_msg.uint32(), eq(0));
+ assert_that!(inner_msg.uint64(), eq(0));
+ assert_that!(inner_msg.sint32(), eq(0));
+ assert_that!(inner_msg.sint64(), eq(0));
+ assert_that!(inner_msg.fixed32(), eq(0));
+ assert_that!(inner_msg.fixed64(), eq(0));
+ assert_that!(inner_msg.sfixed32(), eq(0));
+ assert_that!(inner_msg.sfixed64(), eq(0));
+ assert_that!(inner_msg.bool(), eq(false));
}
#[test]
diff --git a/rust/upb.rs b/rust/upb.rs
index dff07df..778dddc 100644
--- a/rust/upb.rs
+++ b/rust/upb.rs
@@ -261,6 +261,14 @@
MutatorMessageRef { msg: msg.msg, arena: &msg.arena }
}
+ pub fn from_parent(
+ _private: Private,
+ parent_msg: &'msg mut MessageInner,
+ message_field_ptr: RawMessage,
+ ) -> Self {
+ MutatorMessageRef { msg: message_field_ptr, arena: &parent_msg.arena }
+ }
+
pub fn msg(&self) -> RawMessage {
self.msg
}
diff --git a/src/google/protobuf/compiler/rust/accessors/singular_message.cc b/src/google/protobuf/compiler/rust/accessors/singular_message.cc
index 26f3358..972f3cd 100644
--- a/src/google/protobuf/compiler/rust/accessors/singular_message.cc
+++ b/src/google/protobuf/compiler/rust/accessors/singular_message.cc
@@ -22,42 +22,82 @@
auto prefix = "crate::" + GetCrateRelativeQualifiedPath(d);
- if (field.is_cpp()) {
- field.Emit({{"prefix", prefix},
- {"field", field.desc().name()},
- {"getter_thunk", Thunk(field, "get")}},
- R"rs(
- pub fn r#$field$(&self) -> $prefix$View {
- // For C++ kernel, getters automatically return the
- // default_instance if the field is unset.
- let submsg = unsafe { $getter_thunk$(self.inner.msg) };
- $prefix$View::new($pbi$::Private, submsg)
- }
- )rs");
- } else {
- field.Emit({{"prefix", prefix},
- {"field", field.desc().name()},
- {"getter_thunk", Thunk(field, "get")}},
- R"rs(
- pub fn r#$field$(&self) -> $prefix$View {
- let submsg = unsafe { $getter_thunk$(self.inner.msg) };
- // For upb, getters return null if the field is unset, so we need to
- // check for null and return the default instance manually. Note that
- // a null ptr received from upb manifests as Option::None
- match submsg {
+ field.Emit(
+ {
+ {"prefix", prefix},
+ {"field", field.desc().name()},
+ {"getter_thunk", Thunk(field, "get")},
+ {"clearer_thunk", Thunk(field, "clear")},
+ {
+ "view_body",
+ [&] {
+ if (field.is_upb()) {
+ field.Emit({}, R"rs(
+ let submsg = unsafe { $getter_thunk$(self.inner.msg) };
+ // For upb, getters return null if the field is unset, so we need
+ // to check for null and return the default instance manually.
+ // Note that a nullptr received from upb manifests as Option::None
+ match submsg {
// TODO:(b/304357029)
- None => $prefix$View::new($pbi$::Private, $pbr$::ScratchSpace::zeroed_block($pbi$::Private)),
+ None => $prefix$View::new($pbi$::Private,
+ $pbr$::ScratchSpace::zeroed_block($pbi$::Private)),
Some(field) => $prefix$View::new($pbi$::Private, field),
}
- }
)rs");
- }
+ } else {
+ field.Emit({}, R"rs(
+ // For C++ kernel, getters automatically return the
+ // default_instance if the field is unset.
+ let submsg = unsafe { $getter_thunk$(self.inner.msg) };
+ $prefix$View::new($pbi$::Private, submsg)
+ )rs");
+ }
+ },
+ },
+ {"submessage_mut",
+ [&] {
+ if (field.is_upb()) {
+ field.Emit({}, R"rs(
+ let submsg_opt = unsafe { $getter_thunk$(self.inner.msg) };
+ match submsg_opt {
+ None => {
+ $prefix$Mut::new($pbi$::Private,
+ &mut self.inner,
+ $pbr$::ScratchSpace::zeroed_block($pbi$::Private))
+ },
+ Some(submsg) => {
+ $prefix$Mut::new($pbi$::Private, &mut self.inner, submsg)
+ }
+ }
+ )rs");
+ } else {
+ field.Emit({}, R"rs(
+ let submsg = unsafe { $getter_thunk$(self.inner.msg) };
+ $prefix$Mut::new($pbi$::Private, &mut self.inner, submsg)
+ )rs");
+ }
+ }},
+ },
+ R"rs(
+ pub fn r#$field$(&self) -> $prefix$View {
+ $view_body$
+ }
+
+ pub fn $field$_mut(&mut self) -> $prefix$Mut {
+ $submessage_mut$
+ }
+
+ pub fn $field$_clear(&mut self) {
+ unsafe { $clearer_thunk$(self.inner.msg) }
+ }
+ )rs");
}
void SingularMessage::InExternC(Context<FieldDescriptor> field) const {
field.Emit(
{
{"getter_thunk", Thunk(field, "get")},
+ {"clearer_thunk", Thunk(field, "clear")},
{"ReturnType",
[&] {
if (field.is_cpp()) {
@@ -72,6 +112,7 @@
},
R"rs(
fn $getter_thunk$(raw_msg: $pbi$::RawMessage) -> $ReturnType$;
+ fn $clearer_thunk$(raw_msg: $pbi$::RawMessage);
)rs");
}
@@ -79,11 +120,13 @@
field.Emit({{"QualifiedMsg",
cpp::QualifiedClassName(field.desc().containing_type())},
{"getter_thunk", Thunk(field, "get")},
+ {"clearer_thunk", Thunk(field, "clear")},
{"field", cpp::FieldName(&field.desc())}},
R"cc(
const void* $getter_thunk$($QualifiedMsg$* msg) {
return static_cast<const void*>(&msg->$field$());
}
+ void $clearer_thunk$($QualifiedMsg$* msg) { msg->clear_$field$(); }
)cc");
}
diff --git a/src/google/protobuf/compiler/rust/generator.cc b/src/google/protobuf/compiler/rust/generator.cc
index 585b7aa..ecb6d56 100644
--- a/src/google/protobuf/compiler/rust/generator.cc
+++ b/src/google/protobuf/compiler/rust/generator.cc
@@ -98,6 +98,7 @@
pub use crate::$mod$::$Msg$;
// TODO Address use for imported crates
pub use crate::$mod$::$Msg$View;
+ pub use crate::$mod$::$Msg$Mut;
)rs");
}
}
diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc
index 5b4a2fd..cd01d7a 100644
--- a/src/google/protobuf/compiler/rust/message.cc
+++ b/src/google/protobuf/compiler/rust/message.cc
@@ -181,19 +181,33 @@
type == FieldDescriptor::TYPE_BOOL;
}
-void GenerateSubView(Context<FieldDescriptor> field) {
+void GetterForViewOrMut(Context<FieldDescriptor> field, bool is_mut) {
+ // If we're dealing with a Mut, the getter must be supplied self.inner.msg()
+ // whereas a View has to be supplied self.msg
field.Emit(
{
{"field", field.desc().name()},
{"getter_thunk", Thunk(field, "get")},
+ {"self", is_mut ? "self.inner.msg()" : "self.msg"},
{"Scalar", PrimitiveRsTypeName(field.desc())},
},
R"rs(
- pub fn r#$field$(&self) -> $Scalar$ { unsafe {
- $getter_thunk$(self.msg)
- } }
- )rs");
+ pub fn r#$field$(&self) -> $Scalar$ {
+ unsafe { $getter_thunk$($self$) }
+ }
+ )rs");
}
+
+void AccessorsForViewOrMut(Context<Descriptor> msg, bool is_mut) {
+ for (int i = 0; i < msg.desc().field_count(); ++i) {
+ auto field = msg.WithDesc(*msg.desc().field(i));
+ if (field.desc().is_repeated()) continue;
+ if (!IsSimpleScalar(field.desc().type())) continue;
+ GetterForViewOrMut(field, is_mut);
+ msg.printer().PrintRaw("\n");
+ }
+}
+
} // namespace
void GenerateRs(Context<Descriptor> msg) {
@@ -278,17 +292,8 @@
} // mod $Msg$_
)rs");
}},
- {"subviews",
- [&] {
- for (int i = 0; i < msg.desc().field_count(); ++i) {
- auto field = msg.WithDesc(*msg.desc().field(i));
- if (field.desc().is_repeated()) continue;
- if (!IsSimpleScalar(field.desc().type())) continue;
- GenerateSubView(field);
- msg.printer().PrintRaw("\n");
- }
- }},
- },
+ {"accessor_fns_for_views", [&] {AccessorsForViewOrMut(msg, false);}},
+ {"accessor_fns_for_muts", [&] {AccessorsForViewOrMut(msg, true);}}},
R"rs(
#[allow(non_camel_case_types)]
// TODO: Implement support for debug redaction
@@ -321,7 +326,7 @@
pub fn new(_private: $pbi$::Private, msg: $pbi$::RawMessage) -> Self {
Self { msg, _phantom: std::marker::PhantomData }
}
- $subviews$
+ $accessor_fns_for_views$
}
// SAFETY:
@@ -352,10 +357,25 @@
#[derive(Debug)]
#[allow(dead_code)]
+ #[allow(non_camel_case_types)]
pub struct $Msg$Mut<'a> {
inner: $pbr$::MutatorMessageRef<'a>,
}
+ impl<'a> $Msg$Mut<'a> {
+ #[doc(hidden)]
+ pub fn new(_private: $pbi$::Private,
+ parent: &'a mut $pbr$::MessageInner,
+ msg: $pbi$::RawMessage)
+ -> Self {
+ Self {
+ inner: $pbr$::MutatorMessageRef::from_parent(
+ $pbi$::Private, parent, msg)
+ }
+ }
+ $accessor_fns_for_muts$
+ }
+
// SAFETY:
// - `$Msg$Mut` does not perform any shared mutation.
// - `$Msg$Mut` is not `Send`, and so even in the presence of mutator
