Address review comments, second turn

Author: ishitatsuyuki

src/librustc/ty/context.rs

@@ -797,7 +797,7 @@ pub struct GlobalCtxt<'tcx> {
 
     pub maybe_unused_trait_imports: NodeSet,
 
-    pub maybe_unused_extern_crates: Vec<(NodeId, Span, CrateNum)>,
+    pub maybe_unused_extern_crates: Vec<(NodeId, Span)>,
 
     // Internal cache for metadata decoding. No need to track deps on this.
     pub rcache: RefCell<FxHashMap<ty::CReaderCacheKey, Ty<'tcx>>>,

src/librustc/ty/mod.rs

@@ -131,7 +131,7 @@ pub struct Resolutions {
     pub freevars: FreevarMap,
     pub trait_map: TraitMap,
     pub maybe_unused_trait_imports: NodeSet,
-    pub maybe_unused_extern_crates: Vec<(NodeId, Span, CrateNum)>,
+    pub maybe_unused_extern_crates: Vec<(NodeId, Span)>,
     pub export_map: ExportMap,
 }
 

src/librustc_resolve/build_reduced_graph.rs

@@ -264,7 +264,7 @@ impl<'a> Resolver<'a> {
                     id: item.id,
                     parent,
                     imported_module: Cell::new(Some(module)),
-                    subclass: ImportDirectiveSubclass::ExternCrate { cnum: crate_id },
+                    subclass: ImportDirectiveSubclass::ExternCrate,
                     span: item.span,
                     module_path: Vec::new(),
                     vis: Cell::new(vis),

src/librustc_resolve/check_unused.rs

@@ -120,8 +120,8 @@ pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) {
             _ if directive.used.get() ||
                  directive.vis.get() == ty::Visibility::Public ||
                  directive.span.source_equal(&DUMMY_SP) => {}
-            ImportDirectiveSubclass::ExternCrate { cnum } => {
-                resolver.maybe_unused_extern_crates.push((directive.id, directive.span, cnum));
+            ImportDirectiveSubclass::ExternCrate => {
+                resolver.maybe_unused_extern_crates.push((directive.id, directive.span));
             }
             ImportDirectiveSubclass::MacroUse => {
                 let lint = lint::builtin::UNUSED_IMPORTS;

src/librustc_resolve/lib.rs

@@ -35,7 +35,7 @@ use rustc::middle::cstore::CrateLoader;
 use rustc::session::Session;
 use rustc::lint;
 use rustc::hir::def::*;
-use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, CrateNum, DefId};
+use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId};
 use rustc::ty;
 use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
 use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet, DefIdMap};
@@ -1102,7 +1102,7 @@ impl<'a> NameBinding<'a> {
         match self.kind {
             NameBindingKind::Import {
                 directive: &ImportDirective {
-                    subclass: ImportDirectiveSubclass::ExternCrate { .. }, ..
+                    subclass: ImportDirectiveSubclass::ExternCrate, ..
                 }, ..
             } => true,
             _ => false,
@@ -1250,7 +1250,7 @@ pub struct Resolver<'a> {
 
     used_imports: FxHashSet<(NodeId, Namespace)>,
     pub maybe_unused_trait_imports: NodeSet,
-    pub maybe_unused_extern_crates: Vec<(NodeId, Span, CrateNum)>,
+    pub maybe_unused_extern_crates: Vec<(NodeId, Span)>,
 
     /// privacy errors are delayed until the end in order to deduplicate them
     privacy_errors: Vec<PrivacyError<'a>>,

src/librustc_resolve/resolve_imports.rs

@@ -19,7 +19,7 @@ use {resolve_error, ResolutionError};
 
 use rustc::ty;
 use rustc::lint::builtin::PUB_USE_OF_PRIVATE_EXTERN_CRATE;
-use rustc::hir::def_id::{CrateNum, DefId};
+use rustc::hir::def_id::DefId;
 use rustc::hir::def::*;
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
 
@@ -48,9 +48,7 @@ pub enum ImportDirectiveSubclass<'a> {
         max_vis: Cell<ty::Visibility>, // The visibility of the greatest reexport.
         // n.b. `max_vis` is only used in `finalize_import` to check for reexport errors.
     },
-    ExternCrate {
-        cnum: CrateNum,
-    },
+    ExternCrate,
     MacroUse,
 }
 
@@ -926,7 +924,7 @@ fn import_directive_subclass_to_string(subclass: &ImportDirectiveSubclass) -> St
     match *subclass {
         SingleImport { source, .. } => source.to_string(),
         GlobImport { .. } => "*".to_string(),
-        ExternCrate { .. } => "<extern crate>".to_string(),
+        ExternCrate => "<extern crate>".to_string(),
         MacroUse => "#[macro_use]".to_string(),
     }
 }

src/librustc_typeck/check_unused.rs

@@ -73,10 +73,11 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
     let mut visitor = CheckVisitor { tcx, used_trait_imports };
     tcx.hir.krate().visit_all_item_likes(&mut visitor);
 
-    for &(id, span, cnum) in &tcx.maybe_unused_extern_crates {
-        if !tcx.is_compiler_builtins(cnum.as_def_id())
-            && !tcx.is_panic_runtime(cnum.as_def_id())
-            && !tcx.has_global_allocator(cnum.as_def_id()) {
+    for &(id, span) in &tcx.maybe_unused_extern_crates {
+        let cnum = tcx.sess.cstore.extern_mod_stmt_cnum(id).unwrap().as_def_id();
+        if !tcx.is_compiler_builtins(cnum)
+            && !tcx.is_panic_runtime(cnum)
+            && !tcx.has_global_allocator(cnum) {
                 let lint = lint::builtin::UNUSED_EXTERN_CRATES;
                 let msg = "unused extern crate";
                 tcx.lint_node(lint, id, span, msg);

src/test/compile-fail/auxiliary/lint_unused_extern_crate5.rs

@@ -0,0 +1,9 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.

src/test/compile-fail/lint-unused-extern-crate.rs

@@ -12,12 +12,15 @@
 // aux-build:lint_unused_extern_crate2.rs
 // aux-build:lint_unused_extern_crate3.rs
 // aux-build:lint_unused_extern_crate4.rs
+// aux-build:lint_unused_extern_crate5.rs
 
 #![deny(unused_extern_crates)]
 #![allow(unused_variables)]
 #![allow(deprecated)]
 
-extern crate lint_unused_extern_crate4; //~ ERROR: unused extern crate
+extern crate lint_unused_extern_crate5; //~ ERROR: unused extern crate
+
+pub extern crate lint_unused_extern_crate4; // no error, it is reexported
 
 extern crate lint_unused_extern_crate3; // no error, it is used