Skip to content

Commit

Permalink
clear obligations-added flag with nested fulfillcx
Browse files Browse the repository at this point in the history
This flag is a debugging measure designed to detect cases where we start
a snapshot, create type variables, register obligations involving those
type variables in the fulfillment cx, and then have to unroll the
snapshot, leaving "dangling type variables" behind.  HOWEVER, in some
cases the flag is wrong. In particular, we sometimes create a
"mini-fulfilment-cx" in which we enroll obligations. As long as this
fulfillment cx is fully drained before we return, this is not a problem,
as there won't be any escaping obligations in the main cx. So we add a
fn to save/restore the flag.
  • Loading branch information
nikomatsakis committed Sep 14, 2016
1 parent c87ba3f commit 6353e30
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 22 deletions.
27 changes: 27 additions & 0 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,33 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
result.map(move |t| InferOk { value: t, obligations: fields.obligations })
}

// Clear the "obligations in snapshot" flag, invoke the closure,
// then restore the flag to its original value. This flag is a
// debugging measure designed to detect cases where we start a
// snapshot, create type variables, register obligations involving
// those type variables in the fulfillment cx, and then have to
// unroll the snapshot, leaving "dangling type variables" behind.
// In such cases, the flag will be set by the fulfillment cx, and
// an assertion will fail when rolling the snapshot back. Very
// useful, much better than grovelling through megabytes of
// RUST_LOG output.
//
// HOWEVER, in some cases the flag is wrong. In particular, we
// sometimes create a "mini-fulfilment-cx" in which we enroll
// obligations. As long as this fulfillment cx is fully drained
// before we return, this is not a problem, as there won't be any
// escaping obligations in the main cx. In those cases, you can
// use this function.
pub fn save_and_restore_obligations_in_snapshot_flag<F, R>(&self, func: F) -> R
where F: FnOnce(&Self) -> R
{
let flag = self.obligations_in_snapshot.get();
self.obligations_in_snapshot.set(false);
let result = func(self);
self.obligations_in_snapshot.set(flag);
result
}

fn start_snapshot(&self) -> CombinedSnapshot {
debug!("start_snapshot()");

Expand Down
46 changes: 24 additions & 22 deletions src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,32 +203,34 @@ fn fulfill_implication<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
// attempt to prove all of the predicates for impl2 given those for impl1
// (which are packed up in penv)

let mut fulfill_cx = FulfillmentContext::new();
for oblig in obligations.into_iter() {
fulfill_cx.register_predicate_obligation(&infcx, oblig);
}
match fulfill_cx.select_all_or_error(infcx) {
Err(errors) => {
// no dice!
debug!("fulfill_implication: for impls on {:?} and {:?}, could not fulfill: {:?} given \
{:?}",
source_trait_ref,
target_trait_ref,
errors,
infcx.parameter_environment.caller_bounds);
Err(())
infcx.save_and_restore_obligations_in_snapshot_flag(|infcx| {
let mut fulfill_cx = FulfillmentContext::new();
for oblig in obligations.into_iter() {
fulfill_cx.register_predicate_obligation(&infcx, oblig);
}
match fulfill_cx.select_all_or_error(infcx) {
Err(errors) => {
// no dice!
debug!("fulfill_implication: for impls on {:?} and {:?}, \
could not fulfill: {:?} given {:?}",
source_trait_ref,
target_trait_ref,
errors,
infcx.parameter_environment.caller_bounds);
Err(())
}

Ok(()) => {
debug!("fulfill_implication: an impl for {:?} specializes {:?}",
source_trait_ref,
target_trait_ref);
Ok(()) => {
debug!("fulfill_implication: an impl for {:?} specializes {:?}",
source_trait_ref,
target_trait_ref);

// Now resolve the *substitution* we built for the target earlier, replacing
// the inference variables inside with whatever we got from fulfillment.
Ok(infcx.resolve_type_vars_if_possible(&target_substs))
// Now resolve the *substitution* we built for the target earlier, replacing
// the inference variables inside with whatever we got from fulfillment.
Ok(infcx.resolve_type_vars_if_possible(&target_substs))
}
}
}
})
}

pub struct SpecializesCache {
Expand Down
21 changes: 21 additions & 0 deletions src/test/compile-fail/issue-36053-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Regression test for #36053. ICE was caused due to obligations
// being added to a special, dedicated fulfillment cx during
// a probe.

use std::iter::once;
fn main() {
once::<&str>("str").fuse().filter(|a: &str| true).count();
//~^ ERROR no method named `count`
//~| ERROR E0281
//~| ERROR E0281
}
32 changes: 32 additions & 0 deletions src/test/run-pass/issue-36053.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Regression test for #36053. ICE was caused due to obligations being
// added to a special, dedicated fulfillment cx during a
// probe. Problem seems to be related to the particular definition of
// `FusedIterator` in std but I was not able to isolate that into an
// external crate.

#![feature(fused)]
use std::iter::FusedIterator;

struct Thing<'a>(&'a str);
impl<'a> Iterator for Thing<'a> {
type Item = &'a str;
fn next(&mut self) -> Option<&'a str> {
None
}
}

impl<'a> FusedIterator for Thing<'a> {}

fn main() {
Thing("test").fuse().filter(|_| true).count();
}

0 comments on commit 6353e30

Please sign in to comment.