Skip to content

Remove Crubit bridge-related known attr macros from wald. #786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove Crubit bridge-related known attr macros from wald. #786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions rs_bindings_from_cc/generate_bindings/database/code_snippet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1522,7 +1522,6 @@ pub enum ThunkImpl {
},
/// A function that implements a Rust function thunk.
Function {
conversion_externs: TokenStream,
return_type_name: TokenStream,
thunk_ident: Ident,
param_types: Vec<TokenStream>,
Expand Down Expand Up @@ -1567,7 +1566,6 @@ impl ToTokens for ThunkImpl {
.to_tokens(tokens);
}
ThunkImpl::Function {
conversion_externs,
return_type_name,
thunk_ident,
param_types,
Expand All @@ -1576,8 +1574,6 @@ impl ToTokens for ThunkImpl {
return_stmt,
} => {
quote! {
#conversion_externs

extern "C" #return_type_name #thunk_ident( #( #param_types #param_idents ),* ) {
#conversion_stmts
#return_stmt;
Expand Down
60 changes: 6 additions & 54 deletions rs_bindings_from_cc/generate_bindings/database/rs_snippet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,11 +598,6 @@ impl Callable {

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub enum BridgeRsTypeKind {
BridgeVoidConverters {
rust_name: Rc<str>,
cpp_to_rust_converter: Rc<str>,
rust_to_cpp_converter: Rc<str>,
},
Bridge {
rust_name: Rc<str>,
abi_rust: Rc<str>,
Expand Down Expand Up @@ -643,15 +638,6 @@ impl BridgeRsTypeKind {
};

let bridge_rs_type_kind = match bridge_type.clone() {
BridgeType::BridgeVoidConverters {
rust_name,
cpp_to_rust_converter,
rust_to_cpp_converter,
} => BridgeRsTypeKind::BridgeVoidConverters {
rust_name,
cpp_to_rust_converter,
rust_to_cpp_converter,
},
BridgeType::ProtoMessageBridge { rust_name } => {
BridgeRsTypeKind::ProtoMessageBridge { rust_name }
}
Expand Down Expand Up @@ -717,10 +703,6 @@ impl BridgeRsTypeKind {

Ok(Some(bridge_rs_type_kind))
}

pub fn is_void_converters_bridge_type(&self) -> bool {
matches!(self, BridgeRsTypeKind::BridgeVoidConverters { .. })
}
}

fn new_c9_co_record(
Expand Down Expand Up @@ -969,20 +951,6 @@ impl RsTypeKind {
matches!(self.unalias(), RsTypeKind::BridgeType { .. })
}

pub fn is_pointer_bridge_type(&self) -> bool {
matches!(
self.unalias(),
RsTypeKind::BridgeType {
bridge_type: BridgeRsTypeKind::BridgeVoidConverters { .. },
..
}
)
}

pub fn is_crubit_abi_bridge_type(&self) -> bool {
self.is_bridge_type() && !self.is_pointer_bridge_type()
}

pub fn is_proto_message_bridge_type(&self) -> bool {
matches!(
self.unalias(),
Expand Down Expand Up @@ -1086,19 +1054,7 @@ impl RsTypeKind {
)
}
}
RsTypeKind::BridgeType { bridge_type, original_type } => {
let is_pointer_bridge =
matches!(bridge_type, BridgeRsTypeKind::BridgeVoidConverters { .. });

if !original_type.has_unique_owning_target() && is_pointer_bridge {
require_feature(
CrubitFeature::Experimental,
Some(&|| "bridged template instantiation is not yet supported".into()),
)
} else {
require_feature(CrubitFeature::Types, None)
}
}
RsTypeKind::BridgeType { .. } => require_feature(CrubitFeature::Types, None),
RsTypeKind::ExistingRustType { .. }
| RsTypeKind::Enum { .. }
| RsTypeKind::TypeAlias { .. }
Expand Down Expand Up @@ -1308,9 +1264,9 @@ impl RsTypeKind {
RsTypeKind::TypeAlias { underlying_type, .. } => underlying_type.implements_copy(),
RsTypeKind::BridgeType { bridge_type, .. } => match bridge_type {
// We cannot get the information of the Rust type so we assume it is not Copy.
BridgeRsTypeKind::BridgeVoidConverters { .. }
| BridgeRsTypeKind::Bridge { .. }
| BridgeRsTypeKind::ProtoMessageBridge { .. } => false,
BridgeRsTypeKind::Bridge { .. } | BridgeRsTypeKind::ProtoMessageBridge { .. } => {
false
}
BridgeRsTypeKind::StdOptional(t) => t.implements_copy(),
BridgeRsTypeKind::StdPair(t1, t2) => t1.implements_copy() && t2.implements_copy(),
BridgeRsTypeKind::StdString { .. } => false,
Expand Down Expand Up @@ -1548,7 +1504,7 @@ impl RsTypeKind {
} else if self.is_void() {
// void is a subset of is_c_abi_compatible_by_value, so must be checked before.
PassingConvention::Void
} else if self.is_crubit_abi_bridge_type() {
} else if self.is_bridge_type() {
PassingConvention::ComposablyBridged
} else if !self.is_unpin() {
PassingConvention::Ctor
Expand Down Expand Up @@ -1850,9 +1806,6 @@ impl RsTypeKind {
}
RsTypeKind::BridgeType { bridge_type, original_type } => {
match bridge_type {
BridgeRsTypeKind::BridgeVoidConverters { rust_name, .. } => {
fully_qualify_type(db, ir::Item::Record(original_type.clone()), rust_name)
}
BridgeRsTypeKind::Bridge { rust_name, generic_types, .. } => {
let path = fully_qualify_type(
db,
Expand Down Expand Up @@ -2025,8 +1978,7 @@ impl<'ty> Iterator for RsTypeKindIter<'ty> {
self.todo.extend(param_types.iter().rev());
}
RsTypeKind::BridgeType { bridge_type, .. } => match bridge_type {
BridgeRsTypeKind::BridgeVoidConverters { .. }
| BridgeRsTypeKind::ProtoMessageBridge { .. }
BridgeRsTypeKind::ProtoMessageBridge { .. }
| BridgeRsTypeKind::Bridge { .. } => {}
BridgeRsTypeKind::StdOptional(t) => self.todo.push(t),
BridgeRsTypeKind::StdPair(t1, t2) => {
Expand Down
79 changes: 13 additions & 66 deletions rs_bindings_from_cc/generate_bindings/generate_function_thunk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ use crubit_abi_type::{CrubitAbiTypeToCppExprTokens, CrubitAbiTypeToCppTokens};
use database::code_snippet::{Thunk, ThunkImpl};
use database::db::BindingsGenerator;
use database::rs_snippet::{
format_generic_params, unique_lifetimes, BridgeRsTypeKind, Lifetime, Mutability,
PassingConvention, RsTypeKind,
format_generic_params, unique_lifetimes, Lifetime, Mutability, PassingConvention, RsTypeKind,
};
use error_report::{anyhow, bail};
use ir::*;
Expand Down Expand Up @@ -428,43 +427,22 @@ pub fn generate_function_thunk_impl(
.map(|p| format_cc_ident(&p.identifier.identifier))
.collect::<Result<Vec<_>>>()?;

let mut conversion_externs = quote! {};
let mut conversion_stmts = quote! {};
let convert_ident =
|ident: &Ident| -> Ident { format_ident!("__converted_{}", ident.to_string()) };
let mut param_types = func
.params
.iter()
.map(|p| {
let arg_type = db.rs_type_kind(p.type_.clone())?;
let cpp_type = cpp_type_name::format_cpp_type(&arg_type, db)?;
if let RsTypeKind::BridgeType { bridge_type, .. } = arg_type.unalias() {
if arg_type.is_bridge_type() {
let ident = format_cc_ident(&p.identifier.identifier)?;
match bridge_type {
BridgeRsTypeKind::BridgeVoidConverters { rust_to_cpp_converter, .. } => {
let convert_function = format_cc_ident(rust_to_cpp_converter)?;
let cpp_ident = convert_ident(&ident);
conversion_externs.extend(quote! {
extern "C" void #convert_function(void* rust_struct, void* cpp_struct);
});
conversion_stmts.extend(quote! {
::crubit::LazyInit<#cpp_type> #cpp_ident;
});
conversion_stmts.extend(quote! {
#convert_function(#ident, &#cpp_ident.val);
});
Ok(quote! { void* })
}
_ => {
let crubit_abi_type = db.crubit_abi_type(arg_type)?;
let crubit_abi_type_tokens = CrubitAbiTypeToCppTokens(&crubit_abi_type);
let decoder = format_ident!("__{ident}_decoder");
conversion_stmts.extend(quote! {
::crubit::Decoder #decoder(#crubit_abi_type_tokens::kSize, #ident);
});
Ok(quote! { const unsigned char* })
}
}
let crubit_abi_type = db.crubit_abi_type(arg_type)?;
let crubit_abi_type_tokens = CrubitAbiTypeToCppTokens(&crubit_abi_type);
let decoder = format_ident!("__{ident}_decoder");
conversion_stmts.extend(quote! {
::crubit::Decoder #decoder(#crubit_abi_type_tokens::kSize, #ident);
});
Ok(quote! { const unsigned char* })
} else if !arg_type.is_c_abi_compatible_by_value() {
// non-Unpin types are wrapped by a pointer in the thunk.
Ok(quote! {#cpp_type *})
Expand All @@ -479,12 +457,6 @@ pub fn generate_function_thunk_impl(
.iter()
.map(|p| {
let ident = format_cc_ident(&p.identifier.identifier)?;
let ident = if db.rs_type_kind(p.type_.clone())?.is_pointer_bridge_type() {
let formatted_ident = convert_ident(&ident);
quote! { &(#formatted_ident.val) }
} else {
ident.to_token_stream()
};
match &p.type_.variant {
CcTypeVariant::Pointer(pointer) => match pointer.kind {
PointerTypeKind::RValueRef => Ok(quote! { std::move(*#ident) }),
Expand Down Expand Up @@ -545,19 +517,7 @@ pub fn generate_function_thunk_impl(
}
PassingConvention::LayoutCompatible | PassingConvention::Ctor => {
param_idents.insert(0, format_cc_ident("__return")?);
if let RsTypeKind::BridgeType {
bridge_type: BridgeRsTypeKind::BridgeVoidConverters { cpp_to_rust_converter, .. },
..
} = return_type_kind.unalias()
{
let convert_function = format_cc_ident(cpp_to_rust_converter)?;
conversion_externs.extend(quote! {
extern "C" void #convert_function(void* cpp_struct, void* rust_struct);
});
param_types.insert(0, quote! {void *});
} else {
param_types.insert(0, quote! {#return_type_cpp_spelling *});
}
param_types.insert(0, quote! {#return_type_cpp_spelling *});
quote! {void}
}
PassingConvention::AbiCompatible
Expand Down Expand Up @@ -609,21 +569,9 @@ pub fn generate_function_thunk_impl(
}
PassingConvention::LayoutCompatible | PassingConvention::Ctor => {
let out_param = &param_idents[0];
if let RsTypeKind::BridgeType {
bridge_type: BridgeRsTypeKind::BridgeVoidConverters { cpp_to_rust_converter, .. },
..
} = return_type_kind.unalias()
{
let convert_function = format_cc_ident(cpp_to_rust_converter)?;
quote! {
auto __original_cpp_struct = #return_expr;
#convert_function(&__original_cpp_struct, #out_param)
}
} else {
// Explicitly use placement `new` so that we get guaranteed copy elision in
// C++17.
quote! {new(#out_param) auto(#return_expr)}
}
// Explicitly use placement `new` so that we get guaranteed copy elision in
// C++17.
quote! {new(#out_param) auto(#return_expr)}
}
PassingConvention::Void => return_expr,
PassingConvention::AbiCompatible | PassingConvention::OwnedPtr => {
Expand Down Expand Up @@ -654,7 +602,6 @@ pub fn generate_function_thunk_impl(
};

Ok(Some(ThunkImpl::Function {
conversion_externs,
return_type_name,
thunk_ident,
param_types,
Expand Down
1 change: 0 additions & 1 deletion rs_bindings_from_cc/generate_bindings/generate_struct_and_union.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,6 @@ fn cc_struct_operator_delete_impl(

let cc_record_name = cpp_type_name_for_record(record.as_ref(), db)?;
let thunk_impl = ThunkImpl::Function {
conversion_externs: quote! {},
return_type_name: quote! { void },
thunk_ident: thunk_ident.clone(),
param_types: vec![quote! { #cc_record_name* }],
Expand Down
22 changes: 0 additions & 22 deletions rs_bindings_from_cc/generate_bindings/generate_struct_and_union_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,28 +644,6 @@ fn test_copy_derives_dtor_nontrivial_self() {
}
}

/// If a base class is a bridge type, it doesn't exist at all, and can't be upcasted to.
#[gtest]
fn test_bridged_base_class() -> Result<()> {
let ir = ir_from_cc(
r#"
struct [[clang::annotate("crubit_bridge_type", "BridgedBase"),
clang::annotate("crubit_bridge_type_rust_to_cpp_converter",
"rust_to_cpp_converter"),
clang::annotate("crubit_bridge_type_cpp_to_rust_converter",
"cpp_to_rust_converter")]] Base {
int x;
};

struct Derived : Base {};
"#,
)?;
let rs_api = generate_bindings_tokens_for_test(ir)?.rs_api;
assert_rs_not_matches!(rs_api, quote! { Base });
assert_rs_not_matches!(rs_api, quote! { BridgedBase });
Ok(())
}

#[gtest]
fn test_base_class_subobject_layout() -> Result<()> {
let ir = ir_from_cc(
Expand Down
36 changes: 10 additions & 26 deletions rs_bindings_from_cc/generate_bindings/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ fn rs_type_kind_safety(db: &BindingsGenerator, rs_type_kind: RsTypeKind) -> Safe
| RsTypeKind::ExistingRustType { .. } => Safety::Safe,
RsTypeKind::BridgeType { bridge_type, original_type } => match bridge_type {
// TODO(b/390621592): Should bridge types just delegate to the underlying type?
BridgeRsTypeKind::BridgeVoidConverters { .. } | BridgeRsTypeKind::Bridge { .. } => {
BridgeRsTypeKind::Bridge { .. } => {
if record_safety(db, original_type.clone()).is_safe() {
Safety::Safe
} else {
Expand Down Expand Up @@ -850,12 +850,6 @@ fn generate_rs_api_impl_includes(
// we can ignore it.
if let Ok(Some(bridge_type)) = BridgeRsTypeKind::new(record, has_reference_param, db) {
match bridge_type {
BridgeRsTypeKind::BridgeVoidConverters { .. } => {
internal_includes.insert(CcInclude::SupportLibHeader(
crubit_support_path_format.clone(),
"internal/lazy_init.h".into(),
));
}
BridgeRsTypeKind::C9Co { .. } => {
internal_includes.insert(CcInclude::SupportLibHeader(
crubit_support_path_format.clone(),
Expand Down Expand Up @@ -931,22 +925,15 @@ fn generate_rs_api_impl_includes(
continue;
};

if let RsTypeKind::BridgeType { bridge_type, .. } = rs_type_kind.unalias() {
if bridge_type.is_void_converters_bridge_type() {
internal_includes.insert(CcInclude::SupportLibHeader(
crubit_support_path_format.clone(),
"internal/lazy_init.h".into(),
));
} else {
internal_includes.insert(CcInclude::SupportLibHeader(
crubit_support_path_format.clone(),
"bridge.h".into(),
));
internal_includes.insert(CcInclude::SupportLibHeader(
crubit_support_path_format.clone(),
"internal/slot.h".into(),
));
}
if rs_type_kind.is_bridge_type() {
internal_includes.insert(CcInclude::SupportLibHeader(
crubit_support_path_format.clone(),
"bridge.h".into(),
));
internal_includes.insert(CcInclude::SupportLibHeader(
crubit_support_path_format.clone(),
"internal/slot.h".into(),
));
}
}

Expand Down Expand Up @@ -1049,9 +1036,6 @@ fn crubit_abi_type(db: &BindingsGenerator, rs_type_kind: RsTypeKind) -> Result<C
Primitive::StdUint64T => CrubitAbiType::transmute("u64", "std::uint64_t"),
}),
RsTypeKind::BridgeType { bridge_type, original_type } => match bridge_type {
BridgeRsTypeKind::BridgeVoidConverters { .. } => {
bail!("Void pointer bridge types are not allowed within composable bridging")
}
BridgeRsTypeKind::ProtoMessageBridge { .. } => {
let ir = db.ir();
let target = db
Expand Down
Loading