rustc panics on compile-fail/issue-36638

[cuviper]

As noted in #49826 (comment), currently rustc is panicking on compile-fail/issue-36638. This only passed CI because main-thread panics are not being detected as ICE (to be fixed by #49826), and compiletest doesn't notice non-ICE panics (#49888).

It's fine with rustc 1.27.0-nightly (0b72d48f8 2018-04-10), so I was able to bisect this to the recent commit abfc8c2 from #49695 -- cc @michaelwoerister.

$ RUST_BACKTRACE=1 rustc +dev1 -Z continue-parse-after-error src/test/compile-fail/issue-36638.rs
error: expected identifier, found keyword `Self`
  --> src/test/compile-fail/issue-36638.rs:13:12
   |
13 | struct Foo<Self>(Self);
   |            ^^^^ expected identifier, found keyword

error: expected identifier, found keyword `Self`
  --> src/test/compile-fail/issue-36638.rs:16:11
   |
16 | trait Bar<Self> {}
   |           ^^^^ expected identifier, found keyword

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`', librustc/ty/sty.rs:889:13
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: core::ops::function::Fn::call
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic_fmt
   7: rustc::ty::sty::ParamTy::is_self
   8: rustc::ty::flags::FlagComputation::for_sty
   9: rustc::ty::context::CtxtInterners::intern_ty
  10: rustc::ty::subst::<impl rustc::ty::Slice<rustc::ty::subst::Kind<'tcx>>>::fill_item
  11: rustc::ty::subst::<impl rustc::ty::Slice<rustc::ty::subst::Kind<'tcx>>>::identity_for_item
  12: rustc_typeck::collect::predicates_of
  13: rustc::ty::maps::<impl rustc::ty::maps::queries::predicates_of<'tcx>>::compute_result
  14: rustc::dep_graph::graph::DepGraph::with_task_impl
  15: rustc::ty::context::tls::with_related_context
  16: rustc::ty::maps::<impl rustc::ty::maps::queries::predicates_of<'tcx>>::force_with_lock
  17: rustc::ty::maps::<impl rustc::ty::maps::queries::predicates_of<'tcx>>::try_get
  18: rustc::ty::maps::TyCtxtAt::predicates_of
  19: rustc::ty::maps::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::predicates_of
  20: <rustc_typeck::collect::CollectItemTypesVisitor<'a, 'tcx> as rustc::hir::intravisit::Visitor<'tcx>>::visit_item
  21: rustc::hir::Crate::visit_all_item_likes
  22: rustc::util::common::time
  23: rustc_typeck::check_crate
  24: rustc::ty::context::tls::enter_context
  25: <std::thread::local::LocalKey<T>>::with
  26: rustc::ty::context::TyCtxt::create_and_enter
  27: rustc_driver::driver::compile_input
  28: rustc_driver::run_compiler_impl
  29: syntax::with_globals
  30: rustc_driver::run
  31: rustc_driver::main
  32: std::rt::lang_start::{{closure}}
  33: std::panicking::try::do_call
  34: __rust_maybe_catch_panic
  35: std::rt::lang_start_internal
  36: main
  37: __libc_start_main
  38: _start
error: aborting due to 2 previous errors

Note that the test is dealing with improper usage of Self, and the assertion in question is also dealing with self:

rust/src/librustc/ty/sty.rs

Lines 887 to 894 in abfc8c2

	pub fn is_self(&self) -> bool {
	if self.name == keywords::SelfType.name().as_str() {
	assert_eq!(self.idx, 0);
	true
	} else {
	false
	}
	}

[michaelwoerister]

So, #36649 would gensym the name "Self" but #49695 erases that difference again by doing a string contents comparison. However, the test only gets to the failing assertion because of -Z continue-parse-after-error. I'm not sure how useful this test is? Is -Z continue-parse-after-error used in production somewhere?

cc @rust-lang/compiler
cc #49300

[eddyb]

I don't think we should need is_self and has_self_ty (or the gensym hack altogether), and instead we should rely on knowing that e.g. the item in scope is a trait and thus index 0 is Self.

[cuviper]

As a stop-gap, what if we make idx == 0 part of the condition of is_self?

diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs
index 310fcbcfcb37..c34514178d65 100644
--- a/src/librustc/ty/sty.rs
+++ b/src/librustc/ty/sty.rs
@@ -885,8 +885,7 @@ impl<'a, 'gcx, 'tcx> ParamTy {
     }

     pub fn is_self(&self) -> bool {
-        if self.name == keywords::SelfType.name().as_str() {
-            assert_eq!(self.idx, 0);
+        if self.name == keywords::SelfType.name().as_str() && self.idx == 0 {
             true
         } else {
             false

[eddyb]

@cuviper That seems worse, because now instead of an ICE, the compiler might be doing weird things. Then again, this can only arise if there was a parse error, so I think it's an okay stopgap iff it's tagged with a FIXME comment referring to the number of a fresh issue for a better fix.

My suggestion for a better fix remains #49889 (comment) - and the new issue could mention it.

[cuviper]

OK, I filed #50125. I'll add the stopgap with a FIXME to #49891 so we can land that.

[cuviper]

The stopgap has been merged -- closing in favor of further cleanup in #50125.