Skip to content

Commit

Permalink
Avoid ever constructing cyclic types in the first place, rather than …
Browse files Browse the repository at this point in the history
…detecting them in resolve. This simplifies logic elsewhere in the compiler. cc #5527
  • Loading branch information
nikomatsakis committed Sep 9, 2014
1 parent b625d43 commit c4d56b7
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 51 deletions.
4 changes: 3 additions & 1 deletion src/librustc/middle/ty.rs
Expand Up @@ -1002,7 +1002,8 @@ pub enum type_err {
terr_float_mismatch(expected_found<ast::FloatTy>),
terr_traits(expected_found<ast::DefId>),
terr_builtin_bounds(expected_found<BuiltinBounds>),
terr_variadic_mismatch(expected_found<bool>)
terr_variadic_mismatch(expected_found<bool>),
terr_cyclic_ty,
}

/// Bounds suitable for a named type parameter like `A` in `fn foo<A>`
Expand Down Expand Up @@ -3790,6 +3791,7 @@ pub fn type_err_to_str(cx: &ctxt, err: &type_err) -> String {
}

match *err {
terr_cyclic_ty => "cyclic type of infinite size".to_string(),
terr_mismatch => "types differ".to_string(),
terr_fn_style_mismatch(values) => {
format!("expected {} fn, found {} fn",
Expand Down
104 changes: 90 additions & 14 deletions src/librustc/middle/typeck/infer/combine.rs
Expand Up @@ -39,6 +39,7 @@ use middle::ty::{FloatVar, FnSig, IntVar, TyVar};
use middle::ty::{IntType, UintType};
use middle::ty::{BuiltinBounds};
use middle::ty;
use middle::ty_fold;
use middle::typeck::infer::equate::Equate;
use middle::typeck::infer::glb::Glb;
use middle::typeck::infer::lub::Lub;
Expand All @@ -48,14 +49,15 @@ use middle::typeck::infer::{InferCtxt, cres};
use middle::typeck::infer::{MiscVariable, TypeTrace};
use middle::typeck::infer::type_variable::{RelationDir, EqTo,
SubtypeOf, SupertypeOf};
use middle::ty_fold::{RegionFolder, TypeFoldable};
use middle::ty_fold::{TypeFoldable};
use util::ppaux::Repr;

use std::result;

use syntax::ast::{Onceness, FnStyle};
use syntax::ast;
use syntax::abi;
use syntax::codemap::Span;

pub trait Combine<'tcx> {
fn infcx<'a>(&'a self) -> &'a InferCtxt<'a, 'tcx>;
Expand Down Expand Up @@ -637,10 +639,14 @@ impl<'f, 'tcx> CombineFields<'f, 'tcx> {
Some(t) => t, // ...already instantiated.
None => { // ...not yet instantiated:
// Generalize type if necessary.
let generalized_ty = match dir {
EqTo => a_ty,
SupertypeOf | SubtypeOf => self.generalize(a_ty)
};
let generalized_ty = try!(match dir {
EqTo => {
self.generalize(a_ty, b_vid, false)
}
SupertypeOf | SubtypeOf => {
self.generalize(a_ty, b_vid, true)
}
});
debug!("instantiate(a_ty={}, dir={}, \
b_vid={}, generalized_ty={})",
a_ty.repr(tcx), dir, b_vid.repr(tcx),
Expand Down Expand Up @@ -678,15 +684,85 @@ impl<'f, 'tcx> CombineFields<'f, 'tcx> {
Ok(())
}

fn generalize(&self, t: ty::t) -> ty::t {
// FIXME(#16847): This is non-ideal because we don't give a
// very descriptive origin for this region variable.
fn generalize(&self,
ty: ty::t,
for_vid: ty::TyVid,
make_region_vars: bool)
-> cres<ty::t>
{
/*!
* 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 `ty_err` in the case of a cycle, `Ok`
* otherwise.
*/

let mut generalize = Generalizer { infcx: self.infcx,
span: self.trace.origin.span(),
for_vid: for_vid,
make_region_vars: make_region_vars,
cycle_detected: false };
let u = ty.fold_with(&mut generalize);
if generalize.cycle_detected {
Err(ty::terr_cyclic_ty)
} else {
Ok(u)
}
}
}

let infcx = self.infcx;
let span = self.trace.origin.span();
t.fold_with(
&mut RegionFolder::regions(
self.infcx.tcx,
|_| infcx.next_region_var(MiscVariable(span))))
struct Generalizer<'cx, 'tcx:'cx> {
infcx: &'cx InferCtxt<'cx, 'tcx>,
span: Span,
for_vid: ty::TyVid,
make_region_vars: bool,
cycle_detected: bool,
}

impl<'cx, 'tcx> ty_fold::TypeFolder<'tcx> for Generalizer<'cx, 'tcx> {
fn tcx(&self) -> &ty::ctxt<'tcx> {
self.infcx.tcx
}

fn fold_ty(&mut self, t: ty::t) -> ty::t {
// 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>`)
match ty::get(t).sty {
ty::ty_infer(ty::TyVar(vid)) => {
if vid == self.for_vid {
self.cycle_detected = true;
ty::mk_err()
} else {
match self.infcx.type_variables.borrow().probe(vid) {
Some(u) => self.fold_ty(u),
None => t,
}
}
}
_ => {
ty_fold::super_fold_ty(self, t)
}
}
}

fn fold_region(&mut self, r: ty::Region) -> ty::Region {
match r {
ty::ReLateBound(..) | ty::ReEarlyBound(..) => r,
_ if self.make_region_vars => {
// FIXME: This is non-ideal because we don't give a
// very descriptive origin for this region variable.
self.infcx.next_region_var(MiscVariable(self.span))
}
_ => r,
}
}
}


2 changes: 0 additions & 2 deletions src/librustc/middle/typeck/infer/mod.rs
Expand Up @@ -266,7 +266,6 @@ pub enum fixup_err {
unresolved_int_ty(IntVid),
unresolved_float_ty(FloatVid),
unresolved_ty(TyVid),
cyclic_ty(TyVid),
unresolved_region(RegionVid),
region_var_bound_by_region_var(RegionVid, RegionVid)
}
Expand All @@ -282,7 +281,6 @@ pub fn fixup_err_to_string(f: fixup_err) -> String {
the type explicitly".to_string()
}
unresolved_ty(_) => "unconstrained type".to_string(),
cyclic_ty(_) => "cyclic type of infinite size".to_string(),
unresolved_region(_) => "unconstrained region".to_string(),
region_var_bound_by_region_var(r1, r2) => {
format!("region var {:?} bound by another region var {:?}; \
Expand Down
46 changes: 14 additions & 32 deletions src/librustc/middle/typeck/infer/resolve.rs
Expand Up @@ -51,7 +51,7 @@ use middle::ty::{FloatVar, FloatVid, IntVar, IntVid, RegionVid, TyVar, TyVid};
use middle::ty::{IntType, UintType};
use middle::ty;
use middle::ty_fold;
use middle::typeck::infer::{cyclic_ty, fixup_err, fres, InferCtxt};
use middle::typeck::infer::{fixup_err, fres, InferCtxt};
use middle::typeck::infer::{unresolved_int_ty,unresolved_float_ty,unresolved_ty};
use syntax::codemap::Span;
use util::common::indent;
Expand All @@ -78,7 +78,6 @@ pub struct ResolveState<'a, 'tcx: 'a> {
infcx: &'a InferCtxt<'a, 'tcx>,
modes: uint,
err: Option<fixup_err>,
v_seen: Vec<TyVid> ,
type_depth: uint,
}

Expand All @@ -90,7 +89,6 @@ pub fn resolver<'a, 'tcx>(infcx: &'a InferCtxt<'a, 'tcx>,
infcx: infcx,
modes: modes,
err: None,
v_seen: Vec::new(),
type_depth: 0,
}
}
Expand Down Expand Up @@ -126,9 +124,7 @@ impl<'a, 'tcx> ResolveState<'a, 'tcx> {
// n.b. This is a hokey mess because the current fold doesn't
// allow us to pass back errors in any useful way.

assert!(self.v_seen.is_empty());
let rty = indent(|| self.resolve_type(typ) );
assert!(self.v_seen.is_empty());
let rty = self.resolve_type(typ);
match self.err {
None => {
debug!("Resolved {} to {} (modes={:x})",
Expand Down Expand Up @@ -205,33 +201,19 @@ impl<'a, 'tcx> ResolveState<'a, 'tcx> {
}

pub fn resolve_ty_var(&mut self, vid: TyVid) -> ty::t {
if self.v_seen.contains(&vid) {
self.err = Some(cyclic_ty(vid));
return ty::mk_var(self.infcx.tcx, vid);
} else {
self.v_seen.push(vid);
let tcx = self.infcx.tcx;

// Nonobvious: prefer the most specific type
// (i.e., the lower bound) to the more general
// one. More general types in Rust (e.g., fn())
// tend to carry more restrictions or higher
// perf. penalties, so it pays to know more.

let t1 = match self.infcx.type_variables.borrow().probe(vid) {
Some(t) => {
self.resolve_type(t)
}
None => {
if self.should(force_tvar) {
self.err = Some(unresolved_ty(vid));
}
ty::mk_var(tcx, vid)
let tcx = self.infcx.tcx;
let t1 = match self.infcx.type_variables.borrow().probe(vid) {
Some(t) => {
self.resolve_type(t)
}
None => {
if self.should(force_tvar) {
self.err = Some(unresolved_ty(vid));
}
};
self.v_seen.pop().unwrap();
return t1;
}
ty::mk_var(tcx, vid)
}
};
return t1;
}

pub fn resolve_int_var(&mut self, vid: IntVid) -> ty::t {
Expand Down
18 changes: 18 additions & 0 deletions src/test/compile-fail/occurs-check-2.rs
@@ -0,0 +1,18 @@
// Copyright 2012 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.

use std::gc::GC;

fn main() {
let f;
let g;
g = f;
f = box(GC) g; //~ ERROR cyclic type of infinite size
}
4 changes: 2 additions & 2 deletions src/test/compile-fail/occurs-check.rs
Expand Up @@ -12,6 +12,6 @@
use std::gc::GC;

fn main() {
let f; //~ ERROR cyclic type of infinite size
f = box(GC) f;
let f;
f = box(GC) f; //~ ERROR cyclic type of infinite size
}

5 comments on commit c4d56b7

@bors
Copy link
Contributor

@bors bors commented on c4d56b7 Sep 11, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from pcwalton
at nikomatsakis@c4d56b7

@bors
Copy link
Contributor

@bors bors commented on c4d56b7 Sep 11, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging nikomatsakis/rust/occurs-check = c4d56b7 into auto

@bors
Copy link
Contributor

@bors bors commented on c4d56b7 Sep 11, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nikomatsakis/rust/occurs-check = c4d56b7 merged ok, testing candidate = c8b0d66

@bors
Copy link
Contributor

@bors bors commented on c4d56b7 Sep 11, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on c4d56b7 Sep 11, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = c8b0d66

Please sign in to comment.