ICE when using async with references with named lifetimes as arguments

[jkozlowski]

Status

@cramertj suggested this is because when we lower the async function, we add impl Future<Output=something> + 'a, but 'a is lowered and parented as a child of return_impl_trait_id (the argument to lower_async_fn_ret_ty), whereas it should be parented as something outside of the impl trait context (as that is how such an expression would be parented, if it actually existing in the AST).

Description

I have a bit of code where I'd like to use a reference with a lifetime in an "async" function, which panics the compiler.

I tried this code: https://play.rust-lang.org/?gist=dbca2de61c4a02f0ac4a6cf0023ce7b6&version=nightly&mode=debug&edition=2018

I expected to see this happen: My understanding was that async functions are allowed to take references as parameters. @Nemo157 helped to debug this a little bit and there is a suspicion that named lifetimes are somehow not properly propagated. I stumbled into this because I wanted to have 2 args with references, but if the lifetimes are elided the compiler asks for them to be added. Adding them ICEs the compiler.

Instead, this happened: I got a compiler panic.

Meta

Backtrace:

Compiling playground v0.0.1 (file:///playground)
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `DefId(0/1:10 ~ playground[be9c]::hello_world[0]::{{closure}}[0])`,
 right: `DefId(0/0:3 ~ playground[be9c]::hello_world[0])`', librustc/middle/region.rs:761:9
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:479
   6: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   7: std::panicking::begin_panic_fmt
             at libstd/panicking.rs:345
   8: rustc::middle::region::ScopeTree::free_scope
   9: rustc::infer::lexical_region_resolve::LexicalResolver::expand_node
  10: rustc::infer::lexical_region_resolve::resolve
  11: rustc::infer::InferCtxt::resolve_regions_and_report_errors_inner
  12: rustc_typeck::check::regionck::<impl rustc_typeck::check::FnCtxt<'a, 'gcx, 'tcx>>::regionck_fn
  13: rustc::ty::context::tls::with_related_context
  14: rustc::infer::InferCtxtBuilder::enter
  15: rustc_typeck::check::typeck_tables_of
  16: rustc::ty::query::__query_compute::typeck_tables_of
  17: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::typeck_tables_of<'tcx>>::compute
  18: rustc::dep_graph::graph::DepGraph::with_task_impl
  19: rustc::ty::context::tls::with_related_context
  20: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  21: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  22: rustc::ty::query::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::typeck_tables_of
  23: rustc_typeck::check::typeck_tables_of
  24: rustc::ty::query::__query_compute::typeck_tables_of
  25: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::typeck_tables_of<'tcx>>::compute
  26: rustc::dep_graph::graph::DepGraph::with_task_impl
  27: rustc::ty::context::tls::with_related_context
  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>>::get_query
  30: rustc::ty::query::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::typeck_tables_of
  31: rustc_typeck::collect::type_of
  32: rustc::ty::query::__query_compute::type_of
  33: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::type_of<'tcx>>::compute
  34: rustc::dep_graph::graph::DepGraph::with_task_impl
  35: rustc::ty::context::tls::with_related_context
  36: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  37: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  38: rustc::ty::query::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::type_of
  39: <rustc_typeck::collect::CollectItemTypesVisitor<'a, 'tcx> as rustc::hir::intravisit::Visitor<'tcx>>::visit_expr
  40: rustc::hir::intravisit::walk_expr
  41: rustc::hir::intravisit::Visitor::visit_fn
  42: rustc::hir::intravisit::walk_item
  43: <rustc_typeck::collect::CollectItemTypesVisitor<'a, 'tcx> as rustc::hir::intravisit::Visitor<'tcx>>::visit_item
  44: rustc::hir::Crate::visit_all_item_likes
  45: rustc::util::common::time
  46: rustc_typeck::check_crate
  47: rustc::ty::context::tls::enter_context
  48: <std::thread::local::LocalKey<T>>::with
  49: rustc::ty::context::TyCtxt::create_and_enter
  50: rustc_driver::driver::compile_input
  51: rustc_driver::run_compiler_with_pool
  52: <scoped_tls::ScopedKey<T>>::set
  53: syntax::with_globals
  54: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  55: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:105
  56: rustc_driver::run
  57: rustc_driver::main
  58: std::rt::lang_start::{{closure}}
  59: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
  60: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:105
  61: std::rt::lang_start_internal
             at libstd/panicking.rs:289
             at libstd/panic.rs:392
             at libstd/rt.rs:58
  62: main
  63: __libc_start_main
  64: <unknown>
query stack during panic:
#0 [typeck_tables_of] processing `hello_world`
#1 [typeck_tables_of] processing `hello_world::{{closure}}`
#2 [type_of] processing `hello_world::{{closure}}`
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.30.0-nightly (73c78734b 2018-08-05) running on x86_64-unknown-linux-gnu

note: compiler flags: -C codegen-units=1 -C debuginfo=2 --crate-type bin

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

error: Could not compile `playground`.

To learn more, run the command again with --verbose.

[jkozlowski]

@Nemo157 suggested this might be related

[jkozlowski]

It would appear one can workaround this by using async blocks manually:

pub fn hello_world<'a>(hello: &'a String, world: &'a String) -> impl Future<Output=()> + 'a {
    async move {
        println!("Hello world: {:?} {:?}", hello, world);
    }
}

I still need to test this does what I expect.

[panicbit]

@jkozlowski That pattern doesn't work well if your future returns a Result. In that case immediately invoked async closures help:

pub fn hello_world<'a>(hello: &'a String, world: &'a String) -> impl Future<Output=Result<(), Error>> + 'a {
    (async move || {
        may_fail(hello, world)?;
        Ok(())
    })()
}

Here's a reallife example.

[Nemo157]

@panicbit why do you need the immediately invoked closure there, it works fine with an async block

pub fn hello_world<'a>(hello: &'a String, world: &'a String) -> impl Future<Output=Result<(), Error>> + 'a {
    async move {
        may_fail(hello, world)?;
        Ok(())
    }
}

[panicbit]

@Nemo157 Yeah, just noticed. I think I also had some other issue / ICE, but I don't remember exactly what it was. Gonna have to check it again.

Also, I don't think I knew that async *move* on blocks was a thing.

[jkozlowski]

I didn't quite understand why I needed the move. I am trying to fix the original ICE, I think I got some time today tomorrow, hopefully, will make some progress. But compiler newbie, so might not work out.

[panicbit]

@jkozlowski You need to move to avoid the closure borrowing the references locally. It's the same situation as with this non-async example

@Nemo157 I just removed all immeditaly invoked functions from eval 😉

[xrl]

I am playing around with similar types of borrowing patterns, namely using StreamObj to carry a stream around inside of a struct. I can't seem to use the posted work-arounds:

I am taking this working code and I'm refactoring it. Starts as:

#![feature(async_await,futures_api,await_macro)]
use futures::stream::{ repeat, StreamExt, StreamObj };

struct Response<'a, T> {
    stream: StreamObj<'a, T>
}

async fn do_stuff() {
    let mut s = repeat(10).map(|v| v+1).chunks(100);
    let mut response_a_tron = Response {
        stream: StreamObj::new(&mut s)
    };
    while let Some(item) = await!(response_a_tron.stream.next()) {
        assert_eq!(item.len(), 100)
    };
}

pub fn main() {
    fahrenheit::run(do_stuff());
}

this works, compiles and runs.

Then I try my hand at refactoring to move the stream out of the same scope as the await code pulling out values. Now I can get the same ICE as this ticket:

#![feature(async_await,futures_api,await_macro)]
use futures::stream::{ repeat, StreamExt, StreamObj };

struct Response<'a, T> {
    stream: StreamObj<'a, T>
}

async fn do_stuff<'a>(r: Response<'a, Vec<u8>>) {
    while let Some(item) = await!(r.stream.next()) {
        assert_eq!(item.len(), 100)
    };
}

pub fn main() {
    let mut s = repeat(10).map(|v| v+1).chunks(100);
    let mut r = Response {
        stream: StreamObj::new(&mut s)
    };

    fahrenheit::run(do_stuff(r));
}

ICE:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `DefId(0/1:13 ~ freightlines[e644]::do_stuff[0]::{{closure}}[0])`,
 right: `DefId(0/0:8 ~ freightlines[e644]::do_stuff[0])`', librustc/middle/region.rs:761:9
stack backtrace:
[[[ SNIP ]]]

full backtrace: https://gist.github.com/xrl/4057ad3e0b345807280741f646fdf010

Then I try some of your posted workarounds and they don't do the trick, here's my best try:

#![feature(async_await,futures_api,await_macro)]
use futures::Future;
use futures::stream::{ repeat, StreamExt, StreamObj };

struct Response<'a, T> {
    stream: StreamObj<'a, T>
}

fn do_stuff<'a>(mut r: Response<'a, Vec<u8>>) -> impl Future<Output=()> + 'a {
    async move {
        while let Some(item) = await!(r.stream.next()) {
            assert_eq!(item.len(), 100)
        }
    }
}

pub fn main() {
    let mut s = repeat(10).map(|v| v+1).chunks(100);
    let r = Response {
        stream: StreamObj::new(&mut s)
    };

    fahrenheit::run(do_stuff(r));
}

but this fails to compile due to an ownership issue I can't figure out:

    Checking freightlines v0.1.0 (file:///Users/xlange/code/viasat/freightlines)
error[E0597]: `s` does not live long enough
  --> src/main.rs:20:32
   |
20 |         stream: StreamObj::new(&mut s)
   |                                ^^^^^^ borrowed value does not live long enough
...
24 | }
   | - `s` dropped here while still borrowed
   |
   = note: borrowed value must be valid for the static lifetime...

[Nemo157]

@xrl your final borrow error is because Fahrenheit requires futures run on it to have a 'static lifetime while your StreamObj contains a local stack borrow. You could try using futures::executor::block_on instead of Fahrenheit.

[panicbit]

@xrl Alternatively to what @Nemo157 suggested, you could move the code in main to an async fn:

async fn async_main() {
    let mut s = repeat(10).map(|v| v+1).chunks(100);
    let r = Response {
        stream: StreamObj::new(&mut s)
    };

    await!(do_stuff(r));
}

pub fn main() {
    fahrenheit::run(async_main());
}