ICE `!ty.needs_infer() && !ty.has_placeholders()` from boxing closure of type dyn for<'a> _

[nick-fischer]

While experimenting with clonable closures (and encountering like a thousand one type is more general than the other errors), I eventually discovered a scenario which crashes rustc.

I tried this code (play:

trait ClonableFn<T> {
	fn clone(&self) -> Box<dyn Fn(T)>;
}

impl<T, F: 'static> ClonableFn<T> for F
where F: Fn(T) + Clone {
	fn clone(&self) -> Box<dyn Fn(T)> {
		Box::new(self.clone())
	}
}

struct Foo(Box<dyn for<'a> ClonableFn<&'a bool>>);

fn main() {
	Foo(Box::new(|_| ()));
}

I expected to see this happen: Actually, I am not sure ... Anyways, I guess I expected to see rustc surviving the input ...

Meta

rustc --version --verbose:
rustc 1.33.0-nightly (c0bbc39 2019-01-03)
binary: rustc
commit-hash: c0bbc39
commit-date: 2019-01-03
host: x86_64-unknown-linux-gnu
release: 1.33.0-nightly
LLVM version: 8.0

Backtrace:

thread 'rustc' panicked at 'assertion failed: !ty.needs_infer() && !ty.has_placeholders()', src/librustc_typeck/check/writeback.rs:109:9
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
   1: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:70
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:58
             at src/libstd/panicking.rs:200
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:215
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:482
   6: std::panicking::begin_panic
   7: rustc_typeck::check::writeback::WritebackCx::visit_node_id
   8: <rustc_typeck::check::writeback::WritebackCx<'cx, 'gcx, 'tcx> as rustc::hir::intravisit::Visitor<'gcx>>::visit_expr
   9: rustc::hir::intravisit::walk_expr
  10: <rustc_typeck::check::writeback::WritebackCx<'cx, 'gcx, 'tcx> as rustc::hir::intravisit::Visitor<'gcx>>::visit_expr
  11: rustc::hir::intravisit::walk_block
  12: <rustc_typeck::check::writeback::WritebackCx<'cx, 'gcx, 'tcx> as rustc::hir::intravisit::Visitor<'gcx>>::visit_expr
  13: rustc_typeck::check::writeback::<impl rustc_typeck::check::FnCtxt<'a, 'gcx, 'tcx>>::resolve_type_vars_in_body
  14: rustc::ty::context::GlobalCtxt::enter_local
  15: rustc_typeck::check::typeck_tables_of
  16: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::typeck_tables_of<'tcx>>::compute
  17: rustc::dep_graph::graph::DepGraph::with_task_impl
  18: <rustc::ty::query::plumbing::JobOwner<'a, 'tcx, Q>>::start
  19: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  20: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::try_get_with
  21: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::ensure_query
  22: rustc::session::Session::track_errors
  23: rustc_typeck::check::typeck_item_bodies
  24: rustc::ty::query::__query_compute::typeck_item_bodies
  25: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::typeck_item_bodies<'tcx>>::compute
  26: rustc::dep_graph::graph::DepGraph::with_task_impl
  27: <rustc::ty::query::plumbing::JobOwner<'a, 'tcx, Q>>::start
  28: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  29: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::try_get_with
  30: rustc::util::common::time
  31: rustc_typeck::check_crate
  32: <std::thread::local::LocalKey<T>>::with
  33: rustc::ty::context::TyCtxt::create_and_enter
  34: rustc_driver::driver::compile_input
  35: rustc_driver::run_compiler_with_pool
  36: <scoped_tls::ScopedKey<T>>::set
  37: rustc_driver::run_compiler
  38: <scoped_tls::ScopedKey<T>>::set
  39: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  40: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:92
  41: <F as alloc::boxed::FnBox<A>>::call_box
  42: std::sys::unix::thread::Thread::new::thread_start
             at /rustc/c0bbc3927e28c22edefe6a1353b5ecc95ea9a104/src/liballoc/boxed.rs:734
             at src/libstd/sys_common/thread.rs:14
             at src/libstd/sys/unix/thread.rs:81
  43: start_thread
  44: clone
query stack during panic:
#0 [typeck_tables_of] processing `main`
#1 [typeck_item_bodies] type-checking all item bodies
end of query stack

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.33.0-nightly (c0bbc3927 2019-01-03) running on x86_64-unknown-linux-gnu

note: compiler flags: -C debuginfo=2 -C incremental --crate-type bin

note: some of the compiler flags provided by cargo are hidden

error: Could not compile `hello`.

Caused by:
  process didn't exit successfully: `rustc --crate-name hello src/main.rs --color always --crate-type bin --emit=dep-info,metadata -C debuginfo=2 -C metadata=9735c65936601b39 -C extra-filename=-9735c65936601b39 --out-dir /home/nick/Documents/rust/hello/target/debug/deps -C incremental=/home/nick/Documents/rust/hello/target/debug/incremental -L dependency=/home/nick/Documents/rust/hello/target/debug/deps` (exit code: 101)

[hellow554]

Happens on 1.33.0-beta.3 (!) and 1.33.0-nightly (4c2be9c 2019-01-22), but not on stable.

The ICE does not occur, if you change the line to struct Foo(Box<dyn ClonableFn<bool>>);

Was introduced with nightly-2019-01-04, so somewhere between ec19464...c0bbc39

[hellow554]

Based on the commits I guess @nikomatsakis should be able to say something about it

[pnkfelix]

triage. Confirm P-high. Assigning to self for initial investigation.

[pnkfelix]

I have confirmed that this was injected by #55517

cc @nikomatsakis @scalexm

[pnkfelix]

(We've been asserting !ty.needs_infer() or some variant thereof in the writeback method for years. I'm yet not sure what changed in #55517 to disrupt that invariant; its possible that something is triggering the assertion that previously did not. Or we might be writing-back a type that in the past we did not write-back...)

Also, something potentially of note: #50197 changed the definition of needs_infer, but was careful to preserve the meaning of the aforementioned assertion by expanding it to !ty.needs_infer() && !ty.has_placeholders() (or, at that time, !ty.has_skol()). This seems like a good (conservative) decision, but it may have interacted in some manner with the changes in #55517.

• In particular, the type in question that is triggering the assertion has has_placeholders(): true.

[pnkfelix]

@nick-fischer I don't know if this is of interest to you, but I did find that if you use this slightly different variant with an explicit type on the closure parameter, the code compiles (play):

trait ClonableFn<T> {
	fn clone(&self) -> Box<dyn Fn(T)>;
}

impl<T, F: 'static> ClonableFn<T> for F
where F: Fn(T) + Clone {
	fn clone(&self) -> Box<dyn Fn(T)> {
		Box::new(self.clone())
	}
}

struct Foo(Box<dyn for<'a> ClonableFn<&'a bool>>);

fn main() {
	Foo(Box::new(|_: &bool| ()));
	//             ~~~~~~~ this is what changed
}

[nick-fischer]

Interesting ... Thank you!