From 6353e30bb2d7968332217c04ac25e0800500fb33 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 13 Sep 2016 18:31:26 -0400 Subject: [PATCH] clear obligations-added flag with nested fulfillcx 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. --- src/librustc/infer/mod.rs | 27 +++++++++++++++ src/librustc/traits/specialize/mod.rs | 46 ++++++++++++++------------ src/test/compile-fail/issue-36053-2.rs | 21 ++++++++++++ src/test/run-pass/issue-36053.rs | 32 ++++++++++++++++++ 4 files changed, 104 insertions(+), 22 deletions(-) create mode 100644 src/test/compile-fail/issue-36053-2.rs create mode 100644 src/test/run-pass/issue-36053.rs diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index 59431f3f02dce..39fc50666a8ce 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -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(&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()"); diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index f3ba4d16eb0b2..2f63526bf6c27 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -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 { diff --git a/src/test/compile-fail/issue-36053-2.rs b/src/test/compile-fail/issue-36053-2.rs new file mode 100644 index 0000000000000..7da529487aa86 --- /dev/null +++ b/src/test/compile-fail/issue-36053-2.rs @@ -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 or the MIT license +// , 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 +} diff --git a/src/test/run-pass/issue-36053.rs b/src/test/run-pass/issue-36053.rs new file mode 100644 index 0000000000000..2411996cf054b --- /dev/null +++ b/src/test/run-pass/issue-36053.rs @@ -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 or the MIT license +// , 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(); +}