Skip to content

Commit

Permalink
Merge pull request #1550 from alexcrichton/more-better-errors
Browse files Browse the repository at this point in the history
Improve diagnostics with missing trait implementations
  • Loading branch information
alexcrichton authored May 20, 2019
2 parents b96186e + 2cbb8b8 commit bc5f73e
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 34 deletions.
87 changes: 53 additions & 34 deletions crates/backend/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,18 +197,23 @@ impl ToTokens for ast::Struct {
impl wasm_bindgen::__rt::core::convert::From<#name> for
wasm_bindgen::JsValue
{
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
fn from(value: #name) -> Self {
let ptr = wasm_bindgen::convert::IntoWasmAbi::into_abi(
value,
unsafe { &mut wasm_bindgen::convert::GlobalStack::new() },
);

#[link(wasm_import_module = "__wbindgen_placeholder__")]
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
extern "C" {
fn #new_fn(ptr: u32) -> u32;
}

#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
unsafe fn #new_fn(ptr: u32) -> u32 {
panic!("cannot convert to JsValue outside of the wasm target")
}

unsafe {
<wasm_bindgen::JsValue as wasm_bindgen::convert::FromWasmAbi>
::from_abi(
Expand All @@ -217,11 +222,6 @@ impl ToTokens for ast::Struct {
)
}
}

#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
fn from(_value: #name) -> Self {
panic!("cannot convert to JsValue outside of the wasm target")
}
}

#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
Expand Down Expand Up @@ -712,24 +712,22 @@ impl ToTokens for ast::ImportType {
}

impl JsCast for #rust_name {
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
fn instanceof(val: &JsValue) -> bool {
#[link(wasm_import_module = "__wbindgen_placeholder__")]
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
extern "C" {
fn #instanceof_shim(val: u32) -> u32;
}
#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
unsafe fn #instanceof_shim(val: u32) -> u32 {
panic!("cannot check instanceof on non-wasm targets");
}
unsafe {
let idx = val.into_abi(&mut wasm_bindgen::convert::GlobalStack::new());
#instanceof_shim(idx) != 0
}
}

#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
fn instanceof(val: &JsValue) -> bool {
drop(val);
panic!("cannot check instanceof on non-wasm targets");
}

#is_type_of

#[inline]
Expand Down Expand Up @@ -998,6 +996,7 @@ impl TryToTokens for ast::ImportFunction {
let import_name = &self.shim;
let attrs = &self.function.rust_attrs;
let arguments = &arguments;
let abi_arguments = &abi_arguments;

let doc_comment = match &self.doc_comment {
None => "",
Expand All @@ -1009,17 +1008,45 @@ impl TryToTokens for ast::ImportFunction {
quote!()
};

// Route any errors pointing to this imported function to the identifier
// of the function we're imported from so we at least know what function
// is causing issues.
//
// Note that this is where type errors like "doesn't implement
// FromWasmAbi" or "doesn't implement IntoWasmAbi" currently get routed.
// I suspect that's because they show up in the signature via trait
// projections as types of arguments, and all that needs to typecheck
// before the body can be typechecked. Due to rust-lang/rust#60980 (and
// probably related issues) we can't really get a precise span.
//
// Ideally what we want is to point errors for particular types back to
// the specific argument/type that generated the error, but it looks
// like rustc itself doesn't do great in that regard so let's just do
// the best we can in the meantime.
let extern_fn = respan(
quote! {
#[link(wasm_import_module = "__wbindgen_placeholder__")]
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
extern "C" {
fn #import_name(#(#abi_arguments),*) -> #abi_ret;
}
#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
unsafe fn #import_name(#(#abi_arguments),*) -> #abi_ret {
panic!("cannot call wasm-bindgen imported functions on \
non-wasm targets");
}
},
&self.rust_name,
);

let invocation = quote! {
#(#attrs)*
#[allow(bad_style)]
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
#[doc = #doc_comment]
#[allow(clippy::all)]
#vis fn #rust_name(#me #(#arguments),*) #ret {
#[link(wasm_import_module = "__wbindgen_placeholder__")]
extern "C" {
fn #import_name(#(#abi_arguments),*) -> #abi_ret;
}
#extern_fn

unsafe {
#exn_data
let #ret_ident = {
Expand All @@ -1031,17 +1058,6 @@ impl TryToTokens for ast::ImportFunction {
#convert_ret
}
}

#(#attrs)*
#[allow(bad_style, unused_variables)]
#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
#[doc = #doc_comment]
#[allow(clippy::all)]
#vis fn #rust_name(#me #(#arguments),*) #ret {
panic!("cannot call wasm-bindgen imported functions on \
non-wasm targets");
}

};

if let Some(class) = class_ty {
Expand Down Expand Up @@ -1164,12 +1180,17 @@ impl ToTokens for ast::ImportStatic {
#[allow(bad_style)]
#[allow(clippy::all)]
#vis static #name: wasm_bindgen::JsStatic<#ty> = {
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
fn init() -> #ty {
#[link(wasm_import_module = "__wbindgen_placeholder__")]
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
extern "C" {
fn #shim_name() -> <#ty as wasm_bindgen::convert::FromWasmAbi>::Abi;
}
#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
unsafe fn #shim_name() -> <#ty as wasm_bindgen::convert::FromWasmAbi>::Abi {
panic!("cannot access imported statics on non-wasm targets")
}

unsafe {
<#ty as wasm_bindgen::convert::FromWasmAbi>::from_abi(
#shim_name(),
Expand All @@ -1178,10 +1199,6 @@ impl ToTokens for ast::ImportStatic {

}
}
#[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))]
fn init() -> #ty {
panic!("cannot access imported statics on non-wasm targets")
}
thread_local!(static _VAL: #ty = init(););
wasm_bindgen::JsStatic {
__inner: &_VAL,
Expand Down Expand Up @@ -1449,6 +1466,8 @@ impl<'a, T: ToTokens> ToTokens for Descriptor<'a, T> {
}
}

/// Converts `span` into a stream of tokens, and attempts to ensure that `input`
/// has all the appropriate span information so errors in it point to `span`.
fn respan(input: TokenStream, span: &dyn ToTokens) -> TokenStream {
let mut first_span = Span::call_site();
let mut last_span = Span::call_site();
Expand Down
9 changes: 9 additions & 0 deletions crates/macro/ui-tests/missing-catch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
extern "C" {
#[wasm_bindgen]
pub fn foo() -> Result<JsValue, JsValue>;
}

fn main() {}
7 changes: 7 additions & 0 deletions crates/macro/ui-tests/missing-catch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
error[E0277]: the trait bound `std::result::Result<wasm_bindgen::JsValue, wasm_bindgen::JsValue>: wasm_bindgen::convert::traits::FromWasmAbi` is not satisfied
--> $DIR/missing-catch.rs:6:9
|
6 | pub fn foo() -> Result<JsValue, JsValue>;
| ^^^ the trait `wasm_bindgen::convert::traits::FromWasmAbi` is not implemented for `std::result::Result<wasm_bindgen::JsValue, wasm_bindgen::JsValue>`

For more information about this error, try `rustc --explain E0277`.
11 changes: 11 additions & 0 deletions crates/macro/ui-tests/traits-not-implemented.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use wasm_bindgen::prelude::*;

struct A;

#[wasm_bindgen]
extern "C" {
#[wasm_bindgen]
pub fn foo(a: A);
}

fn main() {}
7 changes: 7 additions & 0 deletions crates/macro/ui-tests/traits-not-implemented.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
error[E0277]: the trait bound `A: wasm_bindgen::convert::traits::IntoWasmAbi` is not satisfied
--> $DIR/traits-not-implemented.rs:8:12
|
8 | pub fn foo(a: A);
| ^^^ the trait `wasm_bindgen::convert::traits::IntoWasmAbi` is not implemented for `A`

For more information about this error, try `rustc --explain E0277`.
10 changes: 10 additions & 0 deletions crates/macro/ui-tests/unknown-type-in-import.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
extern "C" {
#[wasm_bindgen]
pub fn foo(a: A);
}

fn main() {}

7 changes: 7 additions & 0 deletions crates/macro/ui-tests/unknown-type-in-import.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
error[E0412]: cannot find type `A` in this scope
--> $DIR/unknown-type-in-import.rs:6:19
|
6 | pub fn foo(a: A);
| ^ not found in this scope

For more information about this error, try `rustc --explain E0412`.

0 comments on commit bc5f73e

Please sign in to comment.