From e90985acdec9928da9f6d157cfeb64f0ee98bffe Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 28 Sep 2018 22:17:59 +0300 Subject: [PATCH 1/2] rustc_resolve: move extern_prelude from Resolver to Session. --- src/librustc/session/mod.rs | 19 ++++++++++++++++++- src/librustc_resolve/lib.rs | 20 +++----------------- src/librustc_resolve/macros.rs | 2 +- src/librustc_resolve/resolve_imports.rs | 6 +++--- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index be347253e7f06..20d4ba9c63231 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -38,7 +38,7 @@ use syntax::parse; use syntax::parse::ParseSess; use syntax::{ast, source_map}; use syntax::feature_gate::AttributeType; -use syntax_pos::{MultiSpan, Span}; +use syntax_pos::{MultiSpan, Span, symbol::Symbol}; use util::profiling::SelfProfiler; use rustc_target::spec::PanicStrategy; @@ -168,6 +168,10 @@ pub struct Session { /// Cap lint level specified by a driver specifically. pub driver_lint_caps: FxHashMap, + + /// All the crate names specified with `--extern`, and the builtin ones. + /// Starting with the Rust 2018 edition, absolute paths resolve in this set. + pub extern_prelude: FxHashSet, } pub struct PerfStats { @@ -1126,6 +1130,18 @@ pub fn build_session_( CguReuseTracker::new_disabled() }; + + let mut extern_prelude: FxHashSet = + sopts.externs.iter().map(|kv| Symbol::intern(kv.0)).collect(); + + // HACK(eddyb) this ignores the `no_{core,std}` attributes. + // FIXME(eddyb) warn (somewhere) if core/std is used with `no_{core,std}`. + // if !attr::contains_name(&krate.attrs, "no_core") { + // if !attr::contains_name(&krate.attrs, "no_std") { + extern_prelude.insert(Symbol::intern("core")); + extern_prelude.insert(Symbol::intern("std")); + extern_prelude.insert(Symbol::intern("meta")); + let sess = Session { target: target_cfg, host, @@ -1201,6 +1217,7 @@ pub fn build_session_( has_global_allocator: Once::new(), has_panic_handler: Once::new(), driver_lint_caps: FxHashMap(), + extern_prelude, }; validate_commandline_args_with_session_available(&sess); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 91b0e9c1dca62..7d5a6bb2a63c3 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1350,7 +1350,6 @@ pub struct Resolver<'a, 'b: 'a> { graph_root: Module<'a>, prelude: Option>, - extern_prelude: FxHashSet, /// n.b. This is used only for better diagnostics, not name resolution itself. has_self: FxHashSet, @@ -1663,17 +1662,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { DefCollector::new(&mut definitions, Mark::root()) .collect_root(crate_name, session.local_crate_disambiguator()); - let mut extern_prelude: FxHashSet = - session.opts.externs.iter().map(|kv| Symbol::intern(kv.0)).collect(); - - // HACK(eddyb) this ignore the `no_{core,std}` attributes. - // FIXME(eddyb) warn (elsewhere) if core/std is used with `no_{core,std}`. - // if !attr::contains_name(&krate.attrs, "no_core") { - // if !attr::contains_name(&krate.attrs, "no_std") { - extern_prelude.insert(Symbol::intern("core")); - extern_prelude.insert(Symbol::intern("std")); - extern_prelude.insert(Symbol::intern("meta")); - let mut invocations = FxHashMap(); invocations.insert(Mark::root(), arenas.alloc_invocation_data(InvocationData::root(graph_root))); @@ -1692,7 +1680,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { // AST. graph_root, prelude: None, - extern_prelude, has_self: FxHashSet(), field_names: FxHashMap(), @@ -1963,7 +1950,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { if !module.no_implicit_prelude { // `record_used` means that we don't try to load crates during speculative resolution - if record_used && ns == TypeNS && self.extern_prelude.contains(&ident.name) { + if record_used && ns == TypeNS && self.session.extern_prelude.contains(&ident.name) { let crate_id = self.crate_loader.process_path_extern(ident.name, ident.span); let crate_root = self.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX }); self.populate_module_if_necessary(&crate_root); @@ -3955,7 +3942,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { } else { // Items from the prelude if !module.no_implicit_prelude { - names.extend(self.extern_prelude.iter().cloned()); + names.extend(self.session.extern_prelude.iter().cloned()); if let Some(prelude) = self.prelude { add_module_candidates(prelude, &mut names); } @@ -4401,8 +4388,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { ); if self.session.rust_2018() { - let extern_prelude_names = self.extern_prelude.clone(); - for &name in extern_prelude_names.iter() { + for &name in &self.session.extern_prelude { let ident = Ident::with_empty_ctxt(name); match self.crate_loader.maybe_process_path_extern(name, ident.span) { Some(crate_id) => { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index ed79ff62f176c..7b5e3ca078ba6 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -682,7 +682,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { result } WhereToResolve::ExternPrelude => { - if use_prelude && self.extern_prelude.contains(&ident.name) { + if use_prelude && self.session.extern_prelude.contains(&ident.name) { let crate_id = self.crate_loader.process_path_extern(ident.name, ident.span); let crate_root = diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index dc4a76db69266..e689e6d70fdf4 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -199,7 +199,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { if !( ns == TypeNS && !ident.is_path_segment_keyword() && - self.extern_prelude.contains(&ident.name) + self.session.extern_prelude.contains(&ident.name) ) { // ... unless the crate name is not in the `extern_prelude`. return binding; @@ -218,7 +218,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { } else if ns == TypeNS && !ident.is_path_segment_keyword() && - self.extern_prelude.contains(&ident.name) + self.session.extern_prelude.contains(&ident.name) { let crate_id = self.crate_loader.process_path_extern(ident.name, ident.span); @@ -735,7 +735,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { let uniform_paths_feature = self.session.features_untracked().uniform_paths; for ((span, _, ns), results) in uniform_paths_canaries { let name = results.name; - let external_crate = if ns == TypeNS && self.extern_prelude.contains(&name) { + let external_crate = if ns == TypeNS && self.session.extern_prelude.contains(&name) { let crate_id = self.crate_loader.process_path_extern(name, span); Some(Def::Mod(DefId { krate: crate_id, index: CRATE_DEF_INDEX })) From 81ca8ebee229ec85d9d2c54d7eda7fd0106a34f3 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 28 Sep 2018 22:54:18 +0300 Subject: [PATCH 2/2] rustc_typeck: don't lint non-extern-prelude extern crate's in Rust 2018. --- src/librustc_typeck/check_unused.rs | 16 +++-- .../ui-fulldeps/unnecessary-extern-crate.rs | 22 ++----- .../unnecessary-extern-crate.stderr | 64 ++----------------- .../ui/rust-2018/remove-extern-crate.fixed | 6 ++ src/test/ui/rust-2018/remove-extern-crate.rs | 6 ++ .../ui/rust-2018/remove-extern-crate.stderr | 8 +-- 6 files changed, 34 insertions(+), 88 deletions(-) diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index 5967bd1ba3eea..4b3f08a10ff5e 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -130,15 +130,13 @@ fn unused_crates_lint<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>) { }); for extern_crate in &crates_to_lint { - assert!(extern_crate.def_id.is_local()); + let id = tcx.hir.as_local_node_id(extern_crate.def_id).unwrap(); + let item = tcx.hir.expect_item(id); // If the crate is fully unused, we suggest removing it altogether. // We do this in any edition. if extern_crate.warn_if_unused { if let Some(&span) = unused_extern_crates.get(&extern_crate.def_id) { - assert_eq!(extern_crate.def_id.krate, LOCAL_CRATE); - let hir_id = tcx.hir.definitions().def_index_to_hir_id(extern_crate.def_id.index); - let id = tcx.hir.hir_to_node_id(hir_id); let msg = "unused extern crate"; tcx.struct_span_lint_node(lint, id, span, msg) .span_suggestion_short_with_applicability( @@ -157,6 +155,13 @@ fn unused_crates_lint<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>) { continue; } + // If the extern crate isn't in the extern prelude, + // there is no way it can be written as an `use`. + let orig_name = extern_crate.orig_name.unwrap_or(item.name); + if !tcx.sess.extern_prelude.contains(&orig_name) { + continue; + } + // If the extern crate has any attributes, they may have funky // semantics we can't faithfully represent using `use` (most // notably `#[macro_use]`). Ignore it. @@ -165,9 +170,6 @@ fn unused_crates_lint<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>) { } // Otherwise, we can convert it into a `use` of some kind. - let hir_id = tcx.hir.definitions().def_index_to_hir_id(extern_crate.def_id.index); - let id = tcx.hir.hir_to_node_id(hir_id); - let item = tcx.hir.expect_item(id); let msg = "`extern crate` is not idiomatic in the new edition"; let help = format!( "convert it to a `{}`", diff --git a/src/test/ui-fulldeps/unnecessary-extern-crate.rs b/src/test/ui-fulldeps/unnecessary-extern-crate.rs index df723ddf590c4..1cdc9229d078c 100644 --- a/src/test/ui-fulldeps/unnecessary-extern-crate.rs +++ b/src/test/ui-fulldeps/unnecessary-extern-crate.rs @@ -20,33 +20,23 @@ extern crate alloc as x; //~^ ERROR unused extern crate //~| HELP remove +extern crate proc_macro; + #[macro_use] extern crate test; pub extern crate test as y; -//~^ ERROR `extern crate` is not idiomatic in the new edition -//~| HELP convert it to a `pub use` pub extern crate libc; -//~^ ERROR `extern crate` is not idiomatic in the new edition -//~| HELP convert it to a `pub use` pub(crate) extern crate libc as a; -//~^ ERROR `extern crate` is not idiomatic in the new edition -//~| HELP convert it to a `pub(crate) use` crate extern crate libc as b; -//~^ ERROR `extern crate` is not idiomatic in the new edition -//~| HELP convert it to a `crate use` mod foo { pub(in crate::foo) extern crate libc as c; - //~^ ERROR `extern crate` is not idiomatic in the new edition - //~| HELP convert it to a `pub(in crate::foo) use` pub(super) extern crate libc as d; - //~^ ERROR `extern crate` is not idiomatic in the new edition - //~| HELP convert it to a `pub(super) use` extern crate alloc; //~^ ERROR unused extern crate @@ -57,12 +47,8 @@ mod foo { //~| HELP remove pub extern crate test; - //~^ ERROR `extern crate` is not idiomatic in the new edition - //~| HELP convert it pub extern crate test as y; - //~^ ERROR `extern crate` is not idiomatic in the new edition - //~| HELP convert it mod bar { extern crate alloc; @@ -74,8 +60,6 @@ mod foo { //~| HELP remove pub(in crate::foo::bar) extern crate libc as e; - //~^ ERROR `extern crate` is not idiomatic in the new edition - //~| HELP convert it to a `pub(in crate::foo::bar) use` fn dummy() { unsafe { @@ -96,4 +80,6 @@ mod foo { fn main() { unsafe { a::getpid(); } unsafe { b::getpid(); } + + proc_macro::TokenStream::new(); } diff --git a/src/test/ui-fulldeps/unnecessary-extern-crate.stderr b/src/test/ui-fulldeps/unnecessary-extern-crate.stderr index a4307112157b0..58ec5901585d4 100644 --- a/src/test/ui-fulldeps/unnecessary-extern-crate.stderr +++ b/src/test/ui-fulldeps/unnecessary-extern-crate.stderr @@ -16,83 +16,29 @@ error: unused extern crate LL | extern crate alloc as x; | ^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:26:1 - | -LL | pub extern crate test as y; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `pub use` - -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:30:1 - | -LL | pub extern crate libc; - | ^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `pub use` - -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:34:1 - | -LL | pub(crate) extern crate libc as a; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `pub(crate) use` - -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:38:1 - | -LL | crate extern crate libc as b; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `crate use` - -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:43:5 - | -LL | pub(in crate::foo) extern crate libc as c; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `pub(in crate::foo) use` - -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:47:5 - | -LL | pub(super) extern crate libc as d; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `pub(super) use` - error: unused extern crate - --> $DIR/unnecessary-extern-crate.rs:51:5 + --> $DIR/unnecessary-extern-crate.rs:41:5 | LL | extern crate alloc; | ^^^^^^^^^^^^^^^^^^^ help: remove it error: unused extern crate - --> $DIR/unnecessary-extern-crate.rs:55:5 + --> $DIR/unnecessary-extern-crate.rs:45:5 | LL | extern crate alloc as x; | ^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:59:5 - | -LL | pub extern crate test; - | ^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `pub use` - -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:63:5 - | -LL | pub extern crate test as y; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `pub use` - error: unused extern crate - --> $DIR/unnecessary-extern-crate.rs:68:9 + --> $DIR/unnecessary-extern-crate.rs:54:9 | LL | extern crate alloc; | ^^^^^^^^^^^^^^^^^^^ help: remove it error: unused extern crate - --> $DIR/unnecessary-extern-crate.rs:72:9 + --> $DIR/unnecessary-extern-crate.rs:58:9 | LL | extern crate alloc as x; | ^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it -error: `extern crate` is not idiomatic in the new edition - --> $DIR/unnecessary-extern-crate.rs:76:9 - | -LL | pub(in crate::foo::bar) extern crate libc as e; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `pub(in crate::foo::bar) use` - -error: aborting due to 15 previous errors +error: aborting due to 6 previous errors diff --git a/src/test/ui/rust-2018/remove-extern-crate.fixed b/src/test/ui/rust-2018/remove-extern-crate.fixed index cdae815b200d5..34c7266b637cb 100644 --- a/src/test/ui/rust-2018/remove-extern-crate.fixed +++ b/src/test/ui/rust-2018/remove-extern-crate.fixed @@ -14,6 +14,7 @@ // aux-build:remove-extern-crate.rs // compile-flags:--extern remove_extern_crate +#![feature(alloc)] #![warn(rust_2018_idioms)] @@ -22,11 +23,16 @@ use remove_extern_crate; #[macro_use] extern crate remove_extern_crate as something_else; +// Shouldn't suggest changing to `use`, as the `alloc` +// crate is not in the extern prelude - see #54381. +extern crate alloc; + fn main() { another_name::mem::drop(3); another::foo(); remove_extern_crate::foo!(); bar!(); + alloc::vec![5]; } mod another { diff --git a/src/test/ui/rust-2018/remove-extern-crate.rs b/src/test/ui/rust-2018/remove-extern-crate.rs index 4984da802c05b..570bbb02f7218 100644 --- a/src/test/ui/rust-2018/remove-extern-crate.rs +++ b/src/test/ui/rust-2018/remove-extern-crate.rs @@ -14,6 +14,7 @@ // aux-build:remove-extern-crate.rs // compile-flags:--extern remove_extern_crate +#![feature(alloc)] #![warn(rust_2018_idioms)] extern crate core; @@ -22,11 +23,16 @@ use remove_extern_crate; #[macro_use] extern crate remove_extern_crate as something_else; +// Shouldn't suggest changing to `use`, as the `alloc` +// crate is not in the extern prelude - see #54381. +extern crate alloc; + fn main() { another_name::mem::drop(3); another::foo(); remove_extern_crate::foo!(); bar!(); + alloc::vec![5]; } mod another { diff --git a/src/test/ui/rust-2018/remove-extern-crate.stderr b/src/test/ui/rust-2018/remove-extern-crate.stderr index 064a853625f74..847ba5f3544b2 100644 --- a/src/test/ui/rust-2018/remove-extern-crate.stderr +++ b/src/test/ui/rust-2018/remove-extern-crate.stderr @@ -1,24 +1,24 @@ warning: unused extern crate - --> $DIR/remove-extern-crate.rs:19:1 + --> $DIR/remove-extern-crate.rs:20:1 | LL | extern crate core; | ^^^^^^^^^^^^^^^^^^ help: remove it | note: lint level defined here - --> $DIR/remove-extern-crate.rs:17:9 + --> $DIR/remove-extern-crate.rs:18:9 | LL | #![warn(rust_2018_idioms)] | ^^^^^^^^^^^^^^^^ = note: #[warn(unused_extern_crates)] implied by #[warn(rust_2018_idioms)] warning: `extern crate` is not idiomatic in the new edition - --> $DIR/remove-extern-crate.rs:20:1 + --> $DIR/remove-extern-crate.rs:21:1 | LL | extern crate core as another_name; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert it to a `use` warning: `extern crate` is not idiomatic in the new edition - --> $DIR/remove-extern-crate.rs:33:5 + --> $DIR/remove-extern-crate.rs:39:5 | LL | extern crate core; | ^^^^^^^^^^^^^^^^^^ help: convert it to a `use`