Add support for references inside bridge types

cramertj · copybara-github · commit 957e54be5e56 · 2026-02-20T15:27:43.000-08:00
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
diff --git a/cc_bindings_from_rs/generate_bindings/format_type.rs b/cc_bindings_from_rs/generate_bindings/format_type.rs
@@ -72,7 +72,7 @@ pub fn format_cc_ident(db: &BindingsGenerator, ident: &str) -> Result<Ident> {
     }
 }
 
-pub fn format_pointer_or_reference_ty_for_cc<'tcx>(
+fn format_pointer_or_reference_ty_for_cc<'tcx>(
     db: &BindingsGenerator<'tcx>,
     pointee: Ty<'tcx>,
     mutability: Mutability,
@@ -91,7 +91,7 @@ pub fn format_pointer_or_reference_ty_for_cc<'tcx>(
     Ok(CcSnippet { prereqs, tokens: quote! { #tokens #const_qualifier #pointer_sigil } })
 }
 
-pub fn format_slice_ref_for_cc<'tcx>(
+fn format_slice_ref_for_cc<'tcx>(
     db: &BindingsGenerator<'tcx>,
     element_ty: Ty<'tcx>,
     mutability: rustc_middle::mir::Mutability,
@@ -118,11 +118,11 @@ pub fn format_slice_ref_for_cc<'tcx>(
 }
 
 /// Returns a CcSnippet referencing `rs_std::StrRef` and its include path.
-pub fn format_str_ref_for_cc<'tcx>(db: &BindingsGenerator<'tcx>) -> CcSnippet<'tcx> {
+fn format_str_ref_for_cc<'tcx>(db: &BindingsGenerator<'tcx>) -> CcSnippet<'tcx> {
     CcSnippet::with_include(quote! { rs_std::StrRef }, db.support_header("rs_std/str_ref.h"))
 }
 
-pub fn format_transparent_pointee_or_reference_for_cc<'tcx>(
+fn format_transparent_pointee_or_reference_for_cc<'tcx>(
     db: &BindingsGenerator<'tcx>,
     referent_ty: Ty<'tcx>,
     mutability: rustc_middle::mir::Mutability,
@@ -315,13 +315,15 @@ pub fn format_ty_for_cc<'tcx>(
                     BridgedType::Composable(mut composable) => {
                         // The existance of crubit_abi_type implies that the type can fully
                         // composably bridge.
-
                         let mut tokens = composable.cpp_type.to_token_stream();
-
                         if !substs.is_empty() {
                             let mut generic_types_tokens = Vec::with_capacity(substs.len());
                             for subst in *substs {
-                                let snippet = format_ty_for_cc(db, subst.expect_ty(), location)?;
+                                let snippet = format_ty_for_cc(
+                                    db,
+                                    subst.expect_ty(),
+                                    TypeLocation::NestedBridgeable,
+                                )?;
                                 generic_types_tokens
                                     .push(snippet.into_tokens(&mut composable.prereqs));
                             }
@@ -562,7 +564,7 @@ pub fn format_ret_ty_for_cc<'tcx>(
         .with_context(|| format!("Error formatting function return type `{output_ty}`"))
 }
 
-pub fn has_elided_region<'tcx>(tcx: TyCtxt<'tcx>, search_ty: ty::Ty<'tcx>) -> bool {
+pub(crate) fn has_elided_region<'tcx>(tcx: TyCtxt<'tcx>, search_ty: ty::Ty<'tcx>) -> bool {
     use core::ops::ControlFlow;
     use rustc_middle::ty::{Region, TyCtxt, TypeVisitor};
 
@@ -637,7 +639,7 @@ fn try_ty_as_maybe_uninit<'tcx>(
 }
 
 /// Format a supported `repr(transparent)` pointee type
-pub fn format_transparent_pointee<'tcx>(
+fn format_transparent_pointee<'tcx>(
     db: &BindingsGenerator<'tcx>,
     ty: Ty<'tcx>,
 ) -> Result<TokenStream> {
@@ -762,12 +764,13 @@ pub fn format_ty_for_rs<'tcx>(db: &BindingsGenerator<'tcx>, ty: Ty<'tcx>) -> Res
                     .map(|subst| match subst.kind() {
                         ty::GenericArgKind::Type(ty) => db.format_ty_for_rs(ty),
                         ty::GenericArgKind::Lifetime(region) => {
-                            assert_eq!(
-                                region.kind(),
-                                ty::RegionKind::ReStatic,
-                                "We should never format types with non-'static regions, as \
-                                    thunks should first call `replace_all_regions_with_static`. {ty}",
-                            );
+                            if !region.is_static() {
+                                panic!(
+                                    "We should never format types with non-'static regions, as \
+                                    thunks should first call `replace_all_regions_with_static`. \
+                                    Found type: `{ty}`."
+                                );
+                            }
                             Ok(quote! { 'static })
                         }
                         ty::GenericArgKind::Const(_) => {
@@ -850,6 +853,7 @@ pub fn format_region_as_cc_lifetime<'tcx>(
     }
 }
 
+#[track_caller]
 pub fn format_region_as_rs_lifetime<'tcx>(
     tcx: TyCtxt<'tcx>,
     region: ty::Region<'tcx>,
@@ -1014,21 +1018,27 @@ pub fn crubit_abi_type_from_ty<'tcx>(
             }
             // Do we need to confirm that pointee is layout compatible?
             let rust_type = db.format_ty_for_rs(pointee)?;
-            let cpp_type_with_prereqs = db.format_ty_for_cc(pointee, TypeLocation::Other)?;
+            let CcSnippet { tokens: cpp_type, prereqs } =
+                db.format_ty_for_cc(pointee, TypeLocation::Other)?;
 
             return Ok(CrubitAbiTypeWithCcPrereqs {
                 crubit_abi_type: CrubitAbiType::Ptr {
                     is_const: mutability.is_not(),
                     is_rust_slice,
                     rust_type,
-                    cpp_type: cpp_type_with_prereqs.tokens,
+                    cpp_type,
                 },
-                prereqs: cpp_type_with_prereqs.prereqs,
+                prereqs,
             });
         }
-        ty::TyKind::Ref(_region, _inner, _mutability) => {
-            // TODO(okabayashi): Support &str and &[T], and possibly other reference types.
-            bail!("Reference types are not yet supported in bridging");
+        ty::TyKind::Ref(_, _, _) => {
+            let rust_type = db.format_ty_for_rs(ty)?;
+            let CcSnippet { tokens: cpp_type, prereqs } =
+                db.format_ty_for_cc(ty, TypeLocation::Other)?;
+            return Ok(CrubitAbiTypeWithCcPrereqs {
+                crubit_abi_type: CrubitAbiType::Transmute { rust_type, cpp_type },
+                prereqs,
+            });
         }
         _ => bail!("Unsupported bridge type: {ty:?}"),
     }))
@@ -1216,30 +1226,40 @@ pub fn is_bridged_type<'tcx>(
     db: &BindingsGenerator<'tcx>,
     ty: Ty<'tcx>,
 ) -> Result<Option<BridgedType<'tcx>>> {
-    match ty.kind() {
-        ty::TyKind::Ref(_, referent, _) if is_bridged_type(db, *referent)?.is_some() => {
-            if let ty::TyKind::Adt(adt, _) = referent.kind() {
-                if let Some(BridgedBuiltin::Option) = BridgedBuiltin::new(db, *adt) {
-                    return Ok(None);
+    // The ABI of bridged types is lifetime-independent, and the Crubit thunks replace all
+    // lifetimes with static.
+    let ty = crate::generate_function_thunk::replace_all_regions_with_static(db.tcx(), ty);
+
+    match *ty.kind() {
+        ty::TyKind::Ref(_, referent, _) => {
+            if is_bridged_type(db, referent)?.is_some() {
+                if let ty::TyKind::Adt(adt, _) = referent.kind() {
+                    if let Some(BridgedBuiltin::Option) = BridgedBuiltin::new(db, *adt) {
+                        return Ok(None);
+                    }
                 }
+                bail!(
+                    "Can't format reference type `{ty}` because the referent is a bridged type. \
+                        Passing bridged types by reference is not supported."
+                )
             }
-            bail!(
-                "Can't format reference type `{ty}` because the referent is a bridged type. \
-                    Passing bridged types by reference is not supported."
-            )
+            Ok(None)
         }
-        ty::TyKind::RawPtr(pointee, _) if is_bridged_type(db, *pointee)?.is_some() => {
-            bail!(
-                "Can't format pointer type `{ty}` because the pointee is a bridged type. \
-                    Passing bridged types by pointer is not supported."
-            )
+        ty::TyKind::RawPtr(pointee, _) => {
+            if is_bridged_type(db, pointee)?.is_some() {
+                bail!(
+                    "Can't format pointer type `{ty}` because the pointee is a bridged type. \
+                        Passing bridged types by pointer is not supported."
+                )
+            }
+            Ok(None)
         }
         ty::TyKind::Adt(adt, substs) => {
             if let Some(bridged_type) = is_manually_annotated_bridged_adt(db, ty)? {
                 return Ok(Some(bridged_type));
             }
 
-            if let Some(bridged_builtin) = BridgedBuiltin::new(db, *adt) {
+            if let Some(bridged_builtin) = BridgedBuiltin::new(db, adt) {
                 // The ADT is either a Result or an Option, which are composable bridged types.
                 let crubit_abi_type_with_cc_prereqs =
                     bridged_builtin.crubit_abi_type(db, substs)?;
@@ -1259,7 +1279,7 @@ pub fn is_bridged_type<'tcx>(
             // The ADT does not need to be bridged, but check if it has generic types that
             // need to be bridged e.g. Box<BridgedType> cannot be formated at
             // the moment. If we encounter a type like this we return an error.
-            for subst in *substs {
+            for subst in substs {
                 if let Some(ty) = subst.as_type() {
                     if is_bridged_type(db, ty)?.is_some() {
                         bail!("Can't format ADT as it has a generic type `{ty}` that is a bridged type");
diff --git a/cc_bindings_from_rs/generate_bindings/generate_function_thunk.rs b/cc_bindings_from_rs/generate_bindings/generate_function_thunk.rs
@@ -337,10 +337,7 @@ pub(crate) fn ident_or_opt_ident(i: &Option<rustc_span::Ident>) -> Option<&rustc
 
 /// Returns an iterator which yields arbitrary unique names for the parameters
 /// of the function identified by `fn_def_id`.
-pub fn thunk_param_names(
-    tcx: ty::TyCtxt<'_>,
-    fn_def_id: DefId,
-) -> impl Iterator<Item = Ident> + '_ {
+fn thunk_param_names(tcx: ty::TyCtxt<'_>, fn_def_id: DefId) -> impl Iterator<Item = Ident> + '_ {
     tcx.fn_arg_idents(fn_def_id).iter().enumerate().map(|(i, ident)| {
         let Some(ident) = ident_or_opt_ident(ident) else {
             return format_ident!("__param_{i}");
diff --git a/cc_bindings_from_rs/test/bridging/composable/composable_bridging.rs b/cc_bindings_from_rs/test/bridging/composable/composable_bridging.rs
@@ -30,6 +30,18 @@ pub fn assert_some_some_5(x: Option<Option<i32>>) {
     assert_eq!(x, Some(Some(5)));
 }
 
+pub fn option_slice_without_first(x: Option<&[i32]>) -> Option<&[i32]> {
+    let (_first, rest) = x?.split_first()?;
+    Some(rest)
+}
+
+pub fn option_adds_one_to_ref(x: Option<&mut i32>) -> Option<&mut i32> {
+    x.map(|x| {
+        *x += 1;
+        x
+    })
+}
+
 #[crubit_annotate::cpp_bridge(
     cpp_type = "std::optional",
     bridge_abi_cpp = "crubit::OptionalAbi",
diff --git a/cc_bindings_from_rs/test/bridging/composable/composable_bridging_test.cc b/cc_bindings_from_rs/test/bridging/composable/composable_bridging_test.cc
@@ -12,6 +12,8 @@
 namespace crubit {
 namespace {
 
+using ::rs_std::SliceRef;
+
 TEST(ComposableBridging, MaybeInt) {
   std::optional<int> maybe_int = composable_bridging::maybe_int();
   ASSERT_TRUE(maybe_int.has_value());
@@ -43,5 +45,24 @@ TEST(ComposableBridging, Parameters) {
       std::make_optional(std::make_optional(5)));
 }
 
+TEST(ComposableBridging, NestedSlices) {
+  using ::composable_bridging::option_slice_without_first;
+  EXPECT_EQ(option_slice_without_first(std::nullopt), std::nullopt);
+  EXPECT_EQ(option_slice_without_first(
+                std::make_optional<SliceRef<const int>>({1, 2, 3})),
+            std::make_optional<SliceRef<const int>>({2, 3}));
+}
+
+TEST(ComposableBridging, NestedRefs) {
+  using ::composable_bridging::option_adds_one_to_ref;
+  EXPECT_EQ(option_adds_one_to_ref(std::nullopt), std::nullopt);
+  int x = 5;
+  std::optional<int* crubit_nonnull> y =
+      option_adds_one_to_ref(std::make_optional(&x));
+  ASSERT_TRUE(y.has_value());
+  EXPECT_EQ(*y.value(), 6);
+  EXPECT_EQ(x, 6);
+}
+
 }  // namespace
 }  // namespace crubit
diff --git a/cc_bindings_from_rs/test/golden/composable_bridging.rs b/cc_bindings_from_rs/test/golden/composable_bridging.rs
@@ -21,6 +21,18 @@ pub fn option_increments(x: Option<i32>) -> Option<i32> {
     x.map(|x| x + 1)
 }
 
+pub fn option_slice_without_first(x: Option<&[i32]>) -> Option<&[i32]> {
+    let (_first, rest) = x?.split_first()?;
+    Some(rest)
+}
+
+pub fn option_adds_one_to_ref(x: Option<&mut i32>) -> Option<&mut i32> {
+    x.map(|x| {
+        *x += 1;
+        x
+    })
+}
+
 ///CRUBIT_ANNOTATE: cpp_type=std::optional
 ///CRUBIT_ANNOTATE: bridge_abi_cpp=crubit::OptionalAbi
 ///CRUBIT_ANNOTATE: bridge_abi_rust=MyOptionRustAbi
diff --git a/cc_bindings_from_rs/test/golden/composable_bridging_cc_api.h b/cc_bindings_from_rs/test/golden/composable_bridging_cc_api.h
@@ -13,7 +13,9 @@
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+#include "support/annotations_internal.h"
 #include "support/bridge.h"
+#include "support/lifetime_annotations.h"
 #include "support/rs_std/slice_ref.h"
 
 #include <cstdint>
@@ -23,27 +25,37 @@ namespace composable_bridging_rust {
 
 // Error generating bindings for `composable_bridging_rust_golden::MyOptionRust`
 // defined at
-// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=27:
+// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=39:
 // Type bindings for composable_bridging_rust_golden::MyOptionRust suppressed
 // due to being mapped to an existing C++ type (std::optional)
 
 // Error generating bindings for
 // `composable_bridging_rust_golden::MyOptionRustAbi` defined at
-// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=38:
+// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=50:
 // Generic types are not supported yet (b/259749095)
 
 // Generated from:
-// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=29
+// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=41
 std::optional<std::int32_t> make_my_option_rust();
 
 // Generated from:
-// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=33
+// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=45
 std::optional<rs_std::SliceRef<const std::int32_t>> maybe_int_slice();
 
+// Generated from:
+// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=29
+std::optional<std::int32_t* $(__anon1) crubit_nonnull> option_adds_one_to_ref(
+    std::optional<std::int32_t* $(__anon1) crubit_nonnull> x);
+
 // Generated from:
 // cc_bindings_from_rs/test/golden/composable_bridging.rs;l=20
 std::optional<std::int32_t> option_increments(std::optional<std::int32_t> x);
 
+// Generated from:
+// cc_bindings_from_rs/test/golden/composable_bridging.rs;l=24
+std::optional<rs_std::SliceRef<const std::int32_t>> option_slice_without_first(
+    std::optional<rs_std::SliceRef<const std::int32_t>> x);
+
 // Generated from:
 // cc_bindings_from_rs/test/golden/composable_bridging.rs;l=12
 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() {
       __return_value_storage);
 }
 
+namespace __crubit_internal {
+extern "C" void __crubit_thunk_option_uadds_uone_uto_uref(
+    unsigned char*, unsigned char* __ret_ptr);
+}
+inline std::optional<std::int32_t* $(__anon1) crubit_nonnull>
+option_adds_one_to_ref(
+    std::optional<std::int32_t* $(__anon1) crubit_nonnull> x) {
+  unsigned char x_buffer[::crubit::OptionAbi<
+      ::crubit::TransmuteAbi<std::int32_t* $static crubit_nonnull>>::kSize];
+  ::crubit::internal::Encode<::crubit::OptionAbi<
+      ::crubit::TransmuteAbi<std::int32_t* $static crubit_nonnull>>>(
+      ::crubit::OptionAbi<
+          ::crubit::TransmuteAbi<std::int32_t* $static crubit_nonnull>>(
+          ::crubit::TransmuteAbi<std::int32_t* $static crubit_nonnull>()),
+      x_buffer, x);
+  unsigned char __return_value_storage[::crubit::OptionAbi<
+      ::crubit::TransmuteAbi<std::int32_t* $static crubit_nonnull>>::kSize];
+  __crubit_internal::__crubit_thunk_option_uadds_uone_uto_uref(
+      x_buffer, __return_value_storage);
+  return ::crubit::internal::Decode<::crubit::OptionAbi<
+      ::crubit::TransmuteAbi<std::int32_t* $static crubit_nonnull>>>(
+      ::crubit::OptionAbi<
+          ::crubit::TransmuteAbi<std::int32_t* $static crubit_nonnull>>(
+          ::crubit::TransmuteAbi<std::int32_t* $static crubit_nonnull>()),
+      __return_value_storage);
+}
+
 namespace __crubit_internal {
 extern "C" void __crubit_thunk_option_uincrements(unsigned char*,
                                                   unsigned char* __ret_ptr);
@@ -110,6 +149,33 @@ inline std::optional<std::int32_t> option_increments(
       __return_value_storage);
 }
 
+namespace __crubit_internal {
+extern "C" void __crubit_thunk_option_uslice_uwithout_ufirst(
+    unsigned char*, unsigned char* __ret_ptr);
+}
+inline std::optional<rs_std::SliceRef<const std::int32_t>>
+option_slice_without_first(
+    std::optional<rs_std::SliceRef<const std::int32_t>> x) {
+  unsigned char x_buffer[::crubit::OptionAbi<
+      ::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>::kSize];
+  ::crubit::internal::Encode<::crubit::OptionAbi<
+      ::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>>(
+      ::crubit::OptionAbi<
+          ::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>(
+          ::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>()),
+      x_buffer, x);
+  unsigned char __return_value_storage[::crubit::OptionAbi<
+      ::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>::kSize];
+  __crubit_internal::__crubit_thunk_option_uslice_uwithout_ufirst(
+      x_buffer, __return_value_storage);
+  return ::crubit::internal::Decode<::crubit::OptionAbi<
+      ::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>>(
+      ::crubit::OptionAbi<
+          ::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>>(
+          ::crubit::TransmuteAbi<rs_std::SliceRef<const std::int32_t>>()),
+      __return_value_storage);
+}
+
 namespace __crubit_internal {
 extern "C" void __crubit_thunk_returns_uno_uint(unsigned char* __ret_ptr);
 }
diff --git a/cc_bindings_from_rs/test/golden/composable_bridging_cc_api_impl.rs b/cc_bindings_from_rs/test/golden/composable_bridging_cc_api_impl.rs
diff --git a/support/rs_std/slice_ref.h b/support/rs_std/slice_ref.h
diff --git a/support/rs_std/slice_ref_test.cc b/support/rs_std/slice_ref_test.cc
