From bca56e82a1bbd775498257cc080f5ea27b214f49 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 11 Apr 2017 17:17:58 -0400 Subject: [PATCH] generalize type variables too When we are generalizing a super/sub-type, we have to replace type variables with a fresh variable (and not just region variables). So if we know that `Box <: ?U`, for example, we instantiate `?U` with `Box` and then relate `Box` to `Box` (and hence require that `?T <: ?V`). This change has some complex interactions, however: First, the occurs check must be updated to detect constraints like `?T <: ?U` and `?U <: Box`. If we're not careful, we'll create a never-ending sequence of new variables. To address this, we add a second unification set into `type_variables` that tracks type variables related through **either** equality **or** subtyping, and use that during the occurs-check. Second, the "fudge regions if ok" code was expecting no new type variables to be created. It must be updated to create new type variables outside of the probe. This is relatively straight-forward under the new scheme, since type variables are now independent from one another, and any relations are moderated by pending subtype obliations and so forth. This part would be tricky to backport though. cc #18653 cc #40951 --- src/librustc/infer/combine.rs | 46 +++++--- src/librustc/infer/fudge.rs | 73 ++++++++---- src/librustc/infer/mod.rs | 6 +- src/librustc/infer/sub.rs | 8 +- src/librustc/infer/type_variable.rs | 108 +++++++++++++++++- src/librustc/traits/error_reporting.rs | 24 +++- src/test/run-pass/issue-40951.rs | 20 ++++ .../run-pass/type-infer-generalize-ty-var.rs | 60 ++++++++++ 8 files changed, 296 insertions(+), 49 deletions(-) create mode 100644 src/test/run-pass/issue-40951.rs create mode 100644 src/test/run-pass/type-infer-generalize-ty-var.rs diff --git a/src/librustc/infer/combine.rs b/src/librustc/infer/combine.rs index a23589e7f3f..03ed654e3cc 100644 --- a/src/librustc/infer/combine.rs +++ b/src/librustc/infer/combine.rs @@ -264,20 +264,27 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> { Ok(()) } - /// Attempts to generalize `ty` for the type variable `for_vid`. This checks for cycle -- that - /// is, whether the type `ty` references `for_vid`. If `make_region_vars` is true, it will also - /// replace all regions with fresh variables. Returns `TyError` in the case of a cycle, `Ok` - /// otherwise. + /// Attempts to generalize `ty` for the type variable `for_vid`. + /// This checks for cycle -- that is, whether the type `ty` + /// references `for_vid`. If `make_region_vars` is true, it will + /// also replace all regions with fresh variables. Returns + /// `TyError` in the case of a cycle, `Ok` otherwise. + /// + /// Preconditions: + /// + /// - `for_vid` is a "root vid" fn generalize(&self, ty: Ty<'tcx>, for_vid: ty::TyVid, make_region_vars: bool) -> RelateResult<'tcx, Ty<'tcx>> { + debug_assert!(self.infcx.type_variables.borrow_mut().root_var(for_vid) == for_vid); + let mut generalize = Generalizer { infcx: self.infcx, span: self.trace.cause.span, - for_vid: for_vid, + for_vid_sub_root: self.infcx.type_variables.borrow_mut().sub_root_var(for_vid), make_region_vars: make_region_vars, cycle_detected: false }; @@ -293,7 +300,7 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> { struct Generalizer<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>, span: Span, - for_vid: ty::TyVid, + for_vid_sub_root: ty::TyVid, make_region_vars: bool, cycle_detected: bool, } @@ -305,17 +312,17 @@ impl<'cx, 'gcx, 'tcx> ty::fold::TypeFolder<'gcx, 'tcx> for Generalizer<'cx, 'gcx fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> { // Check to see whether the type we are genealizing references - // `vid`. At the same time, also update any type variables to - // the values that they are bound to. This is needed to truly - // check for cycles, but also just makes things readable. - // - // (In particular, you could have something like `$0 = Box<$1>` - // where `$1` has already been instantiated with `Box<$0>`) + // any other type variable related to `vid` via + // subtyping. This is basically our "occurs check", preventing + // us from creating infinitely sized types. match t.sty { ty::TyInfer(ty::TyVar(vid)) => { let mut variables = self.infcx.type_variables.borrow_mut(); let vid = variables.root_var(vid); - if vid == self.for_vid { + let sub_vid = variables.sub_root_var(vid); + if sub_vid == self.for_vid_sub_root { + // If sub-roots are equal, then `for_vid` and + // `vid` are related via subtyping. self.cycle_detected = true; self.tcx().types.err } else { @@ -324,7 +331,18 @@ impl<'cx, 'gcx, 'tcx> ty::fold::TypeFolder<'gcx, 'tcx> for Generalizer<'cx, 'gcx drop(variables); self.fold_ty(u) } - None => t, + None => { + if self.make_region_vars { + let origin = variables.origin(vid); + let new_var_id = variables.new_var(false, origin, None); + let u = self.tcx().mk_var(new_var_id); + debug!("generalize: replacing original vid={:?} with new={:?}", + vid, u); + u + } else { + t + } + } } } } diff --git a/src/librustc/infer/fudge.rs b/src/librustc/infer/fudge.rs index 806b9448661..ab0ff32dcc3 100644 --- a/src/librustc/infer/fudge.rs +++ b/src/librustc/infer/fudge.rs @@ -8,7 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use ty::{self, TyCtxt}; +use infer::type_variable::TypeVariableMap; +use ty::{self, Ty, TyCtxt}; use ty::fold::{TypeFoldable, TypeFolder}; use super::InferCtxt; @@ -54,57 +55,52 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { /// the actual types (`?T`, `Option(&self, origin: &RegionVariableOrigin, f: F) -> Result where F: FnOnce() -> Result, T: TypeFoldable<'tcx>, { - let (region_vars, value) = self.probe(|snapshot| { - let vars_at_start = self.type_variables.borrow().num_vars(); + debug!("fudge_regions_if_ok(origin={:?})", origin); + let (type_variables, region_vars, value) = self.probe(|snapshot| { match f() { Ok(value) => { let value = self.resolve_type_vars_if_possible(&value); // At this point, `value` could in principle refer - // to regions that have been created during the - // snapshot (we assert below that `f()` does not - // create any new type variables, so there - // shouldn't be any of those). Once we exit - // `probe()`, those are going to be popped, so we - // will have to eliminate any references to them. + // to types/regions that have been created during + // the snapshot. Once we exit `probe()`, those are + // going to be popped, so we will have to + // eliminate any references to them. - assert_eq!(self.type_variables.borrow().num_vars(), vars_at_start, - "type variables were created during fudge_regions_if_ok"); + let type_variables = + self.type_variables.borrow_mut().types_created_since_snapshot( + &snapshot.type_snapshot); let region_vars = self.region_vars.vars_created_since_snapshot( &snapshot.region_vars_snapshot); - Ok((region_vars, value)) + Ok((type_variables, region_vars, value)) } Err(e) => Err(e), } })?; // At this point, we need to replace any of the now-popped - // region variables that appear in `value` with a fresh region - // variable. We can't do this during the probe because they - // would just get popped then too. =) + // type/region variables that appear in `value` with a fresh + // variable of the appropriate kind. We can't do this during + // the probe because they would just get popped then too. =) // Micro-optimization: if no variables have been created, then // `value` can't refer to any of them. =) So we can just return it. - if region_vars.is_empty() { + if type_variables.is_empty() && region_vars.is_empty() { return Ok(value); } let mut fudger = RegionFudger { infcx: self, + type_variables: &type_variables, region_vars: ®ion_vars, origin: origin }; @@ -115,6 +111,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { pub struct RegionFudger<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, + type_variables: &'a TypeVariableMap, region_vars: &'a Vec, origin: &'a RegionVariableOrigin, } @@ -124,6 +121,40 @@ impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for RegionFudger<'a, 'gcx, 'tcx> { self.infcx.tcx } + fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { + match ty.sty { + ty::TyInfer(ty::InferTy::TyVar(vid)) => { + match self.type_variables.get(&vid) { + None => { + // This variable was created before the + // "fudging". Since we refresh all type + // variables to their binding anyhow, we know + // that it is unbound, so we can just return + // it. + debug_assert!(self.infcx.type_variables.borrow_mut().probe(vid).is_none()); + ty + } + + Some(info) => { + // This variable was created during the + // fudging; it was mapped the root + // `root_vid`. There are now two + // possibilities: either the root was creating + // during the fudging too, in which case we + // want a fresh variable, or it was not, in + // which case we can return it. + if self.type_variables.contains_key(&info.root_vid) { + self.infcx.next_ty_var(info.root_origin) + } else { + self.infcx.tcx.mk_var(info.root_vid) + } + } + } + } + _ => ty.super_fold_with(self), + } + } + fn fold_region(&mut self, r: &'tcx ty::Region) -> &'tcx ty::Region { match *r { ty::ReVar(v) if self.region_vars.contains(&v) => { diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index 999ebbfa20f..e98792b120d 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -1036,9 +1036,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { self.probe(|_| { let origin = &ObligationCause::dummy(); let trace = TypeTrace::types(origin, true, a, b); - self.sub(true, trace, &a, &b).map(|InferOk { obligations, .. }| { - // FIXME(#32730) propagate obligations - assert!(obligations.is_empty()); + self.sub(true, trace, &a, &b).map(|InferOk { obligations: _, .. }| { + // Ignore obligations, since we are unrolling + // everything anyway. }) }) } diff --git a/src/librustc/infer/sub.rs b/src/librustc/infer/sub.rs index f1de9b043e3..2a7dbbc026b 100644 --- a/src/librustc/infer/sub.rs +++ b/src/librustc/infer/sub.rs @@ -80,7 +80,7 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx> let a = infcx.type_variables.borrow_mut().replace_if_possible(a); let b = infcx.type_variables.borrow_mut().replace_if_possible(b); match (&a.sty, &b.sty) { - (&ty::TyInfer(TyVar(_)), &ty::TyInfer(TyVar(_))) => { + (&ty::TyInfer(TyVar(a_vid)), &ty::TyInfer(TyVar(b_vid))) => { // Shouldn't have any LBR here, so we can safely put // this under a binder below without fear of accidental // capture. @@ -88,7 +88,11 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx> assert!(!b.has_escaping_regions()); // can't make progress on `A <: B` if both A and B are - // type variables, so record an obligation. + // type variables, so record an obligation. We also + // have to record in the `type_variables` tracker that + // the two variables are equal modulo subtyping, which + // is important to the occurs check later on. + infcx.type_variables.borrow_mut().sub(a_vid, b_vid); self.fields.obligations.push( Obligation::new( self.fields.trace.cause.clone(), diff --git a/src/librustc/infer/type_variable.rs b/src/librustc/infer/type_variable.rs index 298b2a97d5f..a32404c1ac5 100644 --- a/src/librustc/infer/type_variable.rs +++ b/src/librustc/infer/type_variable.rs @@ -18,16 +18,39 @@ use std::cmp::min; use std::marker::PhantomData; use std::mem; use std::u32; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::snapshot_vec as sv; use rustc_data_structures::unify as ut; pub struct TypeVariableTable<'tcx> { values: sv::SnapshotVec>, + + /// Two variables are unified in `eq_relations` when we have a + /// constraint `?X == ?Y`. eq_relations: ut::UnificationTable, + + /// Two variables are unified in `eq_relations` when we have a + /// constraint `?X <: ?Y` *or* a constraint `?Y <: ?X`. This second + /// table exists only to help with the occurs check. In particular, + /// we want to report constraints like these as an occurs check + /// violation: + /// + /// ?1 <: ?3 + /// Box <: ?1 + /// + /// This works because `?1` and `?3` are unified in the + /// `sub_relations` relation (not in `eq_relations`). Then when we + /// process the `Box <: ?1` constraint, we do an occurs check + /// on `Box` and find a potential cycle. + /// + /// This is reasonable because, in Rust, subtypes have the same + /// "skeleton" and hence there is no possible type such that + /// (e.g.) `Box <: ?3` for any `?3`. + sub_relations: ut::UnificationTable, } /// Reasons to create a type inference variable -#[derive(Debug)] +#[derive(Copy, Clone, Debug)] pub enum TypeVariableOrigin { MiscVariable(Span), NormalizeProjectionType(Span), @@ -41,6 +64,14 @@ pub enum TypeVariableOrigin { DivergingBlockExpr(Span), DivergingFn(Span), LatticeVariable(Span), + Generalized(ty::TyVid), +} + +pub type TypeVariableMap = FxHashMap; + +pub struct TypeVariableInfo { + pub root_vid: ty::TyVid, + pub root_origin: TypeVariableOrigin, } struct TypeVariableData<'tcx> { @@ -70,6 +101,7 @@ pub struct Default<'tcx> { pub struct Snapshot { snapshot: sv::Snapshot, eq_snapshot: ut::Snapshot, + sub_snapshot: ut::Snapshot, } struct Instantiate<'tcx> { @@ -84,6 +116,7 @@ impl<'tcx> TypeVariableTable<'tcx> { TypeVariableTable { values: sv::SnapshotVec::new(), eq_relations: ut::UnificationTable::new(), + sub_relations: ut::UnificationTable::new(), } } @@ -109,6 +142,16 @@ impl<'tcx> TypeVariableTable<'tcx> { debug_assert!(self.probe(a).is_none()); debug_assert!(self.probe(b).is_none()); self.eq_relations.union(a, b); + self.sub_relations.union(a, b); + } + + /// Records that `a <: b`, depending on `dir`. + /// + /// Precondition: neither `a` nor `b` are known. + pub fn sub(&mut self, a: ty::TyVid, b: ty::TyVid) { + debug_assert!(self.probe(a).is_none()); + debug_assert!(self.probe(b).is_none()); + self.sub_relations.union(a, b); } /// Instantiates `vid` with the type `ty`. @@ -141,6 +184,7 @@ impl<'tcx> TypeVariableTable<'tcx> { default: Option>,) -> ty::TyVid { debug!("new_var(diverging={:?}, origin={:?})", diverging, origin); self.eq_relations.new_key(()); + self.sub_relations.new_key(()); let index = self.values.push(TypeVariableData { value: Bounded { default: default }, origin: origin, @@ -155,15 +199,41 @@ impl<'tcx> TypeVariableTable<'tcx> { self.values.len() } + /// Returns the "root" variable of `vid` in the `eq_relations` + /// equivalence table. All type variables that have been equated + /// will yield the same root variable (per the union-find + /// algorithm), so `root_var(a) == root_var(b)` implies that `a == + /// b` (transitively). pub fn root_var(&mut self, vid: ty::TyVid) -> ty::TyVid { self.eq_relations.find(vid) } + /// Returns the "root" variable of `vid` in the `sub_relations` + /// equivalence table. All type variables that have been are + /// related via equality or subtyping will yield the same root + /// variable (per the union-find algorithm), so `sub_root_var(a) + /// == sub_root_var(b)` implies that: + /// + /// exists X. (a <: X || X <: a) && (b <: X || X <: b) + pub fn sub_root_var(&mut self, vid: ty::TyVid) -> ty::TyVid { + self.sub_relations.find(vid) + } + + /// True if `a` and `b` have same "sub-root" (i.e., exists some + /// type X such that `forall i in {a, b}. (i <: X || X <: i)`. + pub fn sub_unified(&mut self, a: ty::TyVid, b: ty::TyVid) -> bool { + self.sub_root_var(a) == self.sub_root_var(b) + } + pub fn probe(&mut self, vid: ty::TyVid) -> Option> { let vid = self.root_var(vid); self.probe_root(vid) } + pub fn origin(&self, vid: ty::TyVid) -> TypeVariableOrigin { + self.values.get(vid.index as usize).origin.clone() + } + /// Retrieves the type of `vid` given that it is currently a root in the unification table pub fn probe_root(&mut self, vid: ty::TyVid) -> Option> { debug_assert!(self.root_var(vid) == vid); @@ -189,6 +259,7 @@ impl<'tcx> TypeVariableTable<'tcx> { Snapshot { snapshot: self.values.start_snapshot(), eq_snapshot: self.eq_relations.snapshot(), + sub_snapshot: self.sub_relations.snapshot(), } } @@ -204,13 +275,40 @@ impl<'tcx> TypeVariableTable<'tcx> { } }); - self.values.rollback_to(s.snapshot); - self.eq_relations.rollback_to(s.eq_snapshot); + let Snapshot { snapshot, eq_snapshot, sub_snapshot } = s; + self.values.rollback_to(snapshot); + self.eq_relations.rollback_to(eq_snapshot); + self.sub_relations.rollback_to(sub_snapshot); } pub fn commit(&mut self, s: Snapshot) { - self.values.commit(s.snapshot); - self.eq_relations.commit(s.eq_snapshot); + let Snapshot { snapshot, eq_snapshot, sub_snapshot } = s; + self.values.commit(snapshot); + self.eq_relations.commit(eq_snapshot); + self.sub_relations.commit(sub_snapshot); + } + + /// Returns a map `{V1 -> V2}`, where the keys `{V1}` are + /// ty-variables created during the snapshot, and the values + /// `{V2}` are the root variables that they were unified with, + /// along with their origin. + pub fn types_created_since_snapshot(&mut self, s: &Snapshot) -> TypeVariableMap { + let actions_since_snapshot = self.values.actions_since_snapshot(&s.snapshot); + let eq_relations = &mut self.eq_relations; + let values = &self.values; + + actions_since_snapshot + .iter() + .filter_map(|action| match action { + &sv::UndoLog::NewElem(index) => Some(ty::TyVid { index: index as u32 }), + _ => None, + }) + .map(|vid| { + let root_vid = eq_relations.find(vid); + let root_origin = values.get(vid.index as usize).origin.clone(); + (vid, TypeVariableInfo { root_vid, root_origin }) + }) + .collect() } pub fn types_escaping_snapshot(&mut self, s: &Snapshot) -> Vec> { diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 8a303a5da11..f7a7d0e2071 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -69,6 +69,19 @@ struct FindLocalByTypeVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { found_pattern: Option<&'a Pat>, } +impl<'a, 'gcx, 'tcx> FindLocalByTypeVisitor<'a, 'gcx, 'tcx> { + fn is_match(&self, ty: Ty<'tcx>) -> bool { + ty == *self.target_ty || match (&ty.sty, &self.target_ty.sty) { + (&ty::TyInfer(ty::TyVar(a_vid)), &ty::TyInfer(ty::TyVar(b_vid))) => + self.infcx.type_variables + .borrow_mut() + .sub_unified(a_vid, b_vid), + + _ => false, + } + } +} + impl<'a, 'gcx, 'tcx> Visitor<'a> for FindLocalByTypeVisitor<'a, 'gcx, 'tcx> { fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'a> { NestedVisitorMap::None @@ -77,7 +90,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'a> for FindLocalByTypeVisitor<'a, 'gcx, 'tcx> { fn visit_local(&mut self, local: &'a Local) { if let Some(&ty) = self.infcx.tables.borrow().node_types.get(&local.id) { let ty = self.infcx.resolve_type_vars_if_possible(&ty); - let is_match = ty.walk().any(|t| t == *self.target_ty); + let is_match = ty.walk().any(|t| self.is_match(t)); if is_match && self.found_pattern.is_none() { self.found_pattern = Some(&*local.pat); @@ -564,8 +577,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } ty::Predicate::Subtype(ref predicate) => { - // TODO - panic!("subtype requirement not satisfied {:?}", predicate) + // Errors for Subtype predicates show up as + // `FulfillmentErrorCode::CodeSubtypeError`, + // not selection error. + span_bug!(span, "subtype requirement gave wrong error: `{:?}`", predicate) } ty::Predicate::Equate(ref predicate) => { @@ -779,7 +794,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // no need to overload user in such cases } else { let &SubtypePredicate { a_is_expected: _, a, b } = data.skip_binder(); - assert!(a.is_ty_var() && b.is_ty_var()); // else other would've been instantiated + // both must be type variables, or the other would've been instantiated + assert!(a.is_ty_var() && b.is_ty_var()); self.need_type_info(obligation, a); } } diff --git a/src/test/run-pass/issue-40951.rs b/src/test/run-pass/issue-40951.rs new file mode 100644 index 00000000000..adc7101b16a --- /dev/null +++ b/src/test/run-pass/issue-40951.rs @@ -0,0 +1,20 @@ +// 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 #40951. + +const FOO: [&'static str; 1] = ["foo"]; + +fn find(t: &[T], element: &T) { } + +fn main() { + let x = format!("hi"); + find(&FOO, &&*x); +} diff --git a/src/test/run-pass/type-infer-generalize-ty-var.rs b/src/test/run-pass/type-infer-generalize-ty-var.rs new file mode 100644 index 00000000000..d7fb85ca484 --- /dev/null +++ b/src/test/run-pass/type-infer-generalize-ty-var.rs @@ -0,0 +1,60 @@ +// 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. + +// Test a scenario where we generate a constraint like `?1 <: &?2`. +// In such a case, it is important that we instantiate `?1` with `&?3` +// where `?3 <: ?2`, and not with `&?2`. This is a regression test for +// #18653. The important thing is that we build. + +use std::cell::RefCell; + +enum Wrap { + WrapSome(A), + WrapNone +} + +use Wrap::*; + +struct T; +struct U; + +trait Get { + fn get(&self) -> &T; +} + +impl Get for Wrap { + fn get(&self) -> &(MyShow + 'static) { + static x: usize = 42; + &x + } +} + +impl Get for Wrap { + fn get(&self) -> &usize { + static x: usize = 55; + &x + } +} + +trait MyShow { fn dummy(&self) { } } +impl<'a> MyShow for &'a (MyShow + 'a) { } +impl MyShow for usize { } +fn constrain<'a>(rc: RefCell<&'a (MyShow + 'a)>) { } + +fn main() { + let mut collection: Wrap<_> = WrapNone; + + { + let __arg0 = Get::get(&collection); + let __args_cell = RefCell::new(__arg0); + constrain(__args_cell); + } + collection = WrapSome(T); +}