Skip to content

Commit 7b522fe

Browse files
dtolnaydtolnay
authored
Merge pull request #1681 from anforowicz/vec-of-box
Add support for `Vec<Box<T>>`.
2 parents 6ab7caa + 7a9cad2 commit 7b522fe

File tree

12 files changed

+120
-23
lines changed

12 files changed

+120
-23
lines changed

‎macro/src/expand.rs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,8 +1670,7 @@ fn expand_rust_box(
16701670
.explicit_impl
16711671
.map_or(key.end_span, |explicit| explicit.brace_token.span.join());
16721672
let unsafe_token = format_ident!("unsafe", span = begin_span);
1673-
let prevent_unwind_drop_label =
1674-
format!("::{} as Drop>::drop", generics::local_type(key.inner).rust);
1673+
let prevent_unwind_type_label = generics::format_for_prevent_unwind_label(key.inner);
16751674

16761675
quote_spanned!(end_span=> {
16771676
#cfg
@@ -1699,7 +1698,7 @@ fn expand_rust_box(
16991698
#[doc(hidden)]
17001699
#[unsafe(export_name = #link_drop)]
17011700
unsafe extern "C" fn __drop #impl_generics(this: *mut ::cxx::alloc::boxed::Box<#inner_with_generics>) {
1702-
let __fn = ::cxx::core::concat!("<", ::cxx::core::module_path!(), #prevent_unwind_drop_label);
1701+
let __fn = ::cxx::core::concat!("<", #prevent_unwind_type_label, " as Drop>::drop");
17031702
::cxx::private::prevent_unwind(__fn, || unsafe { ::cxx::core::ptr::drop_in_place(this) });
17041703
}
17051704
})
@@ -1731,8 +1730,7 @@ fn expand_rust_vec(
17311730
.explicit_impl
17321731
.map_or(key.end_span, |explicit| explicit.brace_token.span.join());
17331732
let unsafe_token = format_ident!("unsafe", span = begin_span);
1734-
let prevent_unwind_drop_label =
1735-
format!("::{} as Drop>::drop", generics::local_type(key.inner).rust);
1733+
let prevent_unwind_type_label = generics::format_for_prevent_unwind_label(key.inner);
17361734

17371735
quote_spanned!(end_span=> {
17381736
#cfg
@@ -1754,7 +1752,7 @@ fn expand_rust_vec(
17541752
#[doc(hidden)]
17551753
#[unsafe(export_name = #link_drop)]
17561754
unsafe extern "C" fn __drop #impl_generics(this: *mut ::cxx::private::RustVec<#inner_with_generics>) {
1757-
let __fn = ::cxx::core::concat!("<", ::cxx::core::module_path!(), #prevent_unwind_drop_label);
1755+
let __fn = ::cxx::core::concat!("<", #prevent_unwind_type_label, " as Drop>::drop");
17581756
::cxx::private::prevent_unwind(
17591757
__fn,
17601758
|| unsafe { ::cxx::core::ptr::drop_in_place(this) },
@@ -1809,7 +1807,7 @@ fn expand_rust_vec(
18091807
#[doc(hidden)]
18101808
#[unsafe(export_name = #link_truncate)]
18111809
unsafe extern "C" fn __truncate #impl_generics(this: *mut ::cxx::private::RustVec<#inner_with_generics>, len: ::cxx::core::primitive::usize) {
1812-
let __fn = ::cxx::core::concat!("<", ::cxx::core::module_path!(), #prevent_unwind_drop_label);
1810+
let __fn = ::cxx::core::concat!("<", #prevent_unwind_type_label, " as Drop>::drop");
18131811
::cxx::private::prevent_unwind(
18141812
__fn,
18151813
|| unsafe { (*this).truncate(len) },

‎macro/src/generics.rs‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use crate::expand::display_namespaced;
22
use crate::syntax::instantiate::NamedImplKey;
33
use crate::syntax::types::ConditionalImpl;
4-
use crate::syntax::{Lifetimes, NamedType, Type, Types};
4+
use crate::syntax::{Lifetimes, Type, Types};
55
use proc_macro2::TokenStream;
6-
use quote::ToTokens;
6+
use quote::{quote_spanned, ToTokens};
77
use syn::{Lifetime, Token};
88

99
pub(crate) struct ResolvedGenericType<'a> {
@@ -26,7 +26,7 @@ pub(crate) fn split_for_impl<'a>(
2626
let impl_generics = if let Some(explicit_impl) = conditional_impl.explicit_impl {
2727
&explicit_impl.impl_generics
2828
} else {
29-
types.resolve(local_type(key.inner)).generics
29+
get_impl_generics(key.inner, types)
3030
};
3131
let ty_generics = ResolvedGenericType {
3232
ty: key.inner,
@@ -61,21 +61,57 @@ impl<'a> ToTokens for ResolvedGenericType<'a> {
6161
}
6262
}
6363
}
64+
Type::RustBox(ty1) => {
65+
let span = ty1.name.span();
66+
let inner = ResolvedGenericType {
67+
ty: &ty1.inner,
68+
explicit_impl: self.explicit_impl,
69+
types: self.types,
70+
};
71+
tokens.extend(quote_spanned! {span=>
72+
::cxx::alloc::boxed::Box<#inner>
73+
});
74+
}
6475
_ => unreachable!("syntax/check.rs should reject other types"),
6576
}
6677
}
6778
}
6879

69-
pub(crate) fn local_type(ty: &Type) -> &NamedType {
80+
fn get_impl_generics<'a>(ty: &Type, types: &'a Types<'a>) -> &'a Lifetimes {
7081
match ty {
71-
Type::Ident(named_type) => named_type,
82+
Type::Ident(named_type) => types.resolve(named_type).generics,
83+
Type::RustBox(ty1) => get_impl_generics(&ty1.inner, types),
84+
_ => unreachable!("syntax/check.rs should reject other types"),
85+
}
86+
}
87+
88+
pub(crate) fn format_for_prevent_unwind_label(ty: &Type) -> TokenStream {
89+
match ty {
90+
Type::Ident(named_type) => {
91+
let span = named_type.rust.span();
92+
let rust_name = named_type.rust.to_string();
93+
quote_spanned! {span=>
94+
::cxx::core::concat!(::cxx::core::module_path!(), "::", #rust_name)
95+
}
96+
}
97+
Type::RustBox(ty1) => {
98+
let span = ty1.name.span();
99+
let inner = format_for_prevent_unwind_label(&ty1.inner);
100+
quote_spanned! {span=>
101+
::cxx::core::concat!("Box<", #inner, ">")
102+
}
103+
}
72104
_ => unreachable!("syntax/check.rs should reject other types"),
73105
}
74106
}
75107

76108
pub(crate) fn concise_rust_name(ty: &Type) -> String {
77109
match ty {
78110
Type::Ident(named_type) => named_type.rust.to_string(),
111+
Type::RustBox(ty1) => {
112+
let inner = concise_rust_name(&ty1.inner);
113+
format!("Box<{inner}>")
114+
}
79115
_ => unreachable!("syntax/check.rs should reject other types"),
80116
}
81117
}
@@ -86,6 +122,10 @@ pub(crate) fn concise_cxx_name(ty: &Type, types: &Types) -> String {
86122
let res = types.resolve(&named_type.rust);
87123
display_namespaced(res.name).to_string()
88124
}
125+
Type::RustBox(ty1) => {
126+
let inner = concise_cxx_name(&ty1.inner, types);
127+
format!("rust::Box<{inner}>")
128+
}
89129
_ => unreachable!("syntax/check.rs should reject other types"),
90130
}
91131
}

‎macro/src/tests.rs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,26 @@ fn test_original_lifetimes_used_in_impls() {
156156
// Verify which lifetime name ('sess, 'srv, 'clt) gets used for this impl.
157157
assert!(rs.contains("impl<'sess> ::cxx::memory::UniquePtrTarget for Context<'sess> {"));
158158
}
159+
160+
/// This test covers implicit impl of `Vec<Box<T>>`.
161+
#[test]
162+
fn test_vec_of_box() {
163+
let rs = bridge(quote! {
164+
mod ffi {
165+
extern "Rust" {
166+
type R;
167+
fn foo() -> Vec<Box<R>>;
168+
}
169+
}
170+
});
171+
assert!(rs.contains("unsafe impl ::cxx::private::ImplBox for R {}"));
172+
assert!(rs.contains("export_name = \"cxxbridge1$box$R$drop\""));
173+
174+
assert!(rs.contains("unsafe impl ::cxx::private::ImplVec for ::cxx::alloc::boxed::Box<R> {}"),);
175+
assert!(rs.contains("export_name = \"cxxbridge1$rust_vec$box$R$set_len\""));
176+
177+
// Covering these lines, because an early WIP incorrectly said
178+
// `RustVec<*mut R>` instead of `RustVec<Box<R>>` in *some* of these lines.
179+
assert!(rs.contains("__return: *mut ::cxx::private::RustVec<::cxx::alloc::boxed::Box<R>>"));
180+
assert!(rs.contains("fn __foo() -> ::cxx::alloc::vec::Vec<::cxx::alloc::boxed::Box<R>>"));
181+
}

‎syntax/check.rs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ fn check_type_rust_vec(cx: &mut Check, ty: &Ty1) {
133133
}
134134
}
135135
Type::Str(_) => return,
136+
Type::RustBox(ty1) => {
137+
check_type_box(cx, ty1);
138+
return;
139+
}
136140
_ => {}
137141
}
138142

@@ -605,15 +609,15 @@ fn check_api_impl(cx: &mut Check, imp: &Impl) {
605609
| Type::WeakPtr(ty)
606610
| Type::CxxVector(ty) => {
607611
if let Type::Ident(inner) = &ty.inner {
608-
if Atom::from(&inner.rust).is_none() {
609-
return;
612+
// Reject `impl Vec<u8>` and other built-in impls.
613+
if Atom::from(&inner.rust).is_some() {
614+
cx.error(imp, "unsupported Self type of explicit impl");
610615
}
611616
}
612617
}
613-
_ => {}
618+
// Reject `impl fn() -> &S {}`, `impl [S]`, etc.
619+
_ => cx.error(imp, "unsupported Self type of explicit impl"),
614620
}
615-
616-
cx.error(imp, "unsupported Self type of explicit impl");
617621
}
618622

619623
fn check_mut_return_restriction(cx: &mut Check, efn: &ExternFn) {

‎syntax/instantiate.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ impl<'a> ImplKey<'a> {
2323
/// present in two places, which is accomplished using trait impls and the
2424
/// orphan rule. Every instantiation of a C++ template like `CxxVector<T>`
2525
/// and Rust generic type like `Vec<T>` requires the implementation of
26-
/// traits defined by the `cxx` crate for some local type. (TODO: or for a
27-
/// fundamental type like `Box<LocalType>`)
26+
/// traits defined by the `cxx` crate for some local type or for a
27+
/// fundamental type like `Box<LocalType>`.
2828
pub(crate) fn is_implicit_impl_ok(&self, types: &Types) -> bool {
2929
// TODO: relax this for Rust generics to allow Vec<Vec<T>> etc.
3030
types.is_local(self.inner())

‎syntax/mangle.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ pub(crate) fn typename(t: &Type, res: &UnorderedMap<&Ident, Resolution>) -> Opti
134134
match t {
135135
Type::Ident(named_type) => res.get(&named_type.rust).map(|res| res.name.to_symbol()),
136136
Type::CxxVector(ty1) => typename(&ty1.inner, res).map(|s| join!("std", "vector", s)),
137+
Type::RustBox(ty1) => typename(&ty1.inner, res).map(|s| join!("box", s)),
137138
_ => None,
138139
}
139140
}

‎syntax/types.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,8 @@ impl<'a> Types<'a> {
338338
|| self.aliases.contains_key(ident)
339339
}
340340
Type::CxxVector(_) => false,
341+
// Note: `Type::RustBox(_)` cannot be used as an inner type of
342+
// `CxxVector`, `UniquePtr`, nor `SharedPtr`.
341343
_ => unreachable!("syntax/check.rs should reject other types"),
342344
}
343345
}
@@ -371,10 +373,11 @@ impl<'a> Types<'a> {
371373
Type::Ident(ident) => {
372374
Atom::from(&ident.rust).is_none() && !self.aliases.contains_key(&ident.rust)
373375
}
374-
Type::RustBox(_) => {
375-
// TODO: We should treat Box<LocalType> as local.
376-
// https://doc.rust-lang.org/reference/items/implementations.html#r-items.impl.trait.fundamental
377-
false
376+
Type::RustBox(ty1) => {
377+
// From Rust reference [1]: "Any time a type T is considered local [...]
378+
// Box<T> [... is] also considered local."
379+
// [1] https://doc.rust-lang.org/reference/glossary.html#fundamental-type-constructors
380+
self.is_local(&ty1.inner)
378381
}
379382
Type::Array(_)
380383
| Type::CxxVector(_)

‎tests/ffi/lib.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,10 @@ pub mod ffi {
306306
fn r_return_rust_vec() -> Vec<u8>;
307307
fn r_return_rust_vec_string() -> Vec<String>;
308308
fn r_return_rust_vec_extern_struct() -> Vec<Job>;
309+
#[allow(clippy::vec_box)]
310+
fn r_return_rust_vec_box() -> Vec<Box<R>>;
311+
#[allow(clippy::vec_box)]
312+
fn r_return_rust_vec_box_other_module_type() -> Vec<Box<OpaqueRust>>;
309313
fn r_return_ref_rust_vec(shared: &Shared) -> &Vec<u8>;
310314
fn r_return_mut_rust_vec(shared: &mut Shared) -> &mut Vec<u8>;
311315
fn r_return_identity(_: usize) -> usize;
@@ -600,6 +604,16 @@ fn r_return_rust_vec_extern_struct() -> Vec<ffi::Job> {
600604
Vec::new()
601605
}
602606

607+
#[allow(clippy::vec_box)]
608+
fn r_return_rust_vec_box() -> Vec<Box<R>> {
609+
vec![Box::new(R(2020))]
610+
}
611+
612+
#[allow(clippy::vec_box)]
613+
fn r_return_rust_vec_box_other_module_type() -> Vec<Box<module::OpaqueRust>> {
614+
vec![Box::new(module::OpaqueRust(2025))]
615+
}
616+
603617
fn r_return_ref_rust_vec(shared: &ffi::Shared) -> &Vec<u8> {
604618
let _ = shared;
605619
unimplemented!()

‎tests/ffi/module.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub mod ffi {
2626
impl Vec<Job> {}
2727
impl Box<OpaqueRust> {}
2828
impl Vec<OpaqueRust> {}
29+
impl Vec<Box<OpaqueRust>> {}
2930
}
3031

3132
#[cxx::bridge(namespace = "tests")]

‎tests/ffi/tests.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,8 @@ extern "C" const char *cxx_run_test() noexcept {
812812
ASSERT(r_return_enum(2021) == Enum::CVal);
813813
ASSERT(Shared::r_static_method_on_shared() == 2023);
814814
ASSERT(R::r_static_method() == 2024);
815+
ASSERT(r_return_rust_vec_box()[0]->get() == 2020);
816+
ASSERT(r_return_rust_vec_box_other_module_type().size() == 1);
815817

816818
r_take_primitive(2020);
817819
r_take_shared(Shared{2020});

0 commit comments

Comments
 (0)