Use `self` for all methods on views, return `'msg`

View is Copy, and so it can preserve the lifetime of itself
when accessing any fields.

PiperOrigin-RevId: 599323122
pull/15470/head
Alyssa Haroldsen 1 year ago committed by Copybara-Service
parent fe6a601598
commit ab8b762941
  1. 4
      rust/test/nested.proto
  2. 34
      rust/test/shared/simple_nested_test.rs
  3. 1
      src/google/protobuf/compiler/rust/BUILD.bazel
  4. 35
      src/google/protobuf/compiler/rust/accessors/accessor_case.cc
  5. 22
      src/google/protobuf/compiler/rust/accessors/accessor_case.h
  6. 1
      src/google/protobuf/compiler/rust/accessors/accessors.cc
  7. 6
      src/google/protobuf/compiler/rust/accessors/repeated_field.cc
  8. 4
      src/google/protobuf/compiler/rust/accessors/singular_message.cc
  9. 5
      src/google/protobuf/compiler/rust/accessors/singular_scalar.cc
  10. 7
      src/google/protobuf/compiler/rust/accessors/singular_string.cc
  11. 10
      src/google/protobuf/compiler/rust/message.cc

@ -35,8 +35,10 @@ message Outer {
optional bool bool = 13;
optional string string = 14;
optional bytes bytes = 15;
optional InnerSubMsg innersubmsg = 16;
optional InnerSubMsg inner_submsg = 16;
optional InnerEnum inner_enum = 17;
repeated int32 repeated_int32 = 18 [packed = true];
repeated InnerSubMsg repeated_inner_submsg = 19;
message SuperInner {
message DuperInner {

@ -50,10 +50,33 @@ fn test_nested_views() {
assert_that!(inner_msg.bool(), eq(false));
assert_that!(*inner_msg.string().as_bytes(), empty());
assert_that!(*inner_msg.bytes(), empty());
assert_that!(inner_msg.innersubmsg().flag(), eq(false));
assert_that!(inner_msg.inner_submsg().flag(), eq(false));
assert_that!(inner_msg.inner_enum(), eq(InnerEnum::Unspecified));
}
#[test]
fn test_nested_view_lifetimes() {
// Ensure that views have the lifetime of the first layer of borrow, and don't
// create intermediate borrows through nested accessors.
let outer_msg = Outer::new();
let string = outer_msg.inner().string();
assert_that!(string, eq(""));
let bytes = outer_msg.inner().bytes();
assert_that!(bytes, eq(b""));
let inner_submsg = outer_msg.inner().inner_submsg();
assert_that!(inner_submsg.flag(), eq(false));
let repeated_int32 = outer_msg.inner().repeated_int32();
assert_that!(repeated_int32, empty());
let repeated_inner_submsg = outer_msg.inner().repeated_inner_submsg();
assert_that!(repeated_inner_submsg, empty());
}
#[test]
fn test_nested_muts() {
// Covers the setting of a mut and the following assertion
@ -129,6 +152,10 @@ fn test_recursive_view() {
assert_that!(rec.rec().num(), eq(0));
assert_that!(rec.rec().rec().num(), eq(0)); // turtles all the way down...
assert_that!(rec.rec().rec().rec().num(), eq(0)); // ... ad infinitum
// Test that intermediate borrows are not created.
let nested = rec.rec().rec().rec();
assert_that!(nested.num(), eq(0));
}
#[test]
@ -145,4 +172,9 @@ fn test_recursive_mut() {
assert_that!(rec.num(), eq(0));
assert_that!(rec.rec().rec().num(), eq(0));
assert_that!(rec.rec().rec().rec().rec().num(), eq(1));
// This fails since `RecursiveMut` has `&mut self` methods.
// See b/314989133.
// let nested = rec.rec_mut().rec_mut().rec_mut();
// assert_that!(nested.num(), eq(0));
}

@ -101,6 +101,7 @@ cc_library(
cc_library(
name = "accessors",
srcs = [
"accessors/accessor_case.cc",
"accessors/accessors.cc",
"accessors/map.cc",
"accessors/repeated_field.cc",

@ -0,0 +1,35 @@
#include "google/protobuf/compiler/rust/accessors/accessor_case.h"
#include "absl/strings/string_view.h"
namespace google {
namespace protobuf {
namespace compiler {
namespace rust {
absl::string_view ViewReceiver(AccessorCase accessor_case) {
switch (accessor_case) {
case AccessorCase::VIEW:
return "self";
case AccessorCase::OWNED:
case AccessorCase::MUT:
return "&self";
}
return "";
}
absl::string_view ViewLifetime(AccessorCase accessor_case) {
switch (accessor_case) {
case AccessorCase::VIEW:
return "'msg";
case AccessorCase::OWNED:
case AccessorCase::MUT:
return "'_";
}
return "";
}
} // namespace rust
} // namespace compiler
} // namespace protobuf
} // namespace google

@ -8,6 +8,13 @@
#ifndef GOOGLE_PROTOBUF_COMPILER_RUST_ACCESSORS_ACCESSOR_CASE_H__
#define GOOGLE_PROTOBUF_COMPILER_RUST_ACCESSORS_ACCESSOR_CASE_H__
#include "absl/strings/string_view.h"
namespace google {
namespace protobuf {
namespace compiler {
namespace rust {
// GenerateAccessorMsgImpl is reused for all three types of $Msg$, $Msg$Mut and
// $Msg$View; this enum signifies which case we are handling so corresponding
// adjustments can be made (for example: to not emit any mutation accessors
@ -18,4 +25,19 @@ enum class AccessorCase {
VIEW,
};
// Returns the `self` receiver type for a subfield view accessor.
absl::string_view ViewReceiver(AccessorCase accessor_case);
// Returns the lifetime of a subfield view accessor.
// Views are Copy, and so the full `'msg` lifetime can be used.
// Any `&self` or `&mut self` accessors need to use the lifetime of that
// borrow, which is referenced via `'_`.
// See b/314989133 for _mut accessors.
absl::string_view ViewLifetime(AccessorCase accessor_case);
} // namespace rust
} // namespace compiler
} // namespace protobuf
} // namespace google
#endif // GOOGLE_PROTOBUF_COMPILER_RUST_ACCESSORS_ACCESSOR_CASE_H__

@ -10,6 +10,7 @@
#include <memory>
#include "absl/log/absl_log.h"
#include "google/protobuf/compiler/rust/accessors/accessor_case.h"
#include "google/protobuf/compiler/rust/accessors/accessor_generator.h"
#include "google/protobuf/compiler/rust/context.h"
#include "google/protobuf/descriptor.h"

@ -24,13 +24,15 @@ void RepeatedField::InMsgImpl(Context& ctx, const FieldDescriptor& field,
AccessorCase accessor_case) const {
ctx.Emit({{"field", RsSafeName(field.name())},
{"RsType", RsTypePath(ctx, field)},
{"view_lifetime", ViewLifetime(accessor_case)},
{"view_self", ViewReceiver(accessor_case)},
{"getter_thunk", ThunkName(ctx, field, "get")},
{"getter_mut_thunk", ThunkName(ctx, field, "get_mut")},
{"getter",
[&] {
if (ctx.is_upb()) {
ctx.Emit({}, R"rs(
pub fn $field$(&self) -> $pb$::RepeatedView<'_, $RsType$> {
pub fn $field$($view_self$) -> $pb$::RepeatedView<$view_lifetime$, $RsType$> {
unsafe {
$getter_thunk$(
self.raw_msg(),
@ -46,7 +48,7 @@ void RepeatedField::InMsgImpl(Context& ctx, const FieldDescriptor& field,
)rs");
} else {
ctx.Emit({}, R"rs(
pub fn $field$(&self) -> $pb$::RepeatedView<'_, $RsType$> {
pub fn $field$($view_self$) -> $pb$::RepeatedView<$view_lifetime$, $RsType$> {
unsafe {
$pb$::RepeatedView::from_raw(
$pbi$::Private,

@ -26,6 +26,8 @@ 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())},
{"view_lifetime", ViewLifetime(accessor_case)},
{"view_self", ViewReceiver(accessor_case)},
{"getter_thunk", ThunkName(ctx, field, "get")},
{"getter_mut_thunk", ThunkName(ctx, field, "get_mut")},
{"clearer_thunk", ThunkName(ctx, field, "clear")},
@ -58,7 +60,7 @@ void SingularMessage::InMsgImpl(Context& ctx, const FieldDescriptor& field,
{"getter",
[&] {
ctx.Emit({}, R"rs(
pub fn $field$(&self) -> $msg_type$View {
pub fn $field$($view_self$) -> $msg_type$View<$view_lifetime$> {
$getter_body$
}
)rs");

@ -26,13 +26,14 @@ void SingularScalar::InMsgImpl(Context& ctx, const FieldDescriptor& field,
ctx.Emit(
{
{"field", RsSafeName(field.name())},
{"view_self", ViewReceiver(accessor_case)},
{"Scalar", RsTypePath(ctx, field)},
{"hazzer_thunk", ThunkName(ctx, field, "has")},
{"default_value", DefaultValue(ctx, field)},
{"getter",
[&] {
ctx.Emit({}, R"rs(
pub fn $field$(&self) -> $Scalar$ {
pub fn $field$($view_self$) -> $Scalar$ {
unsafe { $getter_thunk$(self.raw_msg()) }
}
)rs");
@ -42,7 +43,7 @@ void SingularScalar::InMsgImpl(Context& ctx, const FieldDescriptor& field,
if (!field.is_optional()) return;
if (!field.has_presence()) return;
ctx.Emit({}, R"rs(
pub fn $field$_opt(&self) -> $pb$::Optional<$Scalar$> {
pub fn $field$_opt($view_self$) -> $pb$::Optional<$Scalar$> {
if !unsafe { $hazzer_thunk$(self.raw_msg()) } {
return $pb$::Optional::Unset($default_value$);
}

@ -27,6 +27,7 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field,
std::string getter_thunk = ThunkName(ctx, field, "get");
std::string setter_thunk = ThunkName(ctx, field, "set");
std::string proxied_type = RsTypePath(ctx, field);
auto transform_view = [&] {
if (field.type() == FieldDescriptor::TYPE_STRING) {
ctx.Emit(R"rs(
@ -45,6 +46,8 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field,
{"setter_thunk", setter_thunk},
{"proxied_type", proxied_type},
{"transform_view", transform_view},
{"view_lifetime", ViewLifetime(accessor_case)},
{"view_self", ViewReceiver(accessor_case)},
{"field_optional_getter",
[&] {
if (!field.is_optional()) return;
@ -53,7 +56,7 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field,
{"getter_thunk", getter_thunk},
{"transform_view", transform_view}},
R"rs(
pub fn $field$_opt(&self) -> $pb$::Optional<&$proxied_type$> {
pub fn $field$_opt($view_self$) -> $pb$::Optional<&$view_lifetime$ $proxied_type$> {
let view = unsafe { $getter_thunk$(self.raw_msg()).as_ref() };
$pb$::Optional::new(
$transform_view$ ,
@ -143,7 +146,7 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field,
}},
},
R"rs(
pub fn $field$(&self) -> &$proxied_type$ {
pub fn $field$($view_self$) -> &$view_lifetime$ $proxied_type$ {
let view = unsafe { $getter_thunk$(self.raw_msg()).as_ref() };
$transform_view$
}

@ -549,7 +549,7 @@ void GenerateRs(Context& ctx, const Descriptor& msg) {
}
#[allow(dead_code)]
impl<'a> $Msg$View<'a> {
impl<'msg> $Msg$View<'msg> {
#[doc(hidden)]
pub fn new(_private: $pbi$::Private, msg: $pbi$::RawMessage) -> Self {
Self { msg, _phantom: std::marker::PhantomData }
@ -634,11 +634,11 @@ void GenerateRs(Context& ctx, const Descriptor& msg) {
}
#[allow(dead_code)]
impl<'a> $Msg$Mut<'a> {
impl<'msg> $Msg$Mut<'msg> {
#[doc(hidden)]
pub fn from_parent(
_private: $pbi$::Private,
parent: $pbr$::MutatorMessageRef<'a>,
parent: $pbr$::MutatorMessageRef<'msg>,
msg: $pbi$::RawMessage)
-> Self {
Self {
@ -648,7 +648,7 @@ void GenerateRs(Context& ctx, const Descriptor& msg) {
}
#[doc(hidden)]
pub fn new(_private: $pbi$::Private, msg: &'a mut $pbr$::MessageInner) -> Self {
pub fn new(_private: $pbi$::Private, msg: &'msg mut $pbr$::MessageInner) -> Self {
Self{ inner: $pbr$::MutatorMessageRef::new(_private, msg) }
}
@ -656,7 +656,7 @@ void GenerateRs(Context& ctx, const Descriptor& msg) {
self.inner.msg()
}
fn as_mutator_message_ref(&mut self) -> $pbr$::MutatorMessageRef<'a> {
fn as_mutator_message_ref(&mut self) -> $pbr$::MutatorMessageRef<'msg> {
self.inner
}

Loading…
Cancel
Save