Simplify the way C++ function is passed to `UntypedMapIterator::next_unchecked`

`UntypedMapIterator::next_unchecked` currently expects to be passed a pointer
to a C++ function that it can use to dereference an iterator. However, this is
awkward because it's not natural for this C++ function to have the same
signature for every map type. Maps with a message as value need a
`MapNodeSizeInfo`, but other map types do not. We are working around this by
sometimes passing an ignored placeholder constant, but this is messy.

This CL replaces the `extern "C"` functions with closures. This way, we can
capture the `MapNodeSizeInfo` in the closure in cases where we need it, but
otherwise we no longer need to pass around placeholder values.

(Note: `MapNodeSizeInfo` is going away soon, but this is still relevant because
it will likely need to be replaced by a message default instance pointer.)

PiperOrigin-RevId: 676438640
pull/18386/head
Adam Cozzette 2 months ago committed by Copybara-Service
parent 1f472f1db3
commit dc23fedbf3
  1. 11
      rust/cpp.rs
  2. 4
      rust/cpp_kernel/map.h
  3. 3
      src/google/protobuf/compiler/rust/enum.cc
  4. 4
      src/google/protobuf/compiler/rust/message.cc

@ -722,13 +722,11 @@ impl UntypedMapIterator {
pub unsafe fn next_unchecked<'a, K, V, FfiKey, FfiValue>(
&mut self,
iter_get_thunk: unsafe extern "C" fn(
iter_get_thunk: unsafe fn(
iter: &mut UntypedMapIterator,
size_info: MapNodeSizeInfo,
key: *mut FfiKey,
value: *mut FfiValue,
),
size_info: MapNodeSizeInfo,
from_ffi_key: impl FnOnce(FfiKey) -> View<'a, K>,
from_ffi_value: impl FnOnce(FfiValue) -> View<'a, V>,
) -> Option<(View<'a, K>, View<'a, V>)>
@ -746,7 +744,7 @@ impl UntypedMapIterator {
// - The iterator is not at the end (node is non-null).
// - `ffi_key` and `ffi_value` are not read (as uninit) as promised by the
// caller.
unsafe { (iter_get_thunk)(self, size_info, ffi_key.as_mut_ptr(), ffi_value.as_mut_ptr()) }
unsafe { (iter_get_thunk)(self, ffi_key.as_mut_ptr(), ffi_value.as_mut_ptr()) }
// SAFETY:
// - The backing map is alive as promised by the caller.
@ -876,7 +874,7 @@ macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types {
pub fn [< proto2_rust_thunk_Map_ $key_t _ $t _insert >](m: RawMap, key: $ffi_key_t, value: $ffi_value_t) -> bool;
pub fn [< proto2_rust_thunk_Map_ $key_t _ $t _get >](m: RawMap, key: $ffi_key_t, value: *mut $ffi_view_t) -> bool;
pub fn [< proto2_rust_thunk_Map_ $key_t _ $t _iter >](m: RawMap) -> UntypedMapIterator;
pub fn [< proto2_rust_thunk_Map_ $key_t _ $t _iter_get >](iter: &mut UntypedMapIterator, size_info: MapNodeSizeInfo, key: *mut $ffi_key_t, value: *mut $ffi_view_t);
pub fn [< proto2_rust_thunk_Map_ $key_t _ $t _iter_get >](iter: &mut UntypedMapIterator, key: *mut $ffi_key_t, value: *mut $ffi_view_t);
pub fn [< proto2_rust_thunk_Map_ $key_t _ $t _remove >](m: RawMap, key: $ffi_key_t, value: *mut $ffi_view_t) -> bool;
}
@ -955,8 +953,7 @@ macro_rules! impl_ProxiedInMapValue_for_non_generated_value_types {
// - The thunk does not increment the iterator.
unsafe {
iter.as_raw_mut(Private).next_unchecked::<$key_t, Self, _, _>(
[< proto2_rust_thunk_Map_ $key_t _ $t _iter_get >],
MapNodeSizeInfo(0),
|iter, key, value| { [< proto2_rust_thunk_Map_ $key_t _ $t _iter_get >](iter, key, value) },
$from_ffi_key,
$from_ffi_value,
)

@ -78,8 +78,8 @@ auto MakeCleanup(T value) {
return google::protobuf::internal::UntypedMapIterator::FromTyped(m->cbegin()); \
} \
void proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_iter_get( \
const google::protobuf::internal::UntypedMapIterator* iter, int32_t, \
ffi_key_ty* key, ffi_view_ty* value) { \
const google::protobuf::internal::UntypedMapIterator* iter, ffi_key_ty* key, \
ffi_view_ty* value) { \
auto typed_iter = \
iter->ToTyped<google::protobuf::Map<key_ty, value_ty>::const_iterator>(); \
const auto& cpp_key = typed_iter->first; \

@ -136,8 +136,7 @@ void EnumProxiedInMapValue(Context& ctx, const EnumDescriptor& desc) {
// - The thunk does not increment the iterator.
unsafe {
iter.as_raw_mut($pbi$::Private).next_unchecked::<$key_t$, Self, _, _>(
$pbr$::$map_iter_get_thunk$,
$pbr$::MapNodeSizeInfo(0), // Ignored
|iter, key, value| { $pbr$::$map_iter_get_thunk$(iter, key, value) },
|ffi_key| $from_ffi_key_expr$,
|value| $name$(value),
)

@ -671,8 +671,8 @@ void MessageProxiedInMapValue(Context& ctx, const Descriptor& msg) {
// - The thunk does not increment the iterator.
unsafe {
iter.as_raw_mut($pbi$::Private).next_unchecked::<$key_t$, Self, _, _>(
$pbr$::$map_iter_get$,
$map_size_info_thunk$($key_t$::SIZE_INFO_INDEX),
|iter, key, value| { $pbr$::$map_iter_get$(
iter, $map_size_info_thunk$($key_t$::SIZE_INFO_INDEX), key, value) },
|ffi_key| $from_ffi_key_expr$,
|raw_msg| $Msg$View::new($pbi$::Private, raw_msg)
)

Loading…
Cancel
Save