-Z save-analysis ICE when processing impl under an fn item

[arturoc]

While compiling rayon:

{"message":"src/librustc/ty/context.rs:211: node type <B>::Item (hir_id=HirId { owner: DefIndex(366), local_id: 15 }) with HirId::owner DefId(0:366 ~ rayon[97c5]::iter[0]::chain[0]::{{impl}}[1]::with_producer[0]::{{impl}}[0]) cannot be placed in TypeckTables with local_id_root DefId(0:358 ~ rayon[97c5]::iter[0]::chain[0]::{{impl}}[1]::with_producer[0])","code":null,"level":"error: internal compiler error","spans":[],"children":[],"rendered":"error: internal compiler error: src/librustc/ty/context.rs:211: node type <B>::Item (hir_id=HirId { owner: DefIndex(366), local_id: 15 }) with HirId::owner DefId(0:366 ~ rayon[97c5]::iter[0]::chain[0]::{{impl}}[1]::with_producer[0]::{{impl}}[0]) cannot be placed in TypeckTables with local_id_root DefId(0:358 ~ rayon[97c5]::iter[0]::chain[0]::{{impl}}[1]::with_producer[0])\n\n"}
thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:917:9
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.37/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.37/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:77
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:61
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1028
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1412
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:65
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:50
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:189
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:206
  10: rustc_driver::report_ice
  11: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:473
  12: std::panicking::begin_panic
  13: rustc_errors::HandlerInner::bug
  14: rustc_errors::Handler::bug
  15: rustc::util::bug::opt_span_bug_fmt::{{closure}}
  16: rustc::ty::context::tls::with_opt::{{closure}}
  17: rustc::ty::context::tls::with_context_opt
  18: rustc::ty::context::tls::with_opt
  19: rustc::util::bug::opt_span_bug_fmt
  20: rustc::util::bug::bug_fmt
  21: rustc::ty::context::validate_hir_id_for_typeck_tables::{{closure}}
  22: rustc::ty::context::tls::with::{{closure}}
  23: rustc::ty::context::tls::with_context::{{closure}}
  24: rustc::ty::context::tls::with_context_opt
  25: rustc::ty::context::tls::with_context
  26: rustc::ty::context::tls::with
  27: rustc::ty::context::TypeckTables::qpath_res
  28: rustc_save_analysis::SaveContext::get_path_res
  29: <rustc_save_analysis::dump_visitor::DumpVisitor as syntax::visit::Visitor>::visit_ty
  30: rustc_save_analysis::dump_visitor::DumpVisitor::process_path
  31: <rustc_save_analysis::dump_visitor::DumpVisitor as syntax::visit::Visitor>::visit_item
  32: rustc_save_analysis::dump_visitor::DumpVisitor::process_method::{{closure}}
  33: rustc_save_analysis::dump_visitor::DumpVisitor::process_method
  34: <rustc_save_analysis::dump_visitor::DumpVisitor as syntax::visit::Visitor>::visit_item
  35: <rustc_save_analysis::dump_visitor::DumpVisitor as syntax::visit::Visitor>::visit_item
  36: <rustc_save_analysis::dump_visitor::DumpVisitor as syntax::visit::Visitor>::visit_item
  37: <rustc_save_analysis::dump_visitor::DumpVisitor as syntax::visit::Visitor>::visit_mod
  38: rustc::dep_graph::graph::DepGraph::with_ignore
  39: rustc_driver::run_compiler::{{closure}}::{{closure}}::{{closure}}
  40: rustc_interface::passes::BoxedGlobalCtxt::access::{{closure}}
  41: rustc_interface::passes::create_global_ctxt::{{closure}}
  42: rustc_interface::interface::run_compiler_in_existing_thread_pool
  43: std::thread::local::LocalKey<T>::with
  44: scoped_tls::ScopedKey<T>::set
  45: syntax::with_globals

[Centril]

cc @Xanewok

[pnkfelix]

triage: P-high, assigning to @Xanewok . Removing nomination label.

[pnkfelix]

The fact that this bug was unknown until now strikes me as a sign that we have never attempted to run -Z save-analysis on the rustc test suite (i.e. compiletest).

is this true? Would it be worthwhile to attempt that experiment?

[Xanewok]

I'm not sure if this is the correct approach, but I just executed the ./x.py test with the following patch applied:

diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
index ea31f37c7a5..66546c5ca0c 100644
--- a/src/tools/compiletest/src/runtest.rs
+++ b/src/tools/compiletest/src/runtest.rs
@@ -2869,6 +2869,7 @@ impl<'test> TestCx<'test> {
         // We don't want RUSTFLAGS set from the outside to interfere with
         // compiler flags set in the test cases:
         cmd.env_remove("RUSTFLAGS");
+        cmd.env("RUSTFLAGS", "-Zsave-analysis");
 
         // Use dynamic musl for tests because static doesn't allow creating dylibs
         if self.config.host.contains("musl") {

and the test suite did crash/ended up all green.

It might be a good idea to always run the tests along with save-analysis passed. Should that be optionally enabled? Should we run it, e.g. in the -tools container?