Broken MIR: generator contains type &str in MIR, but typeck only knows about {std::future::ResumeTy, impl std::future::Future, ()}

[tmiasko]

Code

Using -Zmir-opt-level=2:

pub async fn connect() {}
pub fn block_on<F: std::future::Future>(_: F) {}
fn main() {
    block_on(async {
        Vec::<String>::new().first().ok_or("").unwrap();
        connect().await;
    })
}

Meta

rustc --version --verbose:

rustc 1.45.0-nightly (7f65393b9 2020-05-01)
binary: rustc
commit-hash: 7f65393b9abf5e70d0b9a8080558f17c5625bd40
commit-date: 2020-05-01
host: x86_64-unknown-linux-gnu
release: 1.45.0-nightly
LLVM version: 9.0

Error

$ rustc --edition=2018 -Zmir-opt-level=2 connect.rs
error: internal compiler error: src/librustc_mir/transform/generator.rs:722: Broken MIR: generator contains type &str in MIR, but typeck only knows about {std::future::ResumeTy, impl std::future::Future, ()}
 --> connect.rs:4:20
  |
4 |       block_on(async {
  |  ____________________^
5 | |         Vec::<String>::new().first().ok_or("").unwrap();
6 | |         connect().await;
7 | |     })
  | |_____^

Backtrace

thread 'rustc' panicked at 'Box<Any>', /home/tm/test/rust/src/libstd/macros.rs:13:23
stack backtrace:
   0: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
   1: core::fmt::write
   2: std::io::Write::write_fmt
   3: std::sys_common::backtrace::print
   4: std::panicking::default_hook::{{closure}}
   5: std::panicking::default_hook
   6: rustc_driver::report_ice
   7: std::panicking::rust_panic_with_hook
   8: std::panicking::begin_panic
   9: rustc_errors::HandlerInner::span_bug
  10: rustc_errors::Handler::span_bug
  11: rustc_middle::util::bug::opt_span_bug_fmt::{{closure}}
  12: rustc_middle::ty::context::tls::with_opt::{{closure}}
  13: rustc_middle::ty::context::tls::with_opt
  14: rustc_middle::util::bug::opt_span_bug_fmt
  15: rustc_middle::util::bug::span_bug_fmt
  16: <rustc_mir::transform::generator::StateTransform as rustc_mir::transform::MirPass>::run_pass
  17: rustc_mir::transform::run_passes
  18: rustc_mir::transform::run_optimization_passes
  19: rustc_mir::transform::optimized_mir
  20: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::optimized_mir>::compute
  21: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  22: rustc_query_system::query::plumbing::get_query_impl
  23: rustc_middle::ty::<impl rustc_middle::ty::context::TyCtxt>::generator_layout
  24: rustc_middle::ty::layout::LayoutCx<rustc_middle::ty::context::TyCtxt>::layout_raw_uncached
  25: rustc_middle::ty::layout::layout_raw
  26: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::layout_raw>::compute
  27: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  28: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task
  29: rustc_query_system::query::plumbing::get_query_impl
  30: <rustc_middle::ty::layout::LayoutCx<rustc_middle::ty::context::TyCtxt> as rustc_target::abi::LayoutOf>::layout_of
  31: <rustc_mir::transform::inline::Inline as rustc_mir::transform::MirPass>::run_pass
  32: rustc_mir::transform::run_passes
  33: rustc_mir::transform::run_optimization_passes
  34: rustc_mir::transform::optimized_mir
  35: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::optimized_mir>::compute
  36: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  37: rustc_query_system::query::plumbing::get_query_impl
  38: rustc_middle::ty::<impl rustc_middle::ty::context::TyCtxt>::instance_mir
  39: rustc_mir::monomorphize::collector::collect_items_rec
  40: rustc_session::utils::<impl rustc_session::session::Session>::time
  41: rustc_mir::monomorphize::collector::collect_crate_mono_items
  42: rustc_mir::monomorphize::partitioning::collect_and_partition_mono_items
  43: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::collect_and_partition_mono_items>::compute
  44: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  45: rustc_query_system::query::plumbing::get_query_impl
  46: rustc_codegen_ssa::base::codegen_crate
  47: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_ssa::traits::backend::CodegenBackend>::codegen_crate
  48: rustc_session::utils::<impl rustc_session::session::Session>::time
  49: rustc_interface::passes::start_codegen
  50: rustc_middle::ty::context::tls::enter_global
  51: rustc_interface::queries::Query<T>::compute
  52: rustc_interface::queries::Queries::ongoing_codegen
  53: rustc_interface::interface::run_compiler_in_existing_thread_pool
  54: scoped_tls::ScopedKey<T>::set
  55: rustc_ast::attr::with_globals
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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.45.0-dev running on x86_64-unknown-linux-gnu

note: compiler flags: -Z mir-opt-level=2

query stack during panic:
#0 [optimized_mir] processing `main::{{closure}}#0`
#1 [layout_raw] computing layout of `[static generator@connect.rs:4:20: 7:6 {std::future::ResumeTy, std::future::from_generator::GenFuture<[static generator@connect.rs:1:24: 1:26 {}]>, ()}]`
#2 [optimized_mir] processing `main`
#3 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: aborting due to previous error

The issue occurs with nightly, but the backtrace is from locally build 19ae74d0d.

[nikomatsakis]

Could this be a result of inlining? (cc @rust-lang/wg-mir-opt)

Some context:

• type check is assembling an over-approximation of types live at a yield
• I believe that when we do the generator transform, we are checking that the things live across a yield are a subset of those types
• but if we do inlining before-hand, maybe it's possible to alter the set of types in some way? This requires a bit more investigation to be sure, it's not obvious to me at this moment just how that comes about, but I guess it could be true maybe.

[tmandry]

@rustbot claim

I'll do some investigating here, to find out more about what's going on (and probably stop after that).

[wesleywiser]

The MIR inliner certainly seems implicated:

30: <rustc_middle::ty::layout::LayoutCx<rustc_middle::ty::context::TyCtxt> as rustc_target::abi::LayoutOf>::layout_of
31: <rustc_mir::transform::inline::Inline as rustc_mir::transform::MirPass>::run_pass
32: rustc_mir::transform::run_passes
33: rustc_mir::transform::run_optimization_passes
34: rustc_mir::transform::optimized_mir

I seem to recall similar issues with the ConstProp pass and getting the layout of generators. That eventually led us to just ban ConstProp from optimizing generators:

rust/src/librustc_mir/transform/const_prop.rs

Lines 80 to 85 in f8d394e

	// FIXME(welseywiser) const prop doesn't work on generators because of query cycles
	// computing their layout.
	if is_generator {
	trace!("ConstProp skipped for generator {:?}", source.def_id());
	return;
	}

A similar thing might be needed here. It would be great though if we could get the MIR optimizations to work correctly with generators as that might lead to us being able to remove some local variables and shrink the size of the generator.

[RalfJung]

Lol, in #70073 we spent a lot of time making sure that optimizations like inlining do run on StateTransform and now it turns out that's a problem?

Maybe StateTransform should just be moved in the transformation passes to avoid having to add a hack to each pass.

That eventually led us to just ban ConstProp from optimizing generators:

That seems odd, given that this runs after StateTransform -- how can further changes then still be a problem?

[wesleywiser]

IIRC the cycle in ConstProp formed because for generators, their layout uses optimized_mir which invokes ConstProp which wants to know about layout which forms a cycle.

[nikomatsakis]

I'd be ok with banning inlining for now, but I would like to have a precise picture, though, of how things are going wrong.

[tmandry]

Spent some time digging into this today.

• The &str getting included in the generator is of course the "" in the code sample, which is initialized and then dropped before the await.
• Dropping marks the local as being borrowed, which means its storage might be reused by the end of the function.
• We conservatively assume all borrowed locals that aren't StorageDead might be used across an await point. (EDIT: Added)
• The local does not use StorageLive/StorageDead, so we can't use that either.

I've attached the MIR CFG, the local in question is _37 in this MIR.

[nikomatsakis]

@tmandry so we generate an explicit Drop of a &'static str value?

[nikomatsakis]

And what does this have to do with the optimization level btw?

[jonas-schievink]

Perhaps the drop is from a Vec<T> function that drops a T, and inlining makes T = &'static str?

[nikomatsakis]

Ah, yes, sounds likely.

[nikomatsakis]

We should be able to ignore drops of copy types.

[nikomatsakis]

We should also be able to do a peephole optimization to convert them to no-ops.

[tmandry]

Sorry, yes we are generating an explicit drop, and it seems to be the inliner which puts it there when inlining ok_or. (std::option::Option::<&std::string::String>::ok_or::<&str>)

[tmandry]

Triage: Removing A-async-await as this is a more general mir-opt issue. This may be a blocker for MIR inlining.

[Aaron1011]

One solution would be to generate StorageDead statements for locals (right now, it looks like we only do it for temporaries). When we inline ok_or and unwrap, the StateTransform pass will be able to determine that the &str local definitely does not live across a yield.