Skip to content

Commit 4996f97

Browse files
committed
Fix lifetimes in CppRef<'a, T> return types.
Previously these types did contain a lifetime, but we didn't parameterize the function by a lifetime.
1 parent 3c678e2 commit 4996f97

File tree

3 files changed

+31
-6
lines changed

3 files changed

+31
-6
lines changed

engine/src/conversion/codegen_rs/fun_codegen.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ pub(super) fn gen_function(
133133
params,
134134
Cow::Borrowed(&ret_type),
135135
non_pod_types,
136+
&ret_conversion,
136137
);
137138

138139
if analysis.rust_wrapper_needed {
@@ -308,6 +309,7 @@ impl<'a> FnGenerator<'a> {
308309
wrapper_params,
309310
ret_type,
310311
self.non_pod_types,
312+
self.ret_conversion,
311313
);
312314

313315
let cxxbridge_name = self.cxxbridge_name;

engine/src/conversion/codegen_rs/lifetime.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
// except according to those terms.
88
use crate::{
99
conversion::analysis::fun::{
10-
function_wrapper::RustConversionType, ArgumentAnalysis, ReceiverMutability,
10+
function_wrapper::{RustConversionType, TypeConversionPolicy},
11+
ArgumentAnalysis, ReceiverMutability,
1112
},
1213
types::QualifiedName,
1314
};
@@ -22,7 +23,7 @@ use syn::{
2223

2324
/// Function which can add explicit lifetime parameters to function signatures
2425
/// where necessary, based on analysis of parameters and return types.
25-
/// This is necessary in three cases:
26+
/// This is necessary in four cases:
2627
/// 1) where the parameter is a Pin<&mut T>
2728
/// and the return type is some kind of reference - because lifetime elision
2829
/// is not smart enough to see inside a Pin.
@@ -31,11 +32,13 @@ use syn::{
3132
/// built-in type
3233
/// 3) Any parameter is any form of reference, and we're returning an `impl New`
3334
/// 3a) an 'impl ValueParam' counts as a reference.
35+
/// 4) If we're using CppRef<'a, T> as a return type
3436
pub(crate) fn add_explicit_lifetime_if_necessary<'r>(
3537
param_details: &[ArgumentAnalysis],
3638
mut params: Punctuated<FnArg, Comma>,
3739
ret_type: Cow<'r, ReturnType>,
3840
non_pod_types: &HashSet<QualifiedName>,
41+
ret_conversion: &Option<TypeConversionPolicy>,
3942
) -> (
4043
Option<TokenStream>,
4144
Punctuated<FnArg, Comma>,
@@ -54,11 +57,22 @@ pub(crate) fn add_explicit_lifetime_if_necessary<'r>(
5457
)
5558
});
5659
let return_type_is_impl = return_type_is_impl(&ret_type);
60+
let return_type_is_cppref = matches!(
61+
ret_conversion,
62+
Some(TypeConversionPolicy {
63+
rust_conversion: RustConversionType::FromPointerToReferenceWrapper,
64+
..
65+
})
66+
);
5767
let non_pod_ref_param = reference_parameter_is_non_pod_reference(&params, non_pod_types);
5868
let ret_type_pod = return_type_is_pod_or_known_type_reference(&ret_type, non_pod_types);
5969
let returning_impl_with_a_reference_param = return_type_is_impl && any_param_is_reference;
6070
let hits_1024_bug = non_pod_ref_param && ret_type_pod;
61-
if !(has_mutable_receiver || hits_1024_bug || returning_impl_with_a_reference_param) {
71+
if !(has_mutable_receiver
72+
|| hits_1024_bug
73+
|| returning_impl_with_a_reference_param
74+
|| return_type_is_cppref)
75+
{
6276
return (None, params, ret_type);
6377
}
6478
let new_return_type = match ret_type.as_ref() {
@@ -83,6 +97,12 @@ pub(crate) fn add_explicit_lifetime_if_necessary<'r>(
8397
#rarrow #old_tyit + 'a
8498
})
8599
}
100+
Type::Ptr(_) if return_type_is_cppref => {
101+
// The ptr will be converted to CppRef<'a, T> elsewhere, so we
102+
// just need to return the return type as-is such that the
103+
// next match statement adds <'a> to the function.
104+
Some(ret_type.clone().into_owned())
105+
}
86106
_ => None,
87107
},
88108
_ => None,

integration-tests/tests/cpprefs_test.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,11 @@ fn test_return_reference_cpprefs() {
124124
const Bob& give_bob(const Bob& input_bob);
125125
"};
126126
let rs = quote! {
127-
let b = ffi::Bob { a: 3, b: 4 };
128-
assert_eq!(ffi::give_bob(&b).b, 4);
127+
let b = CppPin::new(ffi::Bob { a: 3, b: 4 });
128+
let b_ref = b.as_cpp_ref();
129+
let bob = ffi::give_bob(&b_ref);
130+
let val = unsafe { bob.as_ref() };
131+
assert_eq!(val.b, 4);
129132
};
130133
run_cpprefs_test(cxx, hdr, rs, &["give_bob"], &["Bob"]);
131-
}
134+
}

0 commit comments

Comments
 (0)