Merge pull request #439 from dtolnay/sig

Enforce accurate unsafety declaration on extern Rust sigs

Author: dtolnay

macro/src/expand.rs

@@ -563,14 +563,10 @@ fn expand_rust_function_shim_impl(
     });
     let vars = receiver_var.into_iter().chain(arg_vars);
 
+    let wrap_super = invoke.map(|invoke| expand_rust_function_shim_super(sig, &local_name, invoke));
+
     let mut call = match invoke {
-        Some(ident) => match &sig.receiver {
-            None => quote!(super::#ident),
-            Some(receiver) => {
-                let receiver_type = &receiver.ty;
-                quote!(#receiver_type::#ident)
-            }
-        },
+        Some(_) => quote!(#local_name),
         None => quote!(::std::mem::transmute::<*const (), #sig>(__extern)),
     };
     call.extend(quote! { (#(#vars),*) });
@@ -648,11 +644,61 @@ fn expand_rust_function_shim_impl(
         #[export_name = #link_name]
         unsafe extern "C" fn #local_name(#(#all_args,)* #outparam #pointer) #ret {
             let __fn = concat!(module_path!(), #catch_unwind_label);
+            #wrap_super
             #expr
         }
     }
 }
 
+// A wrapper like `fn f(x: Arg) { super::f(x) }` just to ensure we have the
+// accurate unsafety declaration and no problematic elided lifetimes.
+fn expand_rust_function_shim_super(
+    sig: &Signature,
+    local_name: &Ident,
+    invoke: &Ident,
+) -> TokenStream {
+    let unsafety = sig.unsafety;
+
+    let receiver_var = sig
+        .receiver
+        .as_ref()
+        .map(|receiver| Ident::new("__self", receiver.var.span));
+    let receiver = sig.receiver.iter().map(|receiver| {
+        let receiver_type = receiver.ty();
+        quote!(#receiver_var: #receiver_type)
+    });
+    let args = sig.args.iter().map(|arg| quote!(#arg));
+    let all_args = receiver.chain(args);
+
+    let ret = if sig.throws {
+        let ok = match &sig.ret {
+            Some(ret) => quote!(#ret),
+            None => quote!(()),
+        };
+        quote!(-> ::std::result::Result<#ok, impl ::std::fmt::Display>)
+    } else {
+        expand_return_type(&sig.ret)
+    };
+
+    let arg_vars = sig.args.iter().map(|arg| &arg.ident);
+    let vars = receiver_var.iter().chain(arg_vars);
+
+    let span = invoke.span();
+    let call = match &sig.receiver {
+        None => quote_spanned!(span=> super::#invoke),
+        Some(receiver) => {
+            let receiver_type = &receiver.ty;
+            quote_spanned!(span=> #receiver_type::#invoke)
+        }
+    };
+
+    quote_spanned! {span=>
+        #unsafety fn #local_name(#(#all_args,)*) #ret {
+            #call(#(#vars,)*)
+        }
+    }
+}
+
 fn expand_type_alias(alias: &TypeAlias) -> TokenStream {
     let doc = &alias.doc;
     let ident = &alias.name.rust;

tests/ffi/lib.rs

@@ -1,5 +1,6 @@
 #![allow(
     clippy::boxed_local,
+    clippy::just_underscores_and_digits,
     clippy::ptr_arg,
     clippy::trivially_copy_pass_by_ref
 )]

tests/ui/missing_unsafe.rs

@@ -0,0 +1,10 @@
+#[cxx::bridge]
+mod ffi {
+    extern "Rust" {
+        fn f(x: i32);
+    }
+}
+
+unsafe fn f(_x: i32) {}
+
+fn main() {}

tests/ui/missing_unsafe.stderr

@@ -0,0 +1,7 @@
+error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
+ --> $DIR/missing_unsafe.rs:4:12
+  |
+4 |         fn f(x: i32);
+  |            ^ call to unsafe function
+  |
+  = note: consult the function's documentation for information on how to avoid undefined behavior

tests/ui/result_no_display.stderr

@@ -1,14 +1,8 @@
 error[E0277]: `NonError` doesn't implement `std::fmt::Display`
-  --> $DIR/result_no_display.rs:1:1
-   |
-1  | #[cxx::bridge]
-   | ^^^^^^^^^^^^^^ `NonError` cannot be formatted with the default formatter
-   |
-  ::: $WORKSPACE/src/result.rs
-   |
-   |     E: Display,
-   |        ------- required by this bound in `cxx::private::r#try`
-   |
-   = help: the trait `std::fmt::Display` is not implemented for `NonError`
-   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
-   = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)
+ --> $DIR/result_no_display.rs:1:1
+  |
+1 | #[cxx::bridge]
+  | ^^^^^^^^^^^^^^ `NonError` cannot be formatted with the default formatter
+  |
+  = help: the trait `std::fmt::Display` is not implemented for `NonError`
+  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead

tests/ui/unsupported_elided.rs

@@ -0,0 +1,20 @@
+use std::marker::PhantomData;
+
+#[cxx::bridge]
+mod ffi {
+    extern "Rust" {
+        type T;
+
+        fn f(t: &T) -> &str;
+    }
+}
+
+pub struct T<'a> {
+    _lifetime: PhantomData<&'a ()>,
+}
+
+fn f<'a>(_t: &T<'a>) -> &'a str {
+    ""
+}
+
+fn main() {}

tests/ui/unsupported_elided.stderr

@@ -0,0 +1,11 @@
+error[E0106]: missing lifetime specifier
+ --> $DIR/unsupported_elided.rs:8:24
+  |
+8 |         fn f(t: &T) -> &str;
+  |                 --     ^ expected named lifetime parameter
+  |
+  = help: this function's return type contains a borrowed value, but the signature does not say which one of `t`'s 2 lifetimes it is borrowed from
+help: consider introducing a named lifetime parameter
+  |
+8 |         fn f<'a>(t: &'a T) -> &'a str;
+  |             ^^^^    ^^^^^     ^^^