about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAlex Crichton <alex@alexcrichton.com>2017-11-30 07:59:35 -0800
committerAlex Crichton <alex@alexcrichton.com>2017-11-30 08:03:04 -0800
commit4e74eb5ba0bbf60ab70a0680d1e07d7a0f59d383 (patch)
tree914130863c32f37553779a7ba4f03fcb2b3b4aaa
parent4fa202d23b4c8c81b8ac6cf89cf843d35854d437 (diff)
downloadrust-4e74eb5ba0bbf60ab70a0680d1e07d7a0f59d383.tar.gz
rust-4e74eb5ba0bbf60ab70a0680d1e07d7a0f59d383.zip
rustc: Filter out bogus extern crate warnings
Rustdoc has for some time now used the "everybody loops" pass in the compiler to
avoid typechecking and otherwise avoid looking at implementation details.
In #46115 the placement of this pass was pushed back in the compiler to after
macro expansion to ensure that it works with macro-expanded code as well. This
in turn caused the regression in #46271.

The bug here was that the resolver was producing `def_id` instances for
"possibly unused extern crates" which would then later get processed during
typeck to actually issue lint warnings. The problem was that *after* resolution
these `def_id` nodes were actually removed from the AST by the "everybody loops"
pass. This later, when we tried to take a look at `def_id`, caused the compiler
to panic.

The fix applied here is a bit of a heavy hammer which is to just, in this one
case, ignore the `extern crate` lints if the `def_id` looks "bogus" in any way
(basically if it looks like the node was removed after resolution). The real
underlying bug here is probably that the "everybody loops" AST pass is being
stressed to much beyond what it was originally intended to do, but this should
at least fix the ICE for now...

Closes #46271
-rw-r--r--src/librustc_typeck/check_unused.rs19
-rw-r--r--src/test/rustdoc/issue-46271.rs15
2 files changed, 34 insertions, 0 deletions
diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs
index b867a655b4a..f2f1e2938cb 100644
--- a/src/librustc_typeck/check_unused.rs
+++ b/src/librustc_typeck/check_unused.rs
@@ -75,6 +75,25 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
     tcx.hir.krate().visit_all_item_likes(&mut visitor);
 
     for &(def_id, span) in tcx.maybe_unused_extern_crates(LOCAL_CRATE).iter() {
+        // The `def_id` here actually was calculated during resolution (at least
+        // at the time of this writing) and is being shipped to us via a side
+        // channel of the tcx. There may have been extra expansion phases,
+        // however, which ended up removing the `def_id` *after* expansion such
+        // as the `ReplaceBodyWithLoop` pass (which is a bit of a hack, but hey)
+        //
+        // As a result we need to verify that `def_id` is indeed still valid for
+        // our AST and actually present in the HIR map. If it's not there then
+        // there's safely nothing to warn about, and otherwise we carry on with
+        // our execution.
+        //
+        // Note that if we carry through to the `extern_mod_stmt_cnum` query
+        // below it'll cause a panic because `def_id` is actually bogus at this
+        // point in time otherwise.
+        if let Some(id) = tcx.hir.as_local_node_id(def_id) {
+            if tcx.hir.find(id).is_none() {
+                continue
+            }
+        }
         let cnum = tcx.extern_mod_stmt_cnum(def_id).unwrap();
         if tcx.is_compiler_builtins(cnum) {
             continue
diff --git a/src/test/rustdoc/issue-46271.rs b/src/test/rustdoc/issue-46271.rs
new file mode 100644
index 00000000000..cc3be08c568
--- /dev/null
+++ b/src/test/rustdoc/issue-46271.rs
@@ -0,0 +1,15 @@
+// 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.
+
+// hopefully this doesn't cause an ICE
+
+pub fn foo() {
+    extern crate std;
+}