From a02d210d5c0b18c08888436800dbbf6ee1c49765 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 20 May 2019 10:16:25 -0700 Subject: [PATCH 1/2] Catch more errors on non-wasm32 platforms This commit tweaks the codegen for imported functions and such (anything that relies on some imported intrinsic or function filled in by the CLI) to share as much code as possible on non-wasm32 platforms. This should help us catch more errors before compiling to wasm and also just make it easier to write UI tests! For example a UI test previously couldn't be written for #1528 but now it can be, and one is include (although the error message is quite bad). --- crates/backend/src/codegen.rs | 55 ++++++++++------------ crates/macro/ui-tests/missing-catch.rs | 9 ++++ crates/macro/ui-tests/missing-catch.stderr | 7 +++ 3 files changed, 41 insertions(+), 30 deletions(-) create mode 100644 crates/macro/ui-tests/missing-catch.rs create mode 100644 crates/macro/ui-tests/missing-catch.stderr diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index 2c67c51a..70e7d29b 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -197,7 +197,6 @@ 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, @@ -205,10 +204,16 @@ impl ToTokens for ast::Struct { ); #[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 { ::from_abi( @@ -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")))] @@ -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] @@ -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 => "", @@ -1012,14 +1011,20 @@ impl TryToTokens for ast::ImportFunction { 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__")] + #[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"); + } + unsafe { #exn_data let #ret_ident = { @@ -1031,17 +1036,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 { @@ -1164,12 +1158,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(), @@ -1178,10 +1177,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, diff --git a/crates/macro/ui-tests/missing-catch.rs b/crates/macro/ui-tests/missing-catch.rs new file mode 100644 index 00000000..962e2fad --- /dev/null +++ b/crates/macro/ui-tests/missing-catch.rs @@ -0,0 +1,9 @@ +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen] + pub fn foo() -> Result; +} + +fn main() {} diff --git a/crates/macro/ui-tests/missing-catch.stderr b/crates/macro/ui-tests/missing-catch.stderr new file mode 100644 index 00000000..ac5d52a2 --- /dev/null +++ b/crates/macro/ui-tests/missing-catch.stderr @@ -0,0 +1,7 @@ +error[E0277]: the trait bound `std::result::Result: wasm_bindgen::convert::traits::FromWasmAbi` is not satisfied + --> $DIR/missing-catch.rs:3:1 + | +3 | #[wasm_bindgen] + | ^^^^^^^^^^^^^^^ the trait `wasm_bindgen::convert::traits::FromWasmAbi` is not implemented for `std::result::Result` + +For more information about this error, try `rustc --explain E0277`. From 2cbb8b8a69f2aeeee1fe308348efc662019fa0f6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 20 May 2019 10:49:06 -0700 Subject: [PATCH 2/2] Improve diagnostics with missing trait implementations Rejigger a few spans, work around an odd rustc issue, and hopefully produce higher quality error messages! Closes #1528 --- crates/backend/src/codegen.rs | 36 +++++++++++++++---- crates/macro/ui-tests/missing-catch.stderr | 6 ++-- .../macro/ui-tests/traits-not-implemented.rs | 11 ++++++ .../ui-tests/traits-not-implemented.stderr | 7 ++++ .../macro/ui-tests/unknown-type-in-import.rs | 10 ++++++ .../ui-tests/unknown-type-in-import.stderr | 7 ++++ 6 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 crates/macro/ui-tests/traits-not-implemented.rs create mode 100644 crates/macro/ui-tests/traits-not-implemented.stderr create mode 100644 crates/macro/ui-tests/unknown-type-in-import.rs create mode 100644 crates/macro/ui-tests/unknown-type-in-import.stderr diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index 70e7d29b..98e749fe 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -1008,12 +1008,23 @@ impl TryToTokens for ast::ImportFunction { quote!() }; - let invocation = quote! { - #(#attrs)* - #[allow(bad_style)] - #[doc = #doc_comment] - #[allow(clippy::all)] - #vis fn #rust_name(#me #(#arguments),*) #ret { + // 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" { @@ -1024,6 +1035,17 @@ impl TryToTokens for ast::ImportFunction { panic!("cannot call wasm-bindgen imported functions on \ non-wasm targets"); } + }, + &self.rust_name, + ); + + let invocation = quote! { + #(#attrs)* + #[allow(bad_style)] + #[doc = #doc_comment] + #[allow(clippy::all)] + #vis fn #rust_name(#me #(#arguments),*) #ret { + #extern_fn unsafe { #exn_data @@ -1444,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(); diff --git a/crates/macro/ui-tests/missing-catch.stderr b/crates/macro/ui-tests/missing-catch.stderr index ac5d52a2..b27fbff9 100644 --- a/crates/macro/ui-tests/missing-catch.stderr +++ b/crates/macro/ui-tests/missing-catch.stderr @@ -1,7 +1,7 @@ error[E0277]: the trait bound `std::result::Result: wasm_bindgen::convert::traits::FromWasmAbi` is not satisfied - --> $DIR/missing-catch.rs:3:1 + --> $DIR/missing-catch.rs:6:9 | -3 | #[wasm_bindgen] - | ^^^^^^^^^^^^^^^ the trait `wasm_bindgen::convert::traits::FromWasmAbi` is not implemented for `std::result::Result` +6 | pub fn foo() -> Result; + | ^^^ the trait `wasm_bindgen::convert::traits::FromWasmAbi` is not implemented for `std::result::Result` For more information about this error, try `rustc --explain E0277`. diff --git a/crates/macro/ui-tests/traits-not-implemented.rs b/crates/macro/ui-tests/traits-not-implemented.rs new file mode 100644 index 00000000..56c5900c --- /dev/null +++ b/crates/macro/ui-tests/traits-not-implemented.rs @@ -0,0 +1,11 @@ +use wasm_bindgen::prelude::*; + +struct A; + +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen] + pub fn foo(a: A); +} + +fn main() {} diff --git a/crates/macro/ui-tests/traits-not-implemented.stderr b/crates/macro/ui-tests/traits-not-implemented.stderr new file mode 100644 index 00000000..5aa71503 --- /dev/null +++ b/crates/macro/ui-tests/traits-not-implemented.stderr @@ -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`. diff --git a/crates/macro/ui-tests/unknown-type-in-import.rs b/crates/macro/ui-tests/unknown-type-in-import.rs new file mode 100644 index 00000000..1356a253 --- /dev/null +++ b/crates/macro/ui-tests/unknown-type-in-import.rs @@ -0,0 +1,10 @@ +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen] + pub fn foo(a: A); +} + +fn main() {} + diff --git a/crates/macro/ui-tests/unknown-type-in-import.stderr b/crates/macro/ui-tests/unknown-type-in-import.stderr new file mode 100644 index 00000000..56705e7f --- /dev/null +++ b/crates/macro/ui-tests/unknown-type-in-import.stderr @@ -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`.