When a trait has a nontrivial parameter, materialize it in the caller.
(Technically, this does let you call trait methods with nontrivial arguments,
but it's not in a final/decent place.)
This is an alternate version of unknown commit which tried to continue to defer
materialization. However, as discussed in[]
currently impossible.
In particular, we cannot define a trait like this:
```rs
impl<T: Ctor<Output=A>> MyTrait<T> for S {...}
impl<T: Ctor<Output=B>> MyTrait<T> for S {...}
```
... because Rust does not understand that these impls are disjoint:
https://github.com/rust-lang/rust/issues/20400
#### What's next?
(Apologies if this is a bit unorganized, I've spent too much time in the trenches.)
So this CL is just a first step: we *must* monomorphize the implementation.
Rather than accepting any `T: Ctor`, accept an `RvalueReference` (a concrete type).
After this CL, I think we have a slightly more open field than I thought. In particular,
we should be able to regain the `Ctor` API, except using only *one* parameterized impl.
So, instead of the broken impls above, we can have this impl:
```rs
impl<'a> MyTrait<RvalueReference<'a, A>> for S {...}
impl<'a> MyTrait<RvalueReference<'a, B>> for S {...}
impl<U, CtorType> MyTrait<CtorType> for S
where
&C : for<'a> MyTrait<RvalueReference<'a, U>>,
CtorType: Ctor<Output=U>
{...}
```
Because this is only _one_ parameterized impl, there's no conflicts. It is
implemented in terms of the concrete non-parameterized impls as generated by
this change.
However, I'm not yet 100% certain this will work, and it is actually not a small task
to do, even on top of this CL. For example, there's a bunch of refactoring to let one
generate a second blanket impl using knowledge about the trait function etc. from the
concrete impl.
##### RvalueReference might need to get replaced.
If we can use the `Ctor` approach described above... we can't use `RvalueReference`,
actually, because Rust will then recurse infinitely. The `RvalueReference` type used
for the `for<'a> MyTrait<RvalueReference<...>>` bound must be in the _same_ crate so
that Rust knows that `RvalueReference` doesn't itself impl `Ctor`. And unfortunately,
no, negative impls aren't good enough here, yet, apparently. At least, it didn't
resolve it when I tried it!
You can test this in a local two-crate setup.
Crate 1:
```rs
pub trait Ctor {
type Output;
}
pub struct RvalueReference<'a, T>(&'a T);
```
Crate 2:
```rs
use lib1::*;
pub struct A1;
pub struct A2;
pub struct B;
impl <'a> From<RvalueReference<'a, A1>> for B {
fn from(_: RvalueReference<'a, A1>) -> Self { todo!(); }
}
impl <'a> From<RvalueReference<'a, A2>> for B {
fn from(_: RvalueReference<'a, A2>) -> Self { todo!(); }
}
impl <T: Ctor> From<T> for B
where B : for<'a> From<RvalueReference<'a, T::Output>> {
fn from(_: T) -> Self { todo!(); }
}
```
If you build crate 2, it will fail with the following error:
```
error[E0275]: overflow evaluating the requirement `for<'a> B: From<lib1::RvalueReference<'a, _>>`
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`lib2`)
note: required because of the requirements on the impl of `for<'a> From<lib1::RvalueReference<'a, _>>` for `B`
--> src/lib.rs:15:16
|
15 | impl <T: Ctor> From<T> for B
| ^^^^^^^ ^
= note: 126 redundant requirements hidden
= note: required because of the requirements on the impl of `for<'a> From<lib1::RvalueReference<'a, _>>` for `B`
For more information about this error, try `rustc --explain E0275`.
error: could not compile `lib2` due to previous error
```
But it will work fine if you move `RvalueReference` to another crate!
##### If all else fails, we'll force the caller to materialize the Ctor
If even the approach outlined above doesn't work, well, we'll just have to force callers
to materialize the `Ctor`: call `Trait::method(mov(emplace!(foo())))` instead of
`Trait::method(foo())`.
That's what I described in[]
but I'm hoping we can work our way out after all!
Either way, both approaches build on this change. Investigating the followups may take some
time, so I'd rather not leave this change sitting around generating merge conflicts,
if possible. :X
PiperOrigin-RevId: 464613254
diff --git a/rs_bindings_from_cc/src_code_gen.rs b/rs_bindings_from_cc/src_code_gen.rs
index 7f3d9b0..497755a 100644
--- a/rs_bindings_from_cc/src_code_gen.rs
+++ b/rs_bindings_from_cc/src_code_gen.rs
@@ -495,6 +495,10 @@
/// Returns the shape of the generated Rust API for a given function definition.
///
+/// If the shape is a trait, this also mutates the parameter types to be
+/// trait-compatible. In particular, types which would be `impl Ctor<Output=T>`
+/// become a `RvalueReference<'_, T>`.
+///
/// Returns:
///
/// * `Err(_)`: something went wrong importing this function.
@@ -504,7 +508,7 @@
fn api_func_shape(
func: &Func,
ir: &IR,
- param_types: &[RsTypeKind],
+ param_types: &mut [RsTypeKind],
) -> Result<Option<(Ident, ImplKind)>> {
let maybe_record: Option<&Rc<Record>> = ir.record_for_member_func(func)?;
let has_pointer_params = param_types.iter().any(|p| matches!(p, RsTypeKind::Pointer { .. }));
@@ -555,6 +559,7 @@
if record.is_unpin() {
bail!("operator= for Unpin types is not yet supported.");
}
+ materialize_ctor_in_caller(param_types);
let rhs = ¶m_types[1];
impl_kind = ImplKind::new_trait(
TraitName::Other {
@@ -575,6 +580,7 @@
}
}
+ materialize_ctor_in_caller(param_types);
let (record, impl_for) = if let Some(record) = maybe_record {
(&**record, ImplFor::RefT)
} else {
@@ -660,6 +666,7 @@
);
func_name = make_rs_ident("drop");
} else {
+ materialize_ctor_in_caller(param_types);
impl_kind = ImplKind::new_trait(
TraitName::Other {
name: quote! {::ctor::PinnedDrop},
@@ -693,6 +700,7 @@
);
}
+ materialize_ctor_in_caller(param_types);
let record_name = make_rs_ident(&record.rs_name);
if !record.is_unpin() {
func_name = make_rs_ident("ctor_new");
@@ -763,6 +771,45 @@
Ok(Some((func_name, impl_kind)))
}
+/// Mutates the provided parameters so that nontrivial by-value parameters are,
+/// instead, materialized in the caller and passed by rvalue reference.
+///
+/// This produces rvalue references with elided lifetimes. If they compile,
+/// they are correct, but the resulting signatures can be surprising.
+///
+/// For example, consider `From`. This rewriting of function parameters might
+/// create an impl as follows:
+///
+/// ```
+/// // C++: struct Foo{ Foo(Nontrivial x); };
+/// impl From<RvalueReference<'_, Nontrivial>> for Foo {
+/// fn from(x: RvalueReference<'_, Nontrivial>) -> Self { ... }
+/// }
+/// ```
+///
+/// Each `'_` is actually a different lifetime! However, due to lifetime
+/// subtyping, they are allowed to be used in this sort of way, interchangeably.
+/// And indeed, this is even idiomatic. For example, the exact same pattern is
+/// in use here:
+///
+/// https://doc.rust-lang.org/std/string/struct.String.html#impl-From%3C%26%27_%20String%3E
+///
+/// If there is some case where this is actually _not_ okay, then we would need
+/// to generate new named lifetimes, rather than elided lifetimes.
+fn materialize_ctor_in_caller(params: &mut [RsTypeKind]) {
+ for param in params {
+ if param.is_unpin() {
+ continue;
+ }
+ let value = std::mem::replace(param, RsTypeKind::Unit); // Temporarily swap in a garbage value.
+ *param = RsTypeKind::RvalueReference {
+ referent: Rc::new(value),
+ mutability: Mutability::Mut,
+ lifetime: Lifetime::Elided,
+ };
+ }
+}
+
/// Generates Rust source code for a given `Func`.
///
/// Returns:
@@ -778,7 +825,7 @@
) -> Result<Option<RcEq<(RsSnippet, RsSnippet, Rc<FunctionId>)>>> {
let ir = db.ir();
let mut features = BTreeSet::new();
- let param_types = func
+ let mut param_types = func
.params
.iter()
.map(|p| {
@@ -788,12 +835,12 @@
})
.collect::<Result<Vec<_>>>()?;
- let (func_name, mut impl_kind) = if let Some(values) = api_func_shape(&func, &ir, ¶m_types)?
- {
- values
- } else {
- return Ok(None);
- };
+ let (func_name, mut impl_kind) =
+ if let Some(values) = api_func_shape(&func, &ir, &mut param_types)? {
+ values
+ } else {
+ return Ok(None);
+ };
let mut return_type = db
.rs_type_kind(func.return_type.rs_type.clone())
@@ -807,8 +854,12 @@
for (i, (ident, type_)) in param_idents.iter().zip(param_types.iter()).enumerate() {
if !type_.is_unpin() {
// `impl Ctor` will fail to compile in a trait.
+ // This will only be hit if there was a bug in api_func_shape.
if let ImplKind::Trait { .. } = &impl_kind {
- bail!("b/200067242: non-Unpin types are not yet supported by value in traits");
+ panic!(
+ "non-Unpin types cannot work by value in traits; this should have instead \
+ become an rvalue reference to force the caller to materialize the Ctor."
+ );
}
// The generated bindings require a move constructor.
if !type_.is_move_constructible() {
@@ -1016,8 +1067,7 @@
};
let fn_generic_params: TokenStream;
- if let ImplKind::Trait { trait_name, trait_generic_params, impl_for, .. } = &mut impl_kind
- {
+ if let ImplKind::Trait { trait_name, trait_generic_params, impl_for, .. } = &mut impl_kind {
// When the impl block is for some kind of reference to T, consider the lifetime
// parameters on the self parameter to be trait lifetimes so they can be
// introduced before they are used.
@@ -6388,12 +6438,31 @@
Nontrivial& operator=(Nontrivial) {}
~Nontrivial();
};
+
+ struct Trivial final {
+ /*implicit*/ Trivial(Nontrivial) {}
+ };
"#,
)?;
let rs_api = generate_bindings_tokens(ir)?.rs_api;
- // the assign trait shouldn't be implemented for `Nontrivial`.
- // (In fact, it shouldn't be referenced at all -- thus the very minimal test!)
- assert_rs_not_matches!(rs_api, quote! {Assign});
+ assert_rs_matches!(
+ rs_api,
+ quote! {
+ impl From<::ctor::RvalueReference<'_, crate::Nontrivial>> for Trivial {
+ #[inline(always)]
+ fn from(__param_0: ::ctor::RvalueReference<'_, crate::Nontrivial>) -> Self {
+ let mut tmp = ::std::mem::MaybeUninit::<Self>::zeroed();
+ unsafe {
+ crate::detail::__rust_thunk___ZN7TrivialC1E10Nontrivial(
+ &mut tmp,
+ __param_0
+ );
+ tmp.assume_init()
+ }
+ }
+ }
+ }
+ );
Ok(())
}
