Skip to content

Commit 257d01e

Browse files
cramertjcopybara-github
authored andcommitted
Add support for references inside bridge types
Additionally, this CL: * adds an operator==/!= impl for SliceRef in order to simplify testing. * fixes a bug where substs in composable bridged types were lowered using the TypeLocation of the parent type rather than TypeLocation::NestedBridgable PiperOrigin-RevId: 872574800
1 parent 132dadb commit 257d01e

File tree

7 files changed

+231
-13
lines changed

7 files changed

+231
-13
lines changed

cc_bindings_from_rs/generate_bindings/format_type.rs

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -293,13 +293,15 @@ pub fn format_ty_for_cc<'tcx>(
293293
BridgedType::Composable(mut composable) => {
294294
// The existance of crubit_abi_type implies that the type can fully
295295
// composably bridge.
296-
297296
let mut tokens = composable.cpp_type.to_token_stream();
298-
299297
if !substs.is_empty() {
300298
let mut generic_types_tokens = Vec::with_capacity(substs.len());
301299
for subst in *substs {
302-
let snippet = format_ty_for_cc(db, subst.expect_ty(), location)?;
300+
let snippet = format_ty_for_cc(
301+
db,
302+
subst.expect_ty(),
303+
TypeLocation::NestedBridgeable,
304+
)?;
303305
generic_types_tokens
304306
.push(snippet.into_tokens(&mut composable.prereqs));
305307
}
@@ -990,21 +992,27 @@ pub fn crubit_abi_type_from_ty<'tcx>(
990992
}
991993
// Do we need to confirm that pointee is layout compatible?
992994
let rust_type = db.format_ty_for_rs(pointee)?;
993-
let cpp_type_with_prereqs = db.format_ty_for_cc(pointee, TypeLocation::Other)?;
995+
let CcSnippet { tokens: cpp_type, prereqs } =
996+
db.format_ty_for_cc(pointee, TypeLocation::Other)?;
994997

995998
return Ok(CrubitAbiTypeWithCcPrereqs {
996999
crubit_abi_type: CrubitAbiType::Ptr {
9971000
is_const: mutability.is_not(),
9981001
is_rust_slice,
9991002
rust_type,
1000-
cpp_type: cpp_type_with_prereqs.tokens,
1003+
cpp_type,
10011004
},
1002-
prereqs: cpp_type_with_prereqs.prereqs,
1005+
prereqs,
10031006
});
10041007
}
1005-
ty::TyKind::Ref(_region, _inner, _mutability) => {
1006-
// TODO(okabayashi): Support &str and &[T], and possibly other reference types.
1007-
bail!("Reference types are not yet supported in bridging");
1008+
ty::TyKind::Ref(_, _, _) => {
1009+
let rust_type = db.format_ty_for_rs(ty)?;
1010+
let CcSnippet { tokens: cpp_type, prereqs } =
1011+
db.format_ty_for_cc(ty, TypeLocation::Other)?;
1012+
return Ok(CrubitAbiTypeWithCcPrereqs {
1013+
crubit_abi_type: CrubitAbiType::Transmute { rust_type, cpp_type },
1014+
prereqs,
1015+
});
10081016
}
10091017
_ => bail!("Unsupported bridge type: {ty:?}"),
10101018
}))

cc_bindings_from_rs/test/bridging/composable/composable_bridging.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,18 @@ pub fn assert_some_some_5(x: Option<Option<i32>>) {
3030
assert_eq!(x, Some(Some(5)));
3131
}
3232

33+
pub fn option_slice_without_first(x: Option<&[i32]>) -> Option<&[i32]> {
34+
let (_first, rest) = x?.split_first()?;
35+
Some(rest)
36+
}
37+
38+
pub fn option_adds_one_to_ref(x: Option<&mut i32>) -> Option<&mut i32> {
39+
x.map(|x| {
40+
*x += 1;
41+
x
42+
})
43+
}
44+
3345
#[crubit_annotate::cpp_bridge(
3446
cpp_type = "std::optional",
3547
bridge_abi_cpp = "crubit::OptionalAbi",

cc_bindings_from_rs/test/bridging/composable/composable_bridging_test.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace crubit {
1313
namespace {
1414

15+
using ::rs_std::SliceRef;
16+
1517
TEST(ComposableBridging, MaybeInt) {
1618
std::optional<int> maybe_int = composable_bridging::maybe_int();
1719
ASSERT_TRUE(maybe_int.has_value());
@@ -43,5 +45,24 @@ TEST(ComposableBridging, Parameters) {
4345
std::make_optional(std::make_optional(5)));
4446
}
4547

48+
TEST(ComposableBridging, NestedSlices) {
49+
using ::composable_bridging::option_slice_without_first;
50+
EXPECT_EQ(option_slice_without_first(std::nullopt), std::nullopt);
51+
EXPECT_EQ(option_slice_without_first(
52+
std::make_optional<SliceRef<const int>>({1, 2, 3})),
53+
std::make_optional<SliceRef<const int>>({2, 3}));
54+
}
55+
56+
TEST(ComposableBridging, NestedRefs) {
57+
using ::composable_bridging::option_adds_one_to_ref;
58+
EXPECT_EQ(option_adds_one_to_ref(std::nullopt), std::nullopt);
59+
int x = 5;
60+
std::optional<int* crubit_nonnull> y =
61+
option_adds_one_to_ref(std::make_optional(&x));
62+
ASSERT_TRUE(y.has_value());
63+
EXPECT_EQ(*y.value(), 6);
64+
EXPECT_EQ(x, 6);
65+
}
66+
4667
} // namespace
4768
} // namespace crubit

cc_bindings_from_rs/test/golden/composable_bridging.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@ pub fn option_increments(x: Option<i32>) -> Option<i32> {
2121
x.map(|x| x + 1)
2222
}
2323

24+
pub fn option_slice_without_first(x: Option<&[i32]>) -> Option<&[i32]> {
25+
let (_first, rest) = x?.split_first()?;
26+
Some(rest)
27+
}
28+
29+
pub fn option_adds_one_to_ref(x: Option<&mut i32>) -> Option<&mut i32> {
30+
x.map(|x| {
31+
*x += 1;
32+
x
33+
})
34+
}
35+
2436
///CRUBIT_ANNOTATE: cpp_type=std::optional
2537
///CRUBIT_ANNOTATE: bridge_abi_cpp=crubit::OptionalAbi
2638
///CRUBIT_ANNOTATE: bridge_abi_rust=MyOptionRustAbi

cc_bindings_from_rs/test/golden/composable_bridging_cc_api.h

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
#pragma clang diagnostic push
1515
#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
16+
#include "support/annotations_internal.h"
1617
#include "support/bridge.h"
18+
#include "support/lifetime_annotations.h"
1719
#include "support/rs_std/slice_ref.h"
1820

1921
#include <cstdint>
@@ -23,27 +25,37 @@ namespace composable_bridging_rust {
2325

2426
// Error generating bindings for `composable_bridging_rust_golden::MyOptionRust`
2527
// defined at
26-
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=27:
28+
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=39:
2729
// Type bindings for composable_bridging_rust_golden::MyOptionRust suppressed
2830
// due to being mapped to an existing C++ type (std::optional)
2931

3032
// Error generating bindings for
3133
// `composable_bridging_rust_golden::MyOptionRustAbi` defined at
32-
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=38:
34+
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=50:
3335
// Generic types are not supported yet (b/259749095)
3436

3537
// Generated from:
36-
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=29
38+
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=41
3739
std::optional<std::int32_t> make_my_option_rust();
3840

3941
// Generated from:
40-
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=33
42+
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=45
4143
std::optional<rs_std::SliceRef<const std::int32_t>> maybe_int_slice();
4244

45+
// Generated from:
46+
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=29
47+
std::optional<std::int32_t* $(__anon1) crubit_nonnull> option_adds_one_to_ref(
48+
std::optional<std::int32_t* $(__anon1) crubit_nonnull> x);
49+
4350
// Generated from:
4451
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=20
4552
std::optional<std::int32_t> option_increments(std::optional<std::int32_t> x);
4653

54+
// Generated from:
55+
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=24
56+
std::optional<rs_std::SliceRef<const std::int32_t>> option_slice_without_first(
57+
std::optional<rs_std::SliceRef<const std::int32_t>> x);
58+
4759
// Generated from:
4860
// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=12
4961
std::optional<std::int32_t> returns_no_int();
@@ -86,6 +98,33 @@ inline std::optional<rs_std::SliceRef<const std::int32_t>> maybe_int_slice() {
8698
__return_value_storage);
8799
}
88100

101+
namespace __crubit_internal {
102+
extern "C" void __crubit_thunk_option_uadds_uone_uto_uref(
103+
unsigned char*, unsigned char* __ret_ptr);
104+
}
105+
inline std::optional<std::int32_t* $(__anon1) crubit_nonnull>
106+
option_adds_one_to_ref(
107+
std::optional<std::int32_t* $(__anon1) crubit_nonnull> x) {
108+
unsigned char x_buffer[::crubit::OptionAbi<
109+
::crubit::TransmuteAbi<std::int32_t* $(__anon1) crubit_nonnull>>::kSize];
110+
::crubit::internal::Encode<::crubit::OptionAbi<
111+
::crubit::TransmuteAbi<std::int32_t* $(__anon1) crubit_nonnull>>>(
112+
::crubit::OptionAbi<
113+
::crubit::TransmuteAbi<std::int32_t* $(__anon1) crubit_nonnull>>(
114+
::crubit::TransmuteAbi<std::int32_t* $(__anon1) crubit_nonnull>()),
115+
x_buffer, x);
116+
unsigned char __return_value_storage[::crubit::OptionAbi<
117+
::crubit::TransmuteAbi<std::int32_t* $(__anon1) crubit_nonnull>>::kSize];
118+
__crubit_internal::__crubit_thunk_option_uadds_uone_uto_uref(
119+
x_buffer, __return_value_storage);
120+
return ::crubit::internal::Decode<::crubit::OptionAbi<
121+
::crubit::TransmuteAbi<std::int32_t* $(__anon1) crubit_nonnull>>>(
122+
::crubit::OptionAbi<
123+
::crubit::TransmuteAbi<std::int32_t* $(__anon1) crubit_nonnull>>(
124+
::crubit::TransmuteAbi<std::int32_t* $(__anon1) crubit_nonnull>()),
125+
__return_value_storage);
126+
}
127+
89128
namespace __crubit_internal {
90129
extern "C" void __crubit_thunk_option_uincrements(unsigned char*,
91130
unsigned char* __ret_ptr);
@@ -110,6 +149,33 @@ inline std::optional<std::int32_t> option_increments(
110149
__return_value_storage);
111150
}
112151

152+
namespace __crubit_internal {
153+
extern "C" void __crubit_thunk_option_uslice_uwithout_ufirst(
154+
unsigned char*, unsigned char* __ret_ptr);
155+
}
156+
inline std::optional<rs_std::SliceRef<const std::int32_t>>
157+
option_slice_without_first(
158+
std::optional<rs_std::SliceRef<const std::int32_t>> x) {
159+
unsigned char x_buffer[::crubit::OptionAbi<
160+
::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>::kSize];
161+
::crubit::internal::Encode<::crubit::OptionAbi<
162+
::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>>(
163+
::crubit::OptionAbi<
164+
::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>(
165+
::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>()),
166+
x_buffer, x);
167+
unsigned char __return_value_storage[::crubit::OptionAbi<
168+
::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>::kSize];
169+
__crubit_internal::__crubit_thunk_option_uslice_uwithout_ufirst(
170+
x_buffer, __return_value_storage);
171+
return ::crubit::internal::Decode<::crubit::OptionAbi<
172+
::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>>(
173+
::crubit::OptionAbi<
174+
::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>(
175+
::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>()),
176+
__return_value_storage);
177+
}
178+
113179
namespace __crubit_internal {
114180
extern "C" void __crubit_thunk_returns_uno_uint(unsigned char* __ret_ptr);
115181
}

cc_bindings_from_rs/test/golden/composable_bridging_cc_api_impl.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,28 @@ unsafe extern "C" fn __crubit_thunk_maybe_uint_uslice(__ret_ptr: *mut core::ffi:
4141
}
4242
}
4343
#[unsafe(no_mangle)]
44+
unsafe extern "C" fn __crubit_thunk_option_uadds_uone_uto_uref(
45+
x: *const core::ffi::c_uchar,
46+
__ret_ptr: *mut core::ffi::c_uchar,
47+
) -> () {
48+
unsafe {
49+
let x = unsafe {
50+
::bridge_rust::internal::decode(
51+
::bridge_rust::OptionAbi(::bridge_rust::transmute_abi::<&'static mut i32>()),
52+
x,
53+
)
54+
};
55+
let __rs_return_value = ::composable_bridging_rust_golden::option_adds_one_to_ref(x);
56+
unsafe {
57+
::bridge_rust::internal::encode(
58+
::bridge_rust::OptionAbi(::bridge_rust::transmute_abi::<&'static mut i32>()),
59+
__ret_ptr as *mut core::ffi::c_uchar,
60+
__rs_return_value,
61+
);
62+
}
63+
}
64+
}
65+
#[unsafe(no_mangle)]
4466
unsafe extern "C" fn __crubit_thunk_option_uincrements(
4567
x: *const core::ffi::c_uchar,
4668
__ret_ptr: *mut core::ffi::c_uchar,
@@ -63,6 +85,28 @@ unsafe extern "C" fn __crubit_thunk_option_uincrements(
6385
}
6486
}
6587
#[unsafe(no_mangle)]
88+
unsafe extern "C" fn __crubit_thunk_option_uslice_uwithout_ufirst(
89+
x: *const core::ffi::c_uchar,
90+
__ret_ptr: *mut core::ffi::c_uchar,
91+
) -> () {
92+
unsafe {
93+
let x = unsafe {
94+
::bridge_rust::internal::decode(
95+
::bridge_rust::OptionAbi(::bridge_rust::transmute_abi::<&'static [i32]>()),
96+
x,
97+
)
98+
};
99+
let __rs_return_value = ::composable_bridging_rust_golden::option_slice_without_first(x);
100+
unsafe {
101+
::bridge_rust::internal::encode(
102+
::bridge_rust::OptionAbi(::bridge_rust::transmute_abi::<&'static [i32]>()),
103+
__ret_ptr as *mut core::ffi::c_uchar,
104+
__rs_return_value,
105+
);
106+
}
107+
}
108+
}
109+
#[unsafe(no_mangle)]
66110
unsafe extern "C" fn __crubit_thunk_returns_uno_uint(__ret_ptr: *mut core::ffi::c_uchar) -> () {
67111
unsafe {
68112
let __rs_return_value = ::composable_bridging_rust_golden::returns_no_int();

support/rs_std/slice_ref.h

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,61 @@ class CRUBIT_INTERNAL_RUST_TYPE("&[]")
9999
size_t size_;
100100
};
101101

102+
namespace internal {
103+
template <typename T>
104+
constexpr bool EqualImpl(SliceRef<const T> a, SliceRef<const T> b) {
105+
absl::Span<const T> a_span = a.to_span();
106+
absl::Span<const T> b_span = b.to_span();
107+
return std::equal(a_span.begin(), a_span.end(), b_span.begin(), b_span.end());
108+
}
109+
} // namespace internal
110+
111+
template <typename T>
112+
constexpr bool operator==(SliceRef<T> a, SliceRef<T> b) {
113+
return internal::EqualImpl(a, b);
114+
}
115+
template <typename T>
116+
constexpr bool operator==(SliceRef<const T> a, SliceRef<T> b) {
117+
return internal::EqualImpl(a, b);
118+
}
119+
template <typename T>
120+
constexpr bool operator==(SliceRef<T> a, SliceRef<const T> b) {
121+
return internal::EqualImpl(a, b);
122+
}
123+
template <typename T, typename U>
124+
requires(std::convertible_to<U, SliceRef<const T>>)
125+
constexpr bool operator==(const U& a, SliceRef<T> b) {
126+
return internal::EqualImpl(a, b);
127+
}
128+
template <typename T, typename U>
129+
requires(std::convertible_to<U, SliceRef<const T>>)
130+
constexpr bool operator==(SliceRef<T> a, const U& b) {
131+
return internal::EqualImpl(a, b);
132+
}
133+
134+
template <typename T>
135+
constexpr bool operator!=(SliceRef<T> a, SliceRef<T> b) {
136+
return !(a == b);
137+
}
138+
template <typename T>
139+
constexpr bool operator!=(SliceRef<const T> a, SliceRef<T> b) {
140+
return !(a == b);
141+
}
142+
template <typename T>
143+
constexpr bool operator!=(SliceRef<T> a, SliceRef<const T> b) {
144+
return !(a == b);
145+
}
146+
template <typename T, typename U>
147+
requires(std::convertible_to<U, SliceRef<const T>>)
148+
constexpr bool operator!=(const U& a, SliceRef<T> b) {
149+
return !(a == b);
150+
}
151+
template <typename T, typename U>
152+
requires(std::convertible_to<U, SliceRef<const T>>)
153+
constexpr bool operator!=(SliceRef<T> a, const U& b) {
154+
return !(a == b);
155+
}
156+
102157
} // namespace rs_std
103158

104159
namespace crubit::internal {

0 commit comments

Comments
 (0)