Skip to content

Commit

Permalink
Prevent coercions from polluting the fulfillment context
Browse files Browse the repository at this point in the history
This adds transaction support to fulfill. I can't use it in rust-lang#26282
because that would require nested transactions.

This doesn't seem to cause compilation time to regress.

Fixes rust-lang#24819.
  • Loading branch information
Ariel Ben-Yehuda committed Jun 15, 2015
1 parent 0d82fb5 commit 2cc7ad0
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 1 deletion.
61 changes: 61 additions & 0 deletions src/librustc/middle/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use middle::infer::InferCtxt;
use middle::ty::{self, RegionEscape, Ty};
use std::collections::HashSet;
use std::default::Default;
use std::mem;
use syntax::ast;
use util::common::ErrorReported;
use util::ppaux::Repr;
Expand Down Expand Up @@ -80,6 +81,11 @@ pub struct FulfillmentContext<'tcx> {
// obligations (otherwise, it's easy to fail to walk to a
// particular node-id).
region_obligations: NodeMap<Vec<RegionObligation<'tcx>>>,

// Stored versions of the fields for snapshots
stored_region_obligations: Option<NodeMap<Vec<RegionObligation<'tcx>>>>,
stored_predicates: Option<Vec<PredicateObligation<'tcx>>>,
stored_duplicate_set: Option<HashSet<ty::Predicate<'tcx>>>
}

#[derive(Clone)]
Expand All @@ -96,6 +102,51 @@ impl<'tcx> FulfillmentContext<'tcx> {
predicates: Vec::new(),
attempted_mark: 0,
region_obligations: NodeMap(),

stored_region_obligations: None,
stored_predicates: None,
stored_duplicate_set: None
}
}

/// Begin a snapshot. This should be done in parallel with infcx
/// snapshots.
pub fn begin_transaction(&mut self) {
assert!(self.stored_duplicate_set.is_none(), "nested transactions are not supported");
debug!("begin_transaction");

self.stored_duplicate_set = Some(mem::replace(&mut self.duplicate_set,
HashSet::new()));
self.stored_predicates = Some(self.predicates.clone());
self.stored_region_obligations = Some(mem::replace(&mut self.region_obligations,
NodeMap()));
}

/// Rolls the current transaction back
pub fn rollback(&mut self) {
assert!(self.stored_duplicate_set.is_some(), "rollback not within a transaction");
debug!("rollback!");

self.duplicate_set = self.stored_duplicate_set.take().unwrap();
self.predicates = self.stored_predicates.take().unwrap();
self.region_obligations = self.stored_region_obligations.take().unwrap();
}

/// Commits the current transaction
pub fn commit(&mut self) {
assert!(self.stored_duplicate_set.is_some(), "commit not within a transaction");
debug!("commit!");

let transaction_duplicate_set = mem::replace(&mut self.duplicate_set,
self.stored_duplicate_set.take().unwrap());
let transaction_region_obligations = mem::replace(&mut self.region_obligations,
self.stored_region_obligations.take().unwrap());

self.duplicate_set.extend(transaction_duplicate_set);
self.predicates = self.stored_predicates.take().unwrap();

for (node, mut ros) in transaction_region_obligations {
self.region_obligations.entry(node).or_insert(vec![]).append(&mut ros);
}
}

Expand Down Expand Up @@ -170,6 +221,14 @@ impl<'tcx> FulfillmentContext<'tcx> {
return;
}

if let Some(ref duplicate_set) = self.stored_duplicate_set {
if duplicate_set.contains(&obligation.predicate) {
debug!("register_predicate({}) -- already seen before transaction, skip",
obligation.repr(infcx.tcx));
return;
}
}

debug!("register_predicate({})", obligation.repr(infcx.tcx));
self.predicates.push(obligation);
}
Expand All @@ -178,6 +237,8 @@ impl<'tcx> FulfillmentContext<'tcx> {
body_id: ast::NodeId)
-> &[RegionObligation<'tcx>]
{
assert!(self.stored_region_obligations.is_none(),
"can't get region obligations within a transaction");
match self.region_obligations.get(&body_id) {
None => Default::default(),
Some(vec) => vec,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ pub fn mk_assignty<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
debug!("mk_assignty({} -> {})", a.repr(fcx.tcx()), b.repr(fcx.tcx()));
let mut unsizing_obligations = vec![];
let adjustment = try!(indent(|| {
fcx.infcx().commit_if_ok(|_| {
fcx.select_commit_if_ok(|_| {
let coerce = Coerce {
fcx: fcx,
origin: infer::ExprAssignable(expr.span),
Expand Down
16 changes: 16 additions & 0 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.inh.adjustments.borrow_mut().insert(node_id, adj);
}

/// A version of commit_if_ok that allows for registering trait obligations
fn select_commit_if_ok<T, E, F>(&self, f: F) -> Result<T, E> where
F: FnOnce(&infer::CombinedSnapshot) -> Result<T, E> {
self.inh.fulfillment_cx.borrow_mut().begin_transaction();
match self.infcx().commit_if_ok(f) {
Ok(o) => {
self.inh.fulfillment_cx.borrow_mut().commit();
Ok(o)
}
Err(e) => {
self.inh.fulfillment_cx.borrow_mut().rollback();
Err(e)
}
}
}

/// Basically whenever we are converting from a type scheme into
/// the fn body space, we always want to normalize associated
/// types as well. This function combines the two.
Expand Down
16 changes: 16 additions & 0 deletions src/test/compile-fail/issue-24819.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2015 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.

fn main() {
let mut v = Vec::new();
foo(&mut v); //~ ERROR mismatched types
}

fn foo(h: &mut ()) {}

0 comments on commit 2cc7ad0

Please sign in to comment.