Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lifetime inference failed on HRTB input #54124

Closed
earthengine opened this issue Sep 11, 2018 · 9 comments
Closed

Lifetime inference failed on HRTB input #54124

earthengine opened this issue Sep 11, 2018 · 9 comments
Assignees
Labels
A-lifetimes Area: Lifetimes / regions A-NLL Area: Non-lexical lifetimes (NLL) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ NLL-sound Working towards the "invalid code does not compile" goal

Comments

@earthengine
Copy link

The following code

use std::marker::PhantomData;

trait Lt<'a> {
    type T;
}
impl<'a> Lt<'a> for () {
    type T = PhantomData<&'a ()>;
}

fn test<'a>() {
    let _:fn(<() as Lt<'_>>::T) = |_:PhantomData<&'a ()>| {};
}

fn main() {
    test();
}

gives

error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` due to conflicting requirements
  --> src/main.rs:11:59
   |
11 |     let _:fn(<() as Lt<'_>>::T) = |_:PhantomData<&'a ()>| {};
   |                                                           ^^
   |
note: first, the lifetime cannot outlive the lifetime 'a as defined on the function body at 10:1...
  --> src/main.rs:10:1
   |
10 | fn test<'a>() {
   | ^^^^^^^^^^^^^
   = note: ...so that the types are compatible:
           expected std::marker::PhantomData<&()>
              found std::marker::PhantomData<&'a ()>
note: but, the lifetime must be valid for the anonymous lifetime #2 defined on the body at 11:35...
  --> src/main.rs:11:35
   |
11 |     let _:fn(<() as Lt<'_>>::T) = |_:PhantomData<&'a ()>| {};
   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: ...so that the types are compatible:
           expected Lt<'_>
              found Lt<'_>

(this is a slightly different version of this stackoverflow question)

Analysis

The error message suggest that the type inference engine consider the upper bound of '_ is the lifetime of the closure. This is incorrect, because this lifetime is in an input argument, so it should consider the lower bound to be 'a instead.

This issue may have relation with #53420, but I am not sure yet.

@earthengine earthengine changed the title Lifetime inference failue when HRTB and normal lifetimes are both possible Lifetime inference failed when HRTB and normal lifetimes are both possible Sep 11, 2018
@earthengine earthengine changed the title Lifetime inference failed when HRTB and normal lifetimes are both possible Lifetime inference failed on HRTB input Sep 11, 2018
@memoryruins memoryruins added the A-lifetimes Area: Lifetimes / regions label Sep 15, 2018
@dtolnay
Copy link
Member

dtolnay commented Oct 1, 2018

And with `#![feature(nll)]` you get an ICE instead:
error: internal compiler error: librustc/traits/codegen/mod.rs:68: Encountered error `OutputTypeParameterMismatch(Binder(<[closure@src/main.rs:13:35: 13:61] as std::ops::FnMut<(<() as Lt<'_>>::T,)>>), Binder(<[closure@src/main.rs:13:35: 13:61] as std::ops::FnMut<(std::marker::PhantomData<&()>,)>>), Sorts(ExpectedFound { expected: std::marker::PhantomData<&()>, found: <() as Lt<'_>>::T }))` selecting `Binder(<[closure@src/main.rs:13:35: 13:61] as std::ops::FnMut<(std::marker::PhantomData<&()>,)>>)` during codegen

thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:599: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:480
   6: std::panicking::begin_panic
   7: rustc_errors::Handler::bug
   8: rustc::util::bug::opt_span_bug_fmt::{{closure}}
   9: rustc::ty::context::tls::with_opt::{{closure}}
  10: rustc::ty::context::tls::with_context_opt
  11: rustc::ty::context::tls::with_opt
  12: rustc::util::bug::opt_span_bug_fmt
  13: rustc::util::bug::bug_fmt
  14: rustc::ty::context::tls::with_related_context
  15: rustc::infer::InferCtxtBuilder::enter
  16: rustc::traits::codegen::codegen_fulfill_obligation
  17: rustc::ty::query::__query_compute::codegen_fulfill_obligation
  18: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::codegen_fulfill_obligation<'tcx>>::compute
  19: rustc::dep_graph::graph::DepGraph::with_task_impl
  20: rustc::ty::context::tls::with_related_context
  21: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  22: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  23: rustc::ty::instance::Instance::resolve
  24: rustc_mir::monomorphize::collector::visit_fn_use
  25: <rustc_mir::monomorphize::collector::MirNeighborCollector<'a, 'tcx> as rustc::mir::visit::Visitor<'tcx>>::visit_terminator_kind
  26: rustc_mir::monomorphize::collector::collect_items_rec
  27: rustc_mir::monomorphize::collector::collect_items_rec
  28: rustc_mir::monomorphize::collector::collect_items_rec
  29: rustc_mir::monomorphize::collector::collect_crate_mono_items::{{closure}}
  30: rustc_mir::monomorphize::collector::collect_crate_mono_items
  31: rustc::util::common::time
  32: rustc_codegen_llvm::base::collect_and_partition_mono_items
  33: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::collect_and_partition_mono_items<'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_codegen_llvm::base::codegen_crate
  39: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
  40: rustc::util::common::time
  41: rustc_driver::driver::phase_4_codegen
  42: rustc_driver::driver::compile_input::{{closure}}
  43: rustc::ty::context::tls::enter_context
  44: <std::thread::local::LocalKey<T>>::with
  45: rustc::ty::context::TyCtxt::create_and_enter
  46: rustc_driver::driver::compile_input
  47: rustc_driver::run_compiler_with_pool
  48: rustc_driver::driver::spawn_thread_pool
  49: rustc_driver::run_compiler
  50: syntax::with_globals
  51: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102
  52: rustc_driver::run
  53: rustc_driver::main
  54: std::rt::lang_start::{{closure}}
  55: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
  56: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102
  57: std::rt::lang_start_internal
             at libstd/panicking.rs:289
             at libstd/panic.rs:392
             at libstd/rt.rs:58
  58: main
  59: __libc_start_main
  60: <unknown>
query stack during panic:
#0 [codegen_fulfill_obligation] checking if `std::ops::FnMut` fulfills its obligations
#1 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: aborting due to previous error

@memoryruins memoryruins added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ A-NLL Area: Non-lexical lifetimes (NLL) labels Oct 1, 2018
@nikomatsakis
Copy link
Contributor

Hmm, tricky.

I believe the compiler's error message is correct. The problem has to do — I believe — with where the compiler is inferring the "higher-ranked binders" to be. The original error arises because fn(<() as Lt<'_>>::T) is interpreted as for<'b> fn(<() as Lt<'b>>::T) -- if we could normalize that, it would be for<'b> fn(PhantomData<&'b ()>). However, the closure in contrast requires a specific lifetime -- 'a, from the surrounding context. So it would not be legal to give it a type that permits the caller to give any old lifetime.

The NLL ICE comes from the fact that we "squash" the region error, since NLL is more general, but then we are not prepared for the subtyping fail that arises later. We should probably just report this in a more structured way.

(This is yet another example where it's clear that we need a more flexible syntax for type annotating closure type signatures.)

Interestingly, if we change the closure to just take '_, we still get ICEs (with or without NLL):

use std::marker::PhantomData;

trait Lt<'a> {
    type T;
}
impl<'a> Lt<'a> for () {
    type T = PhantomData<&'a ()>;
}

fn test<'a>() {
    let _:fn(<() as Lt<'_>>::T) = |_:PhantomData<&'_ ()>| {};
}

fn main() {
    test();
}

@earthengine
Copy link
Author

earthengine commented Oct 2, 2018

Interestingly, if we change the closure to just take '_, we still get ICEs (with or without NLL):

Yes. The original ICE was found first(#53420), and then lead me to this.

I understand your analysis though. I guess, we will need some new HRTB grammar like

for <'b> Trait<'b> where 'b: 'a

for lower generic bound and

for <'b> Trait<'b> where 'a: 'b

for upper generic bound?

@pnkfelix pnkfelix added the NLL-sound Working towards the "invalid code does not compile" goal label Oct 2, 2018
@nikomatsakis nikomatsakis added this to the Edition 2018 RC 2 milestone Oct 2, 2018
@nikomatsakis
Copy link
Contributor

Tagging as RC2 for us to resolve the NLL ICE, at least.

@pnkfelix pnkfelix self-assigned this Oct 2, 2018
@nikomatsakis
Copy link
Contributor

So: right now the NLL type-checker is not prepared for non-borrowck-related errors. I am wondering whether we should how broadly we'll have to fix things to catch all the cases here.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 9, 2018

added @spastorino to the assignee list to help take care of squashing the NLL ICE (i.e. turn it into a proper error)

@nikomatsakis
Copy link
Contributor

Bumping this from the RC2 milestone. Doesn't feel like a blocker. It may however be a duplicate of #55183...

@nikomatsakis
Copy link
Contributor

(Nah, probably not)

@nikomatsakis
Copy link
Contributor

So @spastorino and I were looking at this test and unfortunately I think I misdiagnosed it. The NLL ICE arises not for the reason I thought, but rather this ICE is arising from trans. The problem is that NLL is not reporting an error and thus allowing (incorrect) code to get to trans, where it ICEs. (The ICE is one that has been happening for some time and is an orthogonal issue.)

This code however builds with NLL but not with the old checker. I believe it should be illegal (for the reasons I explained here):

#![feature(nll)]

fn test<'a>() {
    let _: for<'b> fn(&'b ()) = |_:&'a ()| {};
}

fn main() {
    test();
}

Basically, the closure as documented accepts only data with lifetime 'a, but the fn(&()) annotation requires that the closure accept data with any lifetime. I would expect this to be a universe error -- even though #54692 means that NLL doesn't consider the specific user anotations, we should still track if the ---- ah.

Right. So the problem is basically #54692 -- I forgot that when type-checking closures with an expectation (in this case, fn(&())) we assign the closure that type as its "official type", but then we check the user-supplied signature (in this case, fn(&'a ())) against that expectation. This check is where the error arises in non-NLL mode. But NLL is skipping the user-supplied signature and hence never performing that second check. My PR for #54692 will .. hopefully .. fix this.

bors added a commit that referenced this issue Oct 23, 2018
…=MatthewJasper

enforce user annotations in closure signatures

Not *quite* ready yet but I'm opening anyway. Still have to finish running tests locally.

Fixes #54692
Fixes #54124

r? @matthewjasper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions A-NLL Area: Non-lexical lifetimes (NLL) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ NLL-sound Working towards the "invalid code does not compile" goal
Projects
None yet
Development

No branches or pull requests

6 participants