Fix symbol conflicts defined in two crates (#496)

Not a lot of attention has been paid to dealing with conflicts of symbols
between crates and different `#[wasm_bindgen]` blocks. This commit starts to fix
this issue by unblocking #486 which first ran into this. Currently there's a bug
where if two independent crates bind the same JS API they'll generate the same
symbol which causes conflicts for things like LTO or linking in general.

This commit starts to add a "salt" to all symbols generated by `wasm-bindgen`
(these are all transparent to the user) to ensure that each crate's invocations
are kept apart from one another and using the correct bindings.

crates/backend/src/codegen.rs

@@ -1,6 +1,4 @@
-use std::borrow::Cow;
 use std::collections::HashSet;
-use std::env;
 use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};
 
 use ast;
@@ -9,24 +7,7 @@ use quote::ToTokens;
 use serde_json;
 use shared;
 use syn;
-
+use util::ShortHash;
-fn to_ident_name(s: &str) -> Cow<str> {
-    if s.chars().all(|c| match c {
-        'a'...'z' | 'A'...'Z' | '0'...'9' | '_' => true,
-        _ => false,
-    }) {
-        return Cow::from(s);
-    }
-
-    Cow::from(
-        s.chars()
-            .map(|c| match c {
-                'a'...'z' | 'A'...'Z' | '0'...'9' | '_' => c,
-                _ => '_',
-            })
-            .collect::<String>(),
-    )
-}
 
 impl ToTokens for ast::Program {
     // Generate wrappers for all the items that we've found
@@ -72,15 +53,9 @@ impl ToTokens for ast::Program {
 
         static CNT: AtomicUsize = ATOMIC_USIZE_INIT;
 
-        let crate_name = env::var("CARGO_PKG_NAME").expect("should have CARGO_PKG_NAME env var");
-        let crate_vers =
-            env::var("CARGO_PKG_VERSION").expect("should have CARGO_PKG_VERSION env var");
-
         let generated_static_name = format!(
-            "__WASM_BINDGEN_GENERATED_{}_{}_{}",
+            "__WASM_BINDGEN_GENERATED_{}",
-            to_ident_name(&crate_name),
+            ShortHash(CNT.fetch_add(1, Ordering::SeqCst)),
-            to_ident_name(&crate_vers),
-            CNT.fetch_add(1, Ordering::SeqCst)
         );
         let generated_static_name = Ident::new(&generated_static_name, Span::call_site());
 

crates/backend/src/util.rs

@@ -1,4 +1,11 @@
+use std::collections::hash_map::DefaultHasher;
+use std::env;
+use std::fmt;
+use std::hash::{Hash, Hasher};
 use std::iter::FromIterator;
+use std::sync::atomic::Ordering::SeqCst;
+use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT};
+use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT};
 
 use ast;
 use proc_macro2::{self, Ident};
@@ -88,3 +95,37 @@ pub fn wrap_import_function(function: ast::ImportFunction) -> ast::Import {
         kind: ast::ImportKind::Function(function),
     }
 }
+
+/// Small utility used when generating symbol names.
+///
+/// Hashes the public field here along with a few cargo-set env vars to
+/// distinguish between runs of the procedural macro.
+pub struct ShortHash<T>(pub T);
+
+impl<T: Hash> fmt::Display for ShortHash<T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        static HASHED: AtomicBool = ATOMIC_BOOL_INIT;
+        static HASH: AtomicUsize = ATOMIC_USIZE_INIT;
+
+        // Try to amortize the cost of loading env vars a lot as we're gonna be
+        // hashing for a lot of symbols.
+        if !HASHED.load(SeqCst) {
+            let mut h = DefaultHasher::new();
+            env::var("CARGO_PKG_NAME")
+                .expect("should have CARGO_PKG_NAME env var")
+                .hash(&mut h);
+            env::var("CARGO_PKG_VERSION")
+                .expect("should have CARGO_PKG_VERSION env var")
+                .hash(&mut h);
+            // This may chop off 32 bits on 32-bit platforms, but that's ok, we
+            // just want something to mix in below anyway.
+            HASH.store(h.finish() as usize, SeqCst);
+            HASHED.store(true, SeqCst);
+        }
+
+        let mut h = DefaultHasher::new();
+        HASH.load(SeqCst).hash(&mut h);
+        self.0.hash(&mut h);
+        write!(f, "{:016x}", h.finish())
+    }
+}

crates/macro/src/parser.rs

@@ -1,4 +1,5 @@
-use backend::{ast, util::ident_ty};
+use backend::ast;
+use backend::util::{ident_ty, ShortHash};
 use proc_macro2::{Ident, Span, TokenStream, TokenTree};
 use quote::ToTokens;
 use shared;
@@ -427,10 +428,11 @@ impl ConvertToAst<BindgenAttrs> for syn::ForeignItemFn {
 
         let shim = {
             let ns = match kind {
-                ast::ImportFunctionKind::Normal => "n",
+                ast::ImportFunctionKind::Normal => (0, "n"),
-                ast::ImportFunctionKind::Method { ref class, .. } => class,
+                ast::ImportFunctionKind::Method { ref class, .. } => (1, &class[..]),
             };
-            format!("__wbg_f_{}_{}_{}", js_name, self.ident, ns)
+            let data = (ns, &self.ident);
+            format!("__wbg_{}_{}", js_name, ShortHash(data))
         };
         ast::ImportKind::Function(ast::ImportFunction {
             function: wasm,

tests/all/dependencies.rs

@@ -90,3 +90,120 @@ fn dependencies_work() {
         )
         .test();
 }
+
+#[test]
+fn same_api_two_crates() {
+    project()
+        .file(
+            "src/lib.rs",
+            r#"
+                #![feature(use_extern_macros, wasm_custom_section, wasm_import_module)]
+                extern crate wasm_bindgen;
+                extern crate a;
+                extern crate b;
+
+                use wasm_bindgen::prelude::*;
+
+                #[wasm_bindgen(module = "./foo")]
+                extern {
+                    fn assert_next_undefined();
+                    fn assert_next_ten();
+                }
+
+                #[wasm_bindgen]
+                pub fn test() {
+                    assert_next_undefined();
+                    a::test();
+                    assert_next_ten();
+                    b::test();
+                }
+            "#,
+        )
+        .file(
+            "foo.js",
+            r#"
+                import { strictEqual } from "assert";
+
+                let next = null;
+
+                export function assert_next_undefined() {
+                    next = undefined;
+                }
+
+                export function assert_next_ten() {
+                    next = 10;
+                }
+
+                export function foo(a) {
+                    console.log(a, next);
+                    strictEqual(a, next);
+                    next = null;
+                }
+            "#,
+        )
+        .add_local_dependency("a", "a")
+        .file(
+            "a/Cargo.toml",
+            &format!(r#"
+                [package]
+                name = 'a'
+                version = '0.0.0'
+
+                [dependencies]
+                wasm-bindgen = {{ path = '{}' }}
+            "#,
+                env!("CARGO_MANIFEST_DIR")
+            ),
+        )
+        .file(
+            "a/src/lib.rs",
+            "
+                #![feature(use_extern_macros, wasm_custom_section, wasm_import_module)]
+                extern crate wasm_bindgen;
+
+                use wasm_bindgen::prelude::*;
+
+                #[wasm_bindgen(module = \"./foo\")]
+                extern {
+                    fn foo();
+                }
+
+                pub fn test() {
+                    foo();
+                }
+            ",
+        )
+        .add_local_dependency("b", "b")
+        .file(
+            "b/Cargo.toml",
+            &format!(r#"
+                [package]
+                name = 'b'
+                version = '0.0.0'
+
+                [dependencies]
+                wasm-bindgen = {{ path = '{}' }}
+            "#,
+                env!("CARGO_MANIFEST_DIR")
+            ),
+        )
+        .file(
+            "b/src/lib.rs",
+            "
+                #![feature(use_extern_macros, wasm_custom_section, wasm_import_module)]
+                extern crate wasm_bindgen;
+
+                use wasm_bindgen::prelude::*;
+
+                #[wasm_bindgen(module = \"./foo\")]
+                extern {
+                    fn foo(x: u32);
+                }
+
+                pub fn test() {
+                    foo(10);
+                }
+            ",
+        )
+        .test();
+}