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

remove keys w/ skolemized regions from proj cache when popping skolemized regions #37294

Merged
merged 3 commits into from
Oct 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/librustc/infer/higher_ranked/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -839,5 +839,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
debug!("pop_skolemized({:?})", skol_map);
let skol_regions: FnvHashSet<_> = skol_map.values().cloned().collect();
self.region_vars.pop_skolemized(&skol_regions, &snapshot.region_vars_snapshot);
if !skol_map.is_empty() {
self.projection_cache.borrow_mut().rollback_skolemized(
&snapshot.projection_cache_snapshot);
}
}
}
10 changes: 8 additions & 2 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ pub fn poly_project_and_unify_type<'cx, 'gcx, 'tcx>(
infcx.skolemize_late_bound_regions(&obligation.predicate, snapshot);

let skol_obligation = obligation.with(skol_predicate);
match project_and_unify_type(selcx, &skol_obligation) {
let r = match project_and_unify_type(selcx, &skol_obligation) {
Ok(result) => {
let span = obligation.cause.span;
match infcx.leak_check(false, span, &skol_map, snapshot) {
Expand All @@ -178,7 +178,9 @@ pub fn poly_project_and_unify_type<'cx, 'gcx, 'tcx>(
Err(e) => {
Err(e)
}
}
};

r
})
}

Expand Down Expand Up @@ -1396,6 +1398,10 @@ impl<'tcx> ProjectionCache<'tcx> {
self.map.rollback_to(snapshot.snapshot);
}

pub fn rollback_skolemized(&mut self, snapshot: &ProjectionCacheSnapshot) {
self.map.partial_rollback(&snapshot.snapshot, &|k| k.has_re_skol());
}

pub fn commit(&mut self, snapshot: ProjectionCacheSnapshot) {
self.map.commit(snapshot.snapshot);
}
Expand Down
22 changes: 4 additions & 18 deletions src/librustc/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use ty::subst::Substs;
use ty::{self, Ty, TypeFlags, TypeFoldable};

#[derive(Debug)]
pub struct FlagComputation {
pub flags: TypeFlags,

Expand Down Expand Up @@ -182,24 +183,9 @@ impl FlagComputation {
}

fn add_region(&mut self, r: &ty::Region) {
match *r {
ty::ReVar(..) => {
self.add_flags(TypeFlags::HAS_RE_INFER);
self.add_flags(TypeFlags::KEEP_IN_LOCAL_TCX);
}
ty::ReSkolemized(..) => {
self.add_flags(TypeFlags::HAS_RE_INFER);
self.add_flags(TypeFlags::HAS_RE_SKOL);
self.add_flags(TypeFlags::KEEP_IN_LOCAL_TCX);
}
ty::ReLateBound(debruijn, _) => { self.add_depth(debruijn.depth); }
ty::ReEarlyBound(..) => { self.add_flags(TypeFlags::HAS_RE_EARLY_BOUND); }
ty::ReStatic | ty::ReErased => {}
_ => { self.add_flags(TypeFlags::HAS_FREE_REGIONS); }
}

if !r.is_global() {
self.add_flags(TypeFlags::HAS_LOCAL_NAMES);
self.add_flags(r.type_flags());
if let ty::ReLateBound(debruijn, _) = *r {
self.add_depth(debruijn.depth);
}
}

Expand Down
26 changes: 9 additions & 17 deletions src/librustc/ty/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone {
fn needs_subst(&self) -> bool {
self.has_type_flags(TypeFlags::NEEDS_SUBST)
}
fn has_re_skol(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_RE_SKOL)
}
fn has_closure_types(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_TY_CLOSURE)
}
Expand Down Expand Up @@ -632,26 +635,15 @@ struct HasTypeFlagsVisitor {

impl<'tcx> TypeVisitor<'tcx> for HasTypeFlagsVisitor {
fn visit_ty(&mut self, t: Ty) -> bool {
t.flags.get().intersects(self.flags)
let flags = t.flags.get();
debug!("HasTypeFlagsVisitor: t={:?} t.flags={:?} self.flags={:?}", t, flags, self.flags);
flags.intersects(self.flags)
}

fn visit_region(&mut self, r: &'tcx ty::Region) -> bool {
if self.flags.intersects(ty::TypeFlags::HAS_LOCAL_NAMES) {
// does this represent a region that cannot be named
// in a global way? used in fulfillment caching.
match *r {
ty::ReStatic | ty::ReEmpty | ty::ReErased => {}
_ => return true,
}
}
if self.flags.intersects(ty::TypeFlags::HAS_RE_INFER |
ty::TypeFlags::KEEP_IN_LOCAL_TCX) {
match *r {
ty::ReVar(_) | ty::ReSkolemized(..) => { return true }
_ => {}
}
}
false
let flags = r.type_flags();
debug!("HasTypeFlagsVisitor: r={:?} r.flags={:?} self.flags={:?}", r, flags, self.flags);
flags.intersects(self.flags)
}
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ bitflags! {
TypeFlags::HAS_SELF.bits |
TypeFlags::HAS_TY_INFER.bits |
TypeFlags::HAS_RE_INFER.bits |
TypeFlags::HAS_RE_SKOL.bits |
TypeFlags::HAS_RE_EARLY_BOUND.bits |
TypeFlags::HAS_FREE_REGIONS.bits |
TypeFlags::HAS_TY_ERR.bits |
Expand Down
31 changes: 30 additions & 1 deletion src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ impl<T> Binder<T> {

impl fmt::Debug for TypeFlags {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", self.bits)
write!(f, "{:x}", self.bits)
}
}

Expand Down Expand Up @@ -866,6 +866,35 @@ impl Region {
r => r
}
}

pub fn type_flags(&self) -> TypeFlags {
let mut flags = TypeFlags::empty();

match *self {
ty::ReVar(..) => {
flags = flags | TypeFlags::HAS_RE_INFER;
flags = flags | TypeFlags::KEEP_IN_LOCAL_TCX;
}
ty::ReSkolemized(..) => {
flags = flags | TypeFlags::HAS_RE_INFER;
flags = flags | TypeFlags::HAS_RE_SKOL;
flags = flags | TypeFlags::KEEP_IN_LOCAL_TCX;
}
ty::ReLateBound(..) => { }
ty::ReEarlyBound(..) => { flags = flags | TypeFlags::HAS_RE_EARLY_BOUND; }
ty::ReStatic | ty::ReErased => { }
_ => { flags = flags | TypeFlags::HAS_FREE_REGIONS; }
}

match *self {
ty::ReStatic | ty::ReEmpty | ty::ReErased => (),
_ => flags = flags | TypeFlags::HAS_LOCAL_NAMES,
}

debug!("type_flags({:?}) = {:?}", self, flags);

flags
}
}

// Type utilities
Expand Down
61 changes: 46 additions & 15 deletions src/librustc_data_structures/snapshot_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use fnv::FnvHashMap;
use std::hash::Hash;
use std::ops;
use std::mem;

#[cfg(test)]
mod test;
Expand All @@ -31,6 +32,7 @@ enum UndoLog<K, V> {
CommittedSnapshot,
Inserted(K),
Overwrite(K, V),
Noop,
}

impl<K, V> SnapshotMap<K, V>
Expand Down Expand Up @@ -100,24 +102,33 @@ impl<K, V> SnapshotMap<K, V>
}
}

pub fn partial_rollback<F>(&mut self,
snapshot: &Snapshot,
should_revert_key: &F)
where F: Fn(&K) -> bool
{
self.assert_open_snapshot(snapshot);
for i in (snapshot.len + 1..self.undo_log.len()).rev() {
let reverse = match self.undo_log[i] {
UndoLog::OpenSnapshot => false,
UndoLog::CommittedSnapshot => false,
UndoLog::Noop => false,
UndoLog::Inserted(ref k) => should_revert_key(k),
UndoLog::Overwrite(ref k, _) => should_revert_key(k),
};

if reverse {
let entry = mem::replace(&mut self.undo_log[i], UndoLog::Noop);
self.reverse(entry);
}
}
}

pub fn rollback_to(&mut self, snapshot: Snapshot) {
self.assert_open_snapshot(&snapshot);
while self.undo_log.len() > snapshot.len + 1 {
match self.undo_log.pop().unwrap() {
UndoLog::OpenSnapshot => {
panic!("cannot rollback an uncommitted snapshot");
}

UndoLog::CommittedSnapshot => {}

UndoLog::Inserted(key) => {
self.map.remove(&key);
}

UndoLog::Overwrite(key, old_value) => {
self.map.insert(key, old_value);
}
}
let entry = self.undo_log.pop().unwrap();
self.reverse(entry);
}

let v = self.undo_log.pop().unwrap();
Expand All @@ -127,6 +138,26 @@ impl<K, V> SnapshotMap<K, V>
});
assert!(self.undo_log.len() == snapshot.len);
}

fn reverse(&mut self, entry: UndoLog<K, V>) {
match entry {
UndoLog::OpenSnapshot => {
panic!("cannot rollback an uncommitted snapshot");
}

UndoLog::CommittedSnapshot => {}

UndoLog::Inserted(key) => {
self.map.remove(&key);
}

UndoLog::Overwrite(key, old_value) => {
self.map.insert(key, old_value);
}

UndoLog::Noop => {}
}
}
}

impl<'k, K, V> ops::Index<&'k K> for SnapshotMap<K, V>
Expand Down
28 changes: 28 additions & 0 deletions src/test/run-pass/project-cache-issue-37154.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 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.

// Regression test for #37154: the problem here was that the cache
// results in a false error because it was caching skolemized results
// even after those skolemized regions had been popped.

trait Foo {
fn method(&self) {}
}

struct Wrapper<T>(T);

impl<T> Foo for Wrapper<T> where for<'a> &'a T: IntoIterator<Item=&'a ()> {}

fn f(x: Wrapper<Vec<()>>) {
x.method(); // This works.
x.method(); // error: no method named `method`
}

fn main() { }