Auto merge of #52445 - alexcrichton:wasm-import-module, r=eddyb

rustc: Stabilize #[wasm_import_module] as #[link(...)]

This commit stabilizes the `#[wasm_import_module]` attribute as
`#[link(wasm_import_module = "...")]`. Tracked by #52090 this new directive in
the `#[link]` attribute is used to configured the module name that the imports
are listed with. The WebAssembly specification indicates two utf-8 names are
associated with all imported items, one for the module the item comes from and
one for the item itself. The item itself is configurable in Rust via its
identifier or `#[link_name = "..."]`, but the module name was previously not
configurable and defaulted to `"env"`. This commit ensures that this is also
configurable.

Closes #52090

Author: bors

src/librustc/hir/check_attr.rs

@@ -65,35 +65,14 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
                 .emit();
         }
 
-        let mut has_wasm_import_module = false;
         for attr in &item.attrs {
             if attr.check_name("inline") {
                 self.check_inline(attr, &item.span, target)
             } else if attr.check_name("non_exhaustive") {
                 self.check_non_exhaustive(attr, item, target)
-            } else if attr.check_name("wasm_import_module") {
-                has_wasm_import_module = true;
-                if attr.value_str().is_none() {
-                    self.tcx.sess.span_err(attr.span, "\
-                        must be of the form #[wasm_import_module = \"...\"]");
-                }
-                if target != Target::ForeignMod {
-                    self.tcx.sess.span_err(attr.span, "\
-                        must only be attached to foreign modules");
-                }
             }
         }
 
-        if target == Target::ForeignMod &&
-            !has_wasm_import_module &&
-            self.tcx.sess.target.target.arch == "wasm32" &&
-            false // FIXME: eventually enable this warning when stable
-        {
-            self.tcx.sess.span_warn(item.span, "\
-                must have a #[wasm_import_module = \"...\"] attribute, this \
-                will become a hard error before too long");
-        }
-
         self.check_repr(item, target);
         self.check_used(item, target);
     }

src/librustc/ich/impls_cstore.rs

@@ -31,7 +31,8 @@ impl_stable_hash_for!(struct middle::cstore::NativeLibrary {
     kind,
     name,
     cfg,
-    foreign_module
+    foreign_module,
+    wasm_import_module
 });
 
 impl_stable_hash_for!(struct middle::cstore::ForeignModule {

src/librustc/middle/cstore.rs

@@ -128,9 +128,10 @@ pub enum NativeLibraryKind {
 #[derive(Clone, RustcEncodable, RustcDecodable)]
 pub struct NativeLibrary {
     pub kind: NativeLibraryKind,
-    pub name: Symbol,
+    pub name: Option<Symbol>,
     pub cfg: Option<ast::MetaItem>,
     pub foreign_module: Option<DefId>,
+    pub wasm_import_module: Option<Symbol>,
 }
 
 #[derive(Clone, Hash, RustcEncodable, RustcDecodable)]

src/librustc_codegen_llvm/attributes.rs

@@ -226,13 +226,22 @@ pub fn provide(providers: &mut Providers) {
 
 pub fn provide_extern(providers: &mut Providers) {
     providers.wasm_import_module_map = |tcx, cnum| {
+        // Build up a map from DefId to a `NativeLibrary` structure, where
+        // `NativeLibrary` internally contains information about
+        // `#[link(wasm_import_module = "...")]` for example.
+        let native_libs = tcx.native_libraries(cnum);
+        let mut def_id_to_native_lib = FxHashMap();
+        for lib in native_libs.iter() {
+            if let Some(id) = lib.foreign_module {
+                def_id_to_native_lib.insert(id, lib);
+            }
+        }
+
         let mut ret = FxHashMap();
         for lib in tcx.foreign_modules(cnum).iter() {
-            let attrs = tcx.get_attrs(lib.def_id);
-            let mut module = None;
-            for attr in attrs.iter().filter(|a| a.check_name("wasm_import_module")) {
-                module = attr.value_str();
-            }
+            let module = def_id_to_native_lib
+                .get(&lib.def_id)
+                .and_then(|s| s.wasm_import_module);
             let module = match module {
                 Some(s) => s,
                 None => continue,
@@ -244,7 +253,7 @@ pub fn provide_extern(providers: &mut Providers) {
         }
 
         Lrc::new(ret)
-    }
+    };
 }
 
 fn wasm_import_module(tcx: TyCtxt, id: DefId) -> Option<CString> {

src/librustc_codegen_llvm/back/link.rs

@@ -449,7 +449,9 @@ fn link_rlib<'a>(sess: &'a Session,
             NativeLibraryKind::NativeFramework |
             NativeLibraryKind::NativeUnknown => continue,
         }
-        ab.add_native_library(&lib.name.as_str());
+        if let Some(name) = lib.name {
+            ab.add_native_library(&name.as_str());
+        }
     }
 
     // After adding all files to the archive, we need to update the
@@ -583,21 +585,24 @@ fn link_staticlib(sess: &Session,
 fn print_native_static_libs(sess: &Session, all_native_libs: &[NativeLibrary]) {
     let lib_args: Vec<_> = all_native_libs.iter()
         .filter(|l| relevant_lib(sess, l))
-        .filter_map(|lib| match lib.kind {
-            NativeLibraryKind::NativeStaticNobundle |
-            NativeLibraryKind::NativeUnknown => {
-                if sess.target.target.options.is_like_msvc {
-                    Some(format!("{}.lib", lib.name))
-                } else {
-                    Some(format!("-l{}", lib.name))
-                }
-            },
-            NativeLibraryKind::NativeFramework => {
-                // ld-only syntax, since there are no frameworks in MSVC
-                Some(format!("-framework {}", lib.name))
-            },
-            // These are included, no need to print them
-            NativeLibraryKind::NativeStatic => None,
+        .filter_map(|lib| {
+            let name = lib.name?;
+            match lib.kind {
+                NativeLibraryKind::NativeStaticNobundle |
+                NativeLibraryKind::NativeUnknown => {
+                    if sess.target.target.options.is_like_msvc {
+                        Some(format!("{}.lib", name))
+                    } else {
+                        Some(format!("-l{}", name))
+                    }
+                },
+                NativeLibraryKind::NativeFramework => {
+                    // ld-only syntax, since there are no frameworks in MSVC
+                    Some(format!("-framework {}", name))
+                },
+                // These are included, no need to print them
+                NativeLibraryKind::NativeStatic => None,
+            }
         })
         .collect();
     if !lib_args.is_empty() {
@@ -1211,11 +1216,15 @@ fn add_local_native_libraries(cmd: &mut dyn Linker,
 
     let search_path = archive_search_paths(sess);
     for lib in relevant_libs {
+        let name = match lib.name {
+            Some(ref l) => l,
+            None => continue,
+        };
         match lib.kind {
-            NativeLibraryKind::NativeUnknown => cmd.link_dylib(&lib.name.as_str()),
-            NativeLibraryKind::NativeFramework => cmd.link_framework(&lib.name.as_str()),
-            NativeLibraryKind::NativeStaticNobundle => cmd.link_staticlib(&lib.name.as_str()),
-            NativeLibraryKind::NativeStatic => cmd.link_whole_staticlib(&lib.name.as_str(),
+            NativeLibraryKind::NativeUnknown => cmd.link_dylib(&name.as_str()),
+            NativeLibraryKind::NativeFramework => cmd.link_framework(&name.as_str()),
+            NativeLibraryKind::NativeStaticNobundle => cmd.link_staticlib(&name.as_str()),
+            NativeLibraryKind::NativeStatic => cmd.link_whole_staticlib(&name.as_str(),
                                                                         &search_path)
         }
     }
@@ -1578,19 +1587,23 @@ fn add_upstream_native_libraries(cmd: &mut dyn Linker,
     let crates = &codegen_results.crate_info.used_crates_static;
     for &(cnum, _) in crates {
         for lib in codegen_results.crate_info.native_libraries[&cnum].iter() {
+            let name = match lib.name {
+                Some(ref l) => l,
+                None => continue,
+            };
             if !relevant_lib(sess, &lib) {
                 continue
             }
             match lib.kind {
-                NativeLibraryKind::NativeUnknown => cmd.link_dylib(&lib.name.as_str()),
-                NativeLibraryKind::NativeFramework => cmd.link_framework(&lib.name.as_str()),
+                NativeLibraryKind::NativeUnknown => cmd.link_dylib(&name.as_str()),
+                NativeLibraryKind::NativeFramework => cmd.link_framework(&name.as_str()),
                 NativeLibraryKind::NativeStaticNobundle => {
                     // Link "static-nobundle" native libs only if the crate they originate from
                     // is being linked statically to the current crate.  If it's linked dynamically
                     // or is an rlib already included via some other dylib crate, the symbols from
                     // native libs will have already been included in that dylib.
                     if data[cnum.as_usize() - 1] == Linkage::Static {
-                        cmd.link_staticlib(&lib.name.as_str())
+                        cmd.link_staticlib(&name.as_str())
                     }
                 },
                 // ignore statically included native libraries here as we've

src/librustc_codegen_llvm/back/wasm.rs

@@ -34,9 +34,9 @@ const WASM_EXTERNAL_KIND_GLOBAL: u8 = 3;
 ///
 /// This function is intended as a hack for now where we manually rewrite the
 /// wasm output by LLVM to have the correct import modules listed. The
-/// `#[wasm_import_module]` attribute in Rust translates to the module that each
-/// symbol is imported from, so here we manually go through the wasm file,
-/// decode it, rewrite imports, and then rewrite the wasm module.
+/// `#[link(wasm_import_module = "...")]` attribute in Rust translates to the
+/// module that each symbol is imported from, so here we manually go through the
+/// wasm file, decode it, rewrite imports, and then rewrite the wasm module.
 ///
 /// Support for this was added to LLVM in
 /// https://github.com/llvm-mirror/llvm/commit/0f32e1365, although support still

src/librustc_metadata/native_libs.rs

@@ -61,56 +61,75 @@ impl<'a, 'tcx> ItemLikeVisitor<'tcx> for Collector<'a, 'tcx> {
                 Some(item) => item,
                 None => continue,
             };
-            let kind = items.iter().find(|k| {
-                k.check_name("kind")
-            }).and_then(|a| a.value_str()).map(Symbol::as_str);
-            let kind = match kind.as_ref().map(|s| &s[..]) {
-                Some("static") => cstore::NativeStatic,
-                Some("static-nobundle") => cstore::NativeStaticNobundle,
-                Some("dylib") => cstore::NativeUnknown,
-                Some("framework") => cstore::NativeFramework,
-                Some(k) => {
-                    struct_span_err!(self.tcx.sess, m.span, E0458,
-                              "unknown kind: `{}`", k)
-                        .span_label(m.span, "unknown kind").emit();
-                    cstore::NativeUnknown
-                }
-                None => cstore::NativeUnknown
-            };
-            let n = items.iter().find(|n| {
-                n.check_name("name")
-            }).and_then(|a| a.value_str());
-            let n = match n {
-                Some(n) => n,
-                None => {
-                    struct_span_err!(self.tcx.sess, m.span, E0459,
-                                     "#[link(...)] specified without `name = \"foo\"`")
-                        .span_label(m.span, "missing `name` argument").emit();
-                    Symbol::intern("foo")
-                }
+            let mut lib = NativeLibrary {
+                name: None,
+                kind: cstore::NativeUnknown,
+                cfg: None,
+                foreign_module: Some(self.tcx.hir.local_def_id(it.id)),
+                wasm_import_module: None,
             };
-            let cfg = items.iter().find(|k| {
-                k.check_name("cfg")
-            }).and_then(|a| a.meta_item_list());
-            let cfg = if let Some(list) = cfg {
-                if list.is_empty() {
-                    self.tcx.sess.span_err(m.span(), "`cfg()` must have an argument");
-                    return;
-                } else if let cfg @ Some(..) = list[0].meta_item() {
-                    cfg.cloned()
+            let mut kind_specified = false;
+
+            for item in items.iter() {
+                if item.check_name("kind") {
+                    kind_specified = true;
+                    let kind = match item.value_str() {
+                        Some(name) => name,
+                        None => continue, // skip like historical compilers
+                    };
+                    lib.kind = match &kind.as_str()[..] {
+                        "static" => cstore::NativeStatic,
+                        "static-nobundle" => cstore::NativeStaticNobundle,
+                        "dylib" => cstore::NativeUnknown,
+                        "framework" => cstore::NativeFramework,
+                        k => {
+                            struct_span_err!(self.tcx.sess, m.span, E0458,
+                                      "unknown kind: `{}`", k)
+                                .span_label(item.span, "unknown kind").emit();
+                            cstore::NativeUnknown
+                        }
+                    };
+                } else if item.check_name("name") {
+                    lib.name = item.value_str();
+                } else if item.check_name("cfg") {
+                    let cfg = match item.meta_item_list() {
+                        Some(list) => list,
+                        None => continue, // skip like historical compilers
+                    };
+                    if cfg.is_empty() {
+                        self.tcx.sess.span_err(
+                            item.span(),
+                            "`cfg()` must have an argument",
+                        );
+                    } else if let cfg @ Some(..) = cfg[0].meta_item() {
+                        lib.cfg = cfg.cloned();
+                    } else {
+                        self.tcx.sess.span_err(cfg[0].span(), "invalid argument for `cfg(..)`");
+                    }
+                } else if item.check_name("wasm_import_module") {
+                    match item.value_str() {
+                        Some(s) => lib.wasm_import_module = Some(s),
+                        None => {
+                            let msg = "must be of the form #[link(wasm_import_module = \"...\")]";
+                            self.tcx.sess.span_err(item.span(), msg);
+                        }
+                    }
                 } else {
-                    self.tcx.sess.span_err(list[0].span(), "invalid argument for `cfg(..)`");
-                    return;
+                    // currently, like past compilers, ignore unknown
+                    // directives here.
                 }
-            } else {
-                None
-            };
-            let lib = NativeLibrary {
-                name: n,
-                kind,
-                cfg,
-                foreign_module: Some(self.tcx.hir.local_def_id(it.id)),
-            };
+            }
+
+            // In general we require #[link(name = "...")] but we allow
+            // #[link(wasm_import_module = "...")] without the `name`.
+            let requires_name = kind_specified || lib.wasm_import_module.is_none();
+            if lib.name.is_none() && requires_name {
+                struct_span_err!(self.tcx.sess, m.span, E0459,
+                                 "#[link(...)] specified without \
+                                  `name = \"foo\"`")
+                    .span_label(m.span, "missing `name` argument")
+                    .emit();
+            }
             self.register_native_lib(Some(m.span), lib);
         }
     }
@@ -121,7 +140,7 @@ impl<'a, 'tcx> ItemLikeVisitor<'tcx> for Collector<'a, 'tcx> {
 
 impl<'a, 'tcx> Collector<'a, 'tcx> {
     fn register_native_lib(&mut self, span: Option<Span>, lib: NativeLibrary) {
-        if lib.name.as_str().is_empty() {
+        if lib.name.as_ref().map(|s| s.as_str().is_empty()).unwrap_or(false) {
             match span {
                 Some(span) => {
                     struct_span_err!(self.tcx.sess, span, E0454,
@@ -167,10 +186,14 @@ impl<'a, 'tcx> Collector<'a, 'tcx> {
         let mut renames = FxHashSet();
         for &(ref name, ref new_name, _) in &self.tcx.sess.opts.libs {
             if let &Some(ref new_name) = new_name {
+                let any_duplicate = self.libs
+                    .iter()
+                    .filter_map(|lib| lib.name.as_ref())
+                    .any(|n| n == name);
                 if new_name.is_empty() {
                     self.tcx.sess.err(
                         &format!("an empty renaming target was specified for library `{}`",name));
-                } else if !self.libs.iter().any(|lib| lib.name == name as &str) {
+                } else if !any_duplicate {
                     self.tcx.sess.err(&format!("renaming of the library `{}` was specified, \
                                                 however this crate contains no #[link(...)] \
                                                 attributes referencing this library.", name));
@@ -189,14 +212,18 @@ impl<'a, 'tcx> Collector<'a, 'tcx> {
         for &(ref name, ref new_name, kind) in &self.tcx.sess.opts.libs {
             let mut found = false;
             for lib in self.libs.iter_mut() {
-                if lib.name == name as &str {
+                let lib_name = match lib.name {
+                    Some(n) => n,
+                    None => continue,
+                };
+                if lib_name == name as &str {
                     let mut changed = false;
                     if let Some(k) = kind {
                         lib.kind = k;
                         changed = true;
                     }
                     if let &Some(ref new_name) = new_name {
-                        lib.name = Symbol::intern(new_name);
+                        lib.name = Some(Symbol::intern(new_name));
                         changed = true;
                     }
                     if !changed {
@@ -212,10 +239,11 @@ impl<'a, 'tcx> Collector<'a, 'tcx> {
                 // Add if not found
                 let new_name = new_name.as_ref().map(|s| &**s); // &Option<String> -> Option<&str>
                 let lib = NativeLibrary {
-                    name: Symbol::intern(new_name.unwrap_or(name)),
+                    name: Some(Symbol::intern(new_name.unwrap_or(name))),
                     kind: if let Some(k) = kind { k } else { cstore::NativeUnknown },
                     cfg: None,
                     foreign_module: None,
+                    wasm_import_module: None,
                 };
                 self.register_native_lib(None, lib);
             }

src/libsyntax/feature_gate.rs

@@ -407,9 +407,6 @@ declare_features! (
     // `use path as _;` and `extern crate c as _;`
     (active, underscore_imports, "1.26.0", Some(48216), None),
 
-    // The #![wasm_import_module] attribute
-    (active, wasm_import_module, "1.26.0", Some(52090), None),
-
     // Allows keywords to be escaped for use as identifiers
     (active, raw_identifiers, "1.26.0", Some(48589), None),
 
@@ -972,10 +969,6 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG
         "the `#[no_debug]` attribute was an experimental feature that has been \
          deprecated due to lack of demand",
         cfg_fn!(no_debug))),
-    ("wasm_import_module", Normal, Gated(Stability::Unstable,
-                                 "wasm_import_module",
-                                 "experimental attribute",
-                                 cfg_fn!(wasm_import_module))),
     ("omit_gdb_pretty_printer_section", Whitelisted, Gated(Stability::Unstable,
                                                        "omit_gdb_pretty_printer_section",
                                                        "the `#[omit_gdb_pretty_printer_section]` \

src/test/run-make/wasm-import-module/bar.rs

@@ -9,12 +9,11 @@
 // except according to those terms.
 
 #![crate_type = "cdylib"]
-#![feature(wasm_import_module)]
 #![deny(warnings)]
 
 extern crate foo;
 
-#[wasm_import_module = "./me"]
+#[link(wasm_import_module = "./me")]
 extern {
     #[link_name = "me_in_dep"]
     fn dep();

src/test/run-make/wasm-import-module/foo.rs

@@ -9,10 +9,9 @@
 // except according to those terms.
 
 #![crate_type = "rlib"]
-#![feature(wasm_import_module)]
 #![deny(warnings)]
 
-#[wasm_import_module = "./dep"]
+#[link(wasm_import_module = "./dep")]
 extern {
     pub fn dep();
 }

src/test/ui/error-codes/E0458.stderr

@@ -2,7 +2,9 @@ error[E0458]: unknown kind: `wonderful_unicorn`
   --> $DIR/E0458.rs:11:1
    |
 LL | #[link(kind = "wonderful_unicorn")] extern {} //~ ERROR E0458
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unknown kind
+   | ^^^^^^^--------------------------^^
+   |        |
+   |        unknown kind
 
 error[E0459]: #[link(...)] specified without `name = "foo"`
   --> $DIR/E0458.rs:11:1

src/test/ui/feature-gate-wasm_import_module.rs

@@ -1,8 +1,8 @@
 error: `cfg()` must have an argument
-  --> $DIR/issue-43926.rs:11:1
+  --> $DIR/issue-43926.rs:11:20
    |
 LL | #[link(name="foo", cfg())] //~ ERROR `cfg()` must have an argument
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                    ^^^^^
 
 error: aborting due to previous error
 

src/test/ui/feature-gate-wasm_import_module.stderr

@@ -8,13 +8,14 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![feature(wasm_import_module)]
+#[link(name = "...", wasm_import_module)] //~ ERROR: must be of the form
+extern {}
 
-#[wasm_import_module] //~ ERROR: must be of the form
+#[link(name = "...", wasm_import_module(x))] //~ ERROR: must be of the form
 extern {}
 
-#[wasm_import_module = "foo"] //~ ERROR: must only be attached to
-fn foo() {}
+#[link(name = "...", wasm_import_module())] //~ ERROR: must be of the form
+extern {}
 
 fn main() {}
 

src/test/ui/issue-43926.stderr

@@ -1,14 +1,20 @@
-error: must be of the form #[wasm_import_module = "..."]
-  --> $DIR/wasm-import-module.rs:13:1
+error: must be of the form #[link(wasm_import_module = "...")]
+  --> $DIR/wasm-import-module.rs:11:22
    |
-LL | #[wasm_import_module] //~ ERROR: must be of the form
-   | ^^^^^^^^^^^^^^^^^^^^^
+LL | #[link(name = "...", wasm_import_module)] //~ ERROR: must be of the form
+   |                      ^^^^^^^^^^^^^^^^^^
 
-error: must only be attached to foreign modules
-  --> $DIR/wasm-import-module.rs:16:1
+error: must be of the form #[link(wasm_import_module = "...")]
+  --> $DIR/wasm-import-module.rs:14:22
    |
-LL | #[wasm_import_module = "foo"] //~ ERROR: must only be attached to
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | #[link(name = "...", wasm_import_module(x))] //~ ERROR: must be of the form
+   |                      ^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: must be of the form #[link(wasm_import_module = "...")]
+  --> $DIR/wasm-import-module.rs:17:22
+   |
+LL | #[link(name = "...", wasm_import_module())] //~ ERROR: must be of the form
+   |                      ^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors