Allow &mut calls on Trivial extern types only.

Previously we allowed extern types to be used as un-pinned
reference parameters only if there happened to be some other
use of the type in some way which required it to be Trivial.

With this change,
* Opaque extern types must always be pinned when used as mutable
  reference parameters;
* Trivial extern types need never be pinned when used as mutable
  reference parameters.
This commit is contained in:
Adrian Taylor 2020-12-06 22:43:26 -08:00
parent 75ea17ce4e
commit 73f2daff89
7 changed files with 98 additions and 18 deletions

View File

@ -189,7 +189,7 @@ fn check_type_ref(cx: &mut Check, ty: &Ref) {
cx.error(
ty,
format!(
"mutable reference to C++ type requires a pin -- use Pin<&mut {}>",
"mutable reference to C++ type requires a pin -- use Pin<&mut {}> or declare the type Trivial in a cxx::ExternType impl",
requires_pin,
),
);
@ -332,6 +332,10 @@ fn check_api_type(cx: &mut Check, ety: &ExternType) {
TrivialReason::FunctionReturn(efn) => format!("a return value of `{}`", efn.name.rust),
TrivialReason::BoxTarget => format!("Box<{}>", ety.name.rust),
TrivialReason::VecElement => format!("a vector element in Vec<{}>", ety.name.rust),
TrivialReason::UnpinnedMutableReferenceFunctionArgument(efn) => format!(
"a non-pinned mutable reference argument of {}",
efn.name.rust
),
};
let msg = format!(
"needs a cxx::ExternType impl in order to be used as {}",
@ -385,7 +389,7 @@ fn check_api_fn(cx: &mut Check, efn: &ExternFn) {
cx.error(
span,
format!(
"mutable reference to C++ type requires a pin -- use `self: Pin<&mut {}>`",
"mutable reference to opaque C++ type requires a pin -- use `self: Pin<&mut {}>` or declare the type Trivial in a cxx::ExternType impl",
receiver.ty.rust,
),
);

View File

@ -170,14 +170,13 @@ impl<'a> Types<'a> {
// the APIs above, in case some function or struct references a type
// which is declared subsequently.
let mut required_trivial = Map::new();
let mut insist_alias_types_are_trivial = |ty: &'a Type, reason| {
if let Type::Ident(ident) = ty {
if cxx.contains(&ident.rust)
&& !structs.contains_key(&ident.rust)
&& !enums.contains_key(&ident.rust)
{
required_trivial.entry(&ident.rust).or_insert(reason);
}
let mut insist_extern_types_are_trivial = |ident: &'a RustName, reason| {
if cxx.contains(&ident.rust)
&& !structs.contains_key(&ident.rust)
&& !enums.contains_key(&ident.rust)
{
required_trivial.entry(&ident.rust).or_insert(reason);
}
};
for api in apis {
@ -185,17 +184,23 @@ impl<'a> Types<'a> {
Api::Struct(strct) => {
let reason = TrivialReason::StructField(strct);
for field in &strct.fields {
insist_alias_types_are_trivial(&field.ty, reason);
if let Type::Ident(ident) = &field.ty {
insist_extern_types_are_trivial(&ident, reason);
}
}
}
Api::CxxFunction(efn) | Api::RustFunction(efn) => {
let reason = TrivialReason::FunctionArgument(efn);
for arg in &efn.args {
insist_alias_types_are_trivial(&arg.ty, reason);
if let Type::Ident(ident) = &arg.ty {
insist_extern_types_are_trivial(&ident, reason);
}
}
if let Some(ret) = &efn.ret {
let reason = TrivialReason::FunctionReturn(efn);
insist_alias_types_are_trivial(&ret, reason);
if let Type::Ident(ident) = &ret {
insist_extern_types_are_trivial(&ident, reason);
}
}
}
_ => {}
@ -205,11 +210,37 @@ impl<'a> Types<'a> {
match ty {
Type::RustBox(ty) => {
let reason = TrivialReason::BoxTarget;
insist_alias_types_are_trivial(&ty.inner, reason);
if let Type::Ident(ident) = &ty.inner {
insist_extern_types_are_trivial(&ident, reason);
}
}
Type::RustVec(ty) => {
let reason = TrivialReason::VecElement;
insist_alias_types_are_trivial(&ty.inner, reason);
if let Type::Ident(ident) = &ty.inner {
insist_extern_types_are_trivial(&ident, reason);
}
}
_ => {}
}
}
for api in apis {
match api {
Api::CxxFunction(efn) | Api::RustFunction(efn) => {
let reason = TrivialReason::UnpinnedMutableReferenceFunctionArgument(efn);
if let Some(receiver) = &efn.receiver {
if receiver.mutable && !receiver.pinned {
insist_extern_types_are_trivial(&receiver.ty, reason);
}
}
for arg in &efn.args {
if let Type::Ref(reff) = &arg.ty {
if reff.mutable && !reff.pinned {
if let Type::Ident(ident) = &reff.inner {
insist_extern_types_are_trivial(&ident, reason);
}
}
}
}
}
_ => {}
}
@ -310,6 +341,7 @@ pub enum TrivialReason<'a> {
FunctionReturn(&'a ExternFn),
BoxTarget,
VecElement,
UnpinnedMutableReferenceFunctionArgument(&'a ExternFn),
}
fn duplicate_name(cx: &mut Errors, sp: impl ToTokens, ident: &Ident) {

View File

@ -32,12 +32,16 @@ pub mod ffi2 {
fn c_take_trivial_ptr(d: UniquePtr<D>);
fn c_take_trivial_ref(d: &D);
fn c_take_trivial_ref_method(self: &D);
fn c_take_trivial_mut_ref_method(self: &mut D);
fn c_take_trivial(d: D);
fn c_take_trivial_ns_ptr(g: UniquePtr<G>);
fn c_take_trivial_ns_ref(g: &G);
fn c_take_trivial_ns(g: G);
fn c_take_opaque_ptr(e: UniquePtr<E>);
fn c_take_opaque_ref(e: &E);
fn c_take_opaque_ref_method(self: &E);
fn c_take_opaque_mut_ref_method(self: Pin<&mut E>);
fn c_take_opaque_ns_ptr(e: UniquePtr<F>);
fn c_take_opaque_ns_ref(e: &F);
fn c_return_trivial_ptr() -> UniquePtr<D>;

View File

@ -529,6 +529,18 @@ void c_take_trivial_ref(const D &d) {
}
}
void D::c_take_trivial_ref_method() const {
if (d == 30) {
cxx_test_suite_set_correct();
}
}
void D::c_take_trivial_mut_ref_method() {
if (d == 30) {
cxx_test_suite_set_correct();
}
}
void c_take_trivial(D d) {
if (d.d == 30) {
cxx_test_suite_set_correct();
@ -571,6 +583,18 @@ void c_take_opaque_ref(const E &e) {
}
}
void E::c_take_opaque_ref_method() const {
if (e == 40 && e_str == "hello") {
cxx_test_suite_set_correct();
}
}
void E::c_take_opaque_mut_ref_method() {
if (e == 40 && e_str == "hello") {
cxx_test_suite_set_correct();
}
}
void c_take_opaque_ns_ref(const ::F::F &f) {
if (f.f == 40 && f.f_str == "hello") {
cxx_test_suite_set_correct();

View File

@ -59,11 +59,15 @@ private:
struct D {
uint64_t d;
void c_take_trivial_ref_method() const;
void c_take_trivial_mut_ref_method();
};
struct E {
uint64_t e;
std::string e_str;
void c_take_opaque_ref_method() const;
void c_take_opaque_mut_ref_method();
};
enum COwnedEnum {

View File

@ -263,10 +263,14 @@ fn test_rust_name_attribute() {
#[test]
fn test_extern_trivial() {
let d = ffi2::c_return_trivial();
let mut d = ffi2::c_return_trivial();
check!(ffi2::c_take_trivial_ref(&d));
check!(d.c_take_trivial_ref_method());
check!(d.c_take_trivial_mut_ref_method());
check!(ffi2::c_take_trivial(d));
let d = ffi2::c_return_trivial_ptr();
let mut d = ffi2::c_return_trivial_ptr();
check!(d.c_take_trivial_ref_method());
check!(d.c_take_trivial_mut_ref_method());
check!(ffi2::c_take_trivial_ptr(d));
cxx::UniquePtr::new(ffi2::D { d: 42 });
let d = ffi2::ns_c_return_trivial();
@ -282,8 +286,10 @@ fn test_extern_trivial() {
#[test]
fn test_extern_opaque() {
let e = ffi2::c_return_opaque_ptr();
let mut e = ffi2::c_return_opaque_ptr();
check!(ffi2::c_take_opaque_ref(e.as_ref().unwrap()));
check!(e.c_take_opaque_ref_method());
check!(e.pin_mut().c_take_opaque_mut_ref_method());
check!(ffi2::c_take_opaque_ptr(e));
let f = ffi2::c_return_ns_opaque_ptr();

View File

@ -16,6 +16,12 @@ error: mutable reference to C++ type requires a pin -- use Pin<&mut CxxVector<..
9 | fn v(v: &mut CxxVector<u8>);
| ^^^^^^^^^^^^^^^^^^
error: needs a cxx::ExternType impl in order to be used as a non-pinned mutable reference argument of f
--> $DIR/pin_mut_opaque.rs:4:9
|
4 | type Opaque;
| ^^^^^^^^^^^
error: mutable reference to C++ type requires a pin -- use `self: Pin<&mut Opaque>`
--> $DIR/pin_mut_opaque.rs:6:14
|