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

region_obligations not empty #51649

Closed
bhuztez opened this issue Jun 20, 2018 · 12 comments
Closed

region_obligations not empty #51649

bhuztez opened this issue Jun 20, 2018 · 12 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ NLL-complete Working towards the "valid code works" goal
Milestone

Comments

@bhuztez
Copy link

bhuztez commented Jun 20, 2018

when compiling syn v0.14.2 with rustc 1.28.0-nightly (523097979 2018-06-18) and compiler flags: -Z borrowck=mir -Z polonius, the compiler panicked.

thread 'main' panicked at 'region_obligations not empty: [
    (
        NodeId(
            16596
        ),
        RegionObligation(sub_region='_#1r, sup_type=T)
    )
]', librustc/infer/mod.rs:1057: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:515
   6: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:426
   7: std::panicking::begin_panic_fmt
             at libstd/panicking.rs:413
   8: rustc::infer::InferCtxt::take_and_reset_region_constraints
   9: rustc_mir::borrow_check::nll::type_check::TypeChecker::prove_trait_ref
  10: rustc_mir::borrow_check::nll::type_check::TypeVerifier::sanitize_place
  11: <rustc_mir::borrow_check::nll::type_check::TypeVerifier<'a, 'b, 'gcx, 'tcx> as rustc::mir::visit::Visitor<'tcx>>::visit_rvalue
  12: <rustc_mir::borrow_check::nll::type_check::TypeVerifier<'a, 'b, 'gcx, 'tcx> as rustc::mir::visit::Visitor<'tcx>>::visit_mir
  13: rustc_mir::borrow_check::nll::type_check::type_check_internal
  14: rustc_mir::borrow_check::nll::compute_regions
  15: rustc_mir::borrow_check::do_mir_borrowck
  16: rustc::ty::context::GlobalCtxt::enter_local
  17: rustc_mir::borrow_check::mir_borrowck
  18: rustc::ty::query::__query_compute::mir_borrowck
  19: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::mir_borrowck<'tcx>>::compute
  20: rustc::dep_graph::graph::DepGraph::with_task_impl
  21: rustc::ty::context::tls::with_related_context
  22: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  23: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  24: rustc::ty::query::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::mir_borrowck
  25: rustc_driver::driver::phase_3_run_analysis_passes::{{closure}}::{{closure}}
  26: rustc::util::common::time
  27: rustc::ty::context::tls::enter_context
  28: <std::thread::local::LocalKey<T>>::with
  29: rustc::ty::context::TyCtxt::create_and_enter
  30: rustc_driver::driver::compile_input
  31: rustc_driver::run_compiler_with_pool
  32: <scoped_tls::ScopedKey<T>>::set
  33: syntax::with_globals
  34: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  35: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:105
  36: rustc_driver::run
  37: rustc_driver::main
  38: std::rt::lang_start::{{closure}}
  39: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
  40: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:105
  41: std::rt::lang_start_internal
             at libstd/panicking.rs:289
             at libstd/panic.rs:392
             at libstd/rt.rs:58
  42: main
  43: __libc_start_main
  44: <unknown>
query stack during panic:
#0 [mir_borrowck] processing `<punctuated::Punctuated<T, P> as std::ops::Index<usize>>::index`
end of query stack

error: internal compiler error: unexpected panic
@thelearnerofcode
Copy link

I get a similar error when compiling actix-web with the same arguments:

thread 'main' panicked at 'region_obligations not empty: [
    (
        NodeId(
            3053
        ),
        RegionObligation(sub_region=ReStatic, sup_type=A)
    )
]', librustc/infer/mod.rs:1057: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:515
   6: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:426
   7: std::panicking::begin_panic_fmt
             at libstd/panicking.rs:413
   8: rustc::infer::InferCtxt::take_and_reset_region_constraints
   9: rustc_mir::borrow_check::nll::type_check::TypeChecker::normalize
  10: <rustc_mir::borrow_check::nll::type_check::TypeVerifier<'a, 'b, 'gcx, 'tcx> as rustc::mir::visit::Visitor<'tcx>>::visit_constant
  11: <rustc_mir::borrow_check::nll::type_check::TypeVerifier<'a, 'b, 'gcx, 'tcx> as rustc::mir::visit::Visitor<'tcx>>::visit_mir
  12: rustc_mir::borrow_check::nll::type_check::type_check_internal
  13: rustc_mir::borrow_check::nll::compute_regions
  14: rustc_mir::borrow_check::do_mir_borrowck
  15: rustc::ty::context::GlobalCtxt::enter_local
  16: rustc_mir::borrow_check::mir_borrowck
  17: rustc::ty::query::__query_compute::mir_borrowck
  18: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::mir_borrowck<'tcx>>::compute
  19: rustc::ty::context::tls::with_context
  20: rustc::dep_graph::graph::DepGraph::with_task_impl
  21: rustc::ty::context::tls::with_related_context
  22: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  23: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  24: rustc::ty::query::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::mir_borrowck
  25: rustc::ty::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::par_body_owners
  26: rustc::util::common::time
  27: rustc::ty::context::tls::enter_context
  28: <std::thread::local::LocalKey<T>>::with
  29: rustc::ty::context::TyCtxt::create_and_enter
  30: rustc_driver::driver::compile_input
  31: rustc_driver::run_compiler_with_pool
  32: <scoped_tls::ScopedKey<T>>::set
  33: syntax::with_globals
  34: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  35: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:105
  36: rustc_driver::run
  37: rustc_driver::main
  38: std::rt::lang_start::{{closure}}
  39: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
  40: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:105
  41: std::rt::lang_start_internal
             at libstd/panicking.rs:289
             at libstd/panic.rs:392
             at libstd/rt.rs:58
  42: main
  43: __libc_start_main
  44: <unknown>
query stack during panic:
#0 [mir_borrowck] processing `<context::Drain<A> as actix_inner::ActorFuture>::poll`
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.28.0-nightly (f28c7aef7 2018-06-19) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z borrowck=mir -Z polonius -C debuginfo=2 -C incremental --crate-type lib

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

error: Could not compile `actix-web`.

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

@KodrAus
Copy link
Contributor

KodrAus commented Jun 20, 2018

I can repro this with rustc 1.28.0-nightly (f28c7aef7 2018-06-19) with the following:

#![feature(nll)]

use std::ops::Deref;

struct A<T>(T);

impl<T> Deref for A<T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        unimplemented!()
    }
}

cargo build results in:

thread 'main' panicked at 'region_obligations not empty: [
    (
        NodeId(
            22
        ),
        RegionObligation(sub_region='_#1r, sup_type=T)
    )
]', librustc/infer/mod.rs:1057:9
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: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::continue_panic_fmt
   7: std::panicking::begin_panic_fmt
   8: rustc::infer::InferCtxt::take_and_reset_region_constraints
   9: rustc_mir::borrow_check::nll::type_check::TypeChecker::sub_types
  10: rustc_mir::borrow_check::nll::type_check::TypeChecker::typeck_mir
  11: rustc_mir::borrow_check::nll::type_check::type_check_internal
  12: rustc_mir::borrow_check::nll::compute_regions
  13: rustc_mir::borrow_check::do_mir_borrowck
  14: rustc::ty::context::GlobalCtxt::enter_local
  15: rustc_mir::borrow_check::mir_borrowck
  16: rustc::ty::query::__query_compute::mir_borrowck
  17: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::mir_borrowck<'tcx>>::compute
  18: rustc::ty::context::tls::with_context
  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::query::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::mir_borrowck
  24: rustc::ty::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::par_body_owners
  25: rustc::util::common::time
  26: rustc::ty::context::tls::enter_context
  27: <std::thread::local::LocalKey<T>>::with
  28: rustc::ty::context::TyCtxt::create_and_enter
  29: rustc_driver::driver::compile_input
  30: rustc_driver::run_compiler_with_pool
  31: <scoped_tls::ScopedKey<T>>::set
  32: syntax::with_globals
  33: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  34: __rust_maybe_catch_panic
  35: rustc_driver::run
  36: rustc_driver::main
  37: std::rt::lang_start::{{closure}}
  38: std::panicking::try::do_call
  39: __rust_maybe_catch_panic
  40: std::rt::lang_start_internal
  41: main
query stack during panic:
#0 [mir_borrowck] processing `<A<T> as std::ops::Deref>::deref`
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.28.0-nightly (f28c7aef7 2018-06-19) running on x86_64-apple-darwin

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

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

@KodrAus KodrAus added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Jun 21, 2018
@ishitatsuyuki ishitatsuyuki added the A-NLL Area: Non-lexical lifetimes (NLL) label Jun 22, 2018
@jice-nospam
Copy link

Reproduced on 1.28.0-nightly (2a1c4ee 2018-06-25) running on x86_64-unknown-linux-gnu.
compiler flags : -C debuginfo=2 -C incremental --crate-type lib

thread 'main' panicked at 'region_obligations not empty: [
    (
        NodeId(
            2868
        ),
        RegionObligation(sub_region='_#1r, sup_type=engine::asset::asset_database::AssetDatabaseCOntext<FS>)
    )
]', librustc/infer/mod.rs:1057:9

@jonhoo
Copy link
Contributor

jonhoo commented Jun 27, 2018

I don't think unimplemented!() is even necessary:

#![feature(nll)]
use std::ops::Deref;
pub struct Foo<T>(T);
impl<T> Deref for Foo<T> {
    type Target = T;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

@Mark-Simulacrum Mark-Simulacrum added the NLL-complete Working towards the "valid code works" goal label Jun 27, 2018
@nikomatsakis
Copy link
Contributor

I think I know the problem, but my old minimizations were no longer reproducing. Thanks all for the test cases. I'll try to get this fixed ASAP.

@nikomatsakis nikomatsakis self-assigned this Jun 28, 2018
@nikomatsakis
Copy link
Contributor

I can't reproduce the ICE on master (but I can with nightly) — however, I suspect there is still a bug here. Gonna dig a bit deeper.

@jonhoo
Copy link
Contributor

jonhoo commented Jun 28, 2018

Latest master fixes both the minimization and the full codebase issue for me 👍

@nikomatsakis
Copy link
Contributor

OK so let me leave some notes on what's going wrong. As I thought, the ICE is gone, but the underlying problem remains. Explaining the problem, I fear, is a bit complex.

First: in some parts, the NLL type checker uses a kind of gruesome hack to extract its constraints. Essentially, it is building on the old inference context, which accumulates various region constraints in a format designed for the old lexical solver. NLL hijacks this by running the type-check, generating those old-style constraints, and then "taking" them and converting them to the new format. We do this "take" operation frequently so that we can isolate where in the program the constraint arose. So we do something like this to process some point P:

process(infcx, P); // processes P, adding constraints to infcx
let constraints = infcx.take_constraints(); // takes all registered constriants
nll_context.record_constraints(constriants, P); // record them in the new format

(Since my latest PR, we are doing this a lot less, and instead we are using queries to do this isolation; this is much better. But anyway.)

There is however an implicit assumption that the constriants we get from take_constraints all arose as part of process(infcx, P) -- but what if there were constriants recorded before we started? Constraints that never got "taken"? This is precisely the problem we are seeing -- before, it was resulting in an ICE, but now we are not checking.

If you add this diff, though, you will start to see ICEs again:

modified   src/librustc/traits/query/type_op/custom.rs
@@ -75,6 +75,14 @@ fn scrape_region_constraints<'gcx, 'tcx, R>(
 ) -> Fallible<(R, Option<Rc<Vec<QueryRegionConstraint<'tcx>>>>)> {
     let mut fulfill_cx = TraitEngine::new(infcx.tcx);
     let dummy_body_id = ObligationCause::dummy().body_id;
+
+    let pre_obligations = infcx.take_registered_region_obligations();
+    assert!(
+        pre_obligations.is_empty(),
+        "scrape_region_constraints: incoming region obligations = {:#?}",
+        pre_obligations,
+    );
+
     let InferOk { value, obligations } = infcx.commit_if_ok(|_| op())?;
     debug_assert!(obligations.iter().all(|o| o.cause.body_id == dummy_body_id));
     fulfill_cx.register_predicate_obligations(infcx, obligations);

@nikomatsakis
Copy link
Contributor

So what code is causing these wayward region obligations? The answer is that a very early part of the NLL process is causing the problem. Specifically, when we are creating the universal regions table:

// Compute named region information. This also renumbers the inputs/outputs.
let universal_regions = UniversalRegions::new(infcx, def_id, param_env);

During that process, we compute the implied bounds from the argument types:

let bounds = self.infcx
.implied_outlives_bounds(self.param_env, self.mir_node_id, ty, span);

That code winds up (in some weird cases involving associated types) having to solve some obligations:

// Compute the obligations for `ty` to be well-formed. If `ty` is
// an unresolved inference variable, just substituted an empty set
// -- because the return type here is going to be things we *add*
// to the environment, it's always ok for this set to be smaller
// than the ultimate set. (Note: normally there won't be
// unresolved inference variables here anyway, but there might be
// during typeck under some circumstances.)
let obligations = wf::obligations(self, param_env, body_id, ty, span).unwrap_or(vec![]);
// NB: All of these predicates *ought* to be easily proven
// true. In fact, their correctness is (mostly) implied by
// other parts of the program. However, in #42552, we had
// an annoying scenario where:
//
// - Some `T::Foo` gets normalized, resulting in a
// variable `_1` and a `T: Trait<Foo=_1>` constraint
// (not sure why it couldn't immediately get
// solved). This result of `_1` got cached.
// - These obligations were dropped on the floor here,
// rather than being registered.
// - Then later we would get a request to normalize
// `T::Foo` which would result in `_1` being used from
// the cache, but hence without the `T: Trait<Foo=_1>`
// constraint. As a result, `_1` never gets resolved,
// and we get an ICE (in dropck).
//
// Therefore, we register any predicates involving
// inference variables. We restrict ourselves to those
// involving inference variables both for efficiency and
// to avoids duplicate errors that otherwise show up.
fulfill_cx.register_predicate_obligations(
self,
obligations
.iter()
.filter(|o| o.predicate.has_infer_types())
.cloned(),
);

Solving those obligations may produce the region-obligations that are causing us trouble. But if you look carefully at that code, you'll notice a weird thing: these obligations that we are solving are basically the requirements that are needed for types to be well-formed. But we should already be guaranteeing that the types are WF (I can't, anyway, think of an exception off the top of my head... my comment suggests that there may have been one...)

I think my preference here would be to make a (canonicalized) query for computing the implied region bounds from a ty. This query would be invoked from librustc_typeck and also, later, from NLL MIR. It would also resolve all of these nested obligations as needed. The expectation here would be that there are no errors that result — typically we can't do a good job reporting errors from queries.

That said, we should first check whether this is "dead code" here (that is, the error reporting code):

Err(errors) => self.report_fulfillment_errors(&errors, None, false),

@nikomatsakis
Copy link
Contributor

So this ICE doesn't occur on master, therefore I'm going to defer it, but the code is still kinda wrong. I may try to fix it in "spare time". I haven't yet finishd my essay series on the best fix yet, so I could start there. :)

@nikomatsakis
Copy link
Contributor

I'd like to see this fixed by the RC though.

@tmandry
Copy link
Member

tmandry commented Jul 1, 2018

we should first check whether this is "dead code" here (that is, the error reporting code)

I checked this; it only occurs if there are other (already reported) errors in the program. I was only able to trigger this in a single test. Without any error handling, it has the same behavior.

compile-fail/impl-trait/where-allowed.rs

bors added a commit that referenced this issue Jul 21, 2018
…sakis

Turn implied_outlives_bounds into a query

Right now all this does is remove the error reporting in `implied_outlives_bounds`, which seems to work. Farming out full tests to Travis.

For #51649. That issue is deferred so not sure what's next.

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

No branches or pull requests

10 participants