Skip to content

Commit

Permalink
Auto merge of #22512 - nikomatsakis:issue-20300-where-clause-not-boun…
Browse files Browse the repository at this point in the history
…ds, r=nikomatsakis

This is a fix for #20300 though as a side-sweep it fixes a number of stack overflows because it integrates cycle detection into the conversion process. I didn't go through and retest everything.

The tricky part here is that in some cases we have to go find the information we need from the AST -- we can't use the converted form of the where-clauses because we often have to handle something like `T::Item` *while converting the where-clauses themselves*. Since this is also not a fixed-point process we can't just try and keep trying to find the best order. So instead I modified the `AstConv` interface to allow you to request the bounds for a type parameter; we'll then do a secondary scan of the where-clauses to figure out what we need. This may create a cycle in some cases, but we have code to catch that.

Another approach that is NOT taken by this PR would be to "convert" `T::Item` into a form that does not specify what trait it's using. This then kind of defers the problem of picking the trait till later. That might be a good idea, but it would make normalization and everything else much harder, so I'm holding off on that (and hoping to find a better way for handling things like `i32::T`).

This PR also removes "most of" the `bounds` struct from `TypeParameterDef`. Still a little ways to go before `ParamBounds` can be removed entirely -- it's used for supertraits, for example (though those really ought to be converted, I think, to a call to `get_type_parameter_bounds` on `Self` from within the trait definition).

cc @jroesch 

Fixes #20300
  • Loading branch information
bors committed Feb 25, 2015
2 parents ad04cce + 1ef3598 commit 880fb89
Show file tree
Hide file tree
Showing 18 changed files with 1,066 additions and 417 deletions.
27 changes: 20 additions & 7 deletions src/librustc/metadata/tydecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,6 @@ fn parse_type_param_def_<'a, 'tcx, F>(st: &mut PState<'a, 'tcx>, conv: &mut F)
assert_eq!(next(st), '|');
let index = parse_u32(st);
assert_eq!(next(st), '|');
let bounds = parse_bounds_(st, conv);
let default = parse_opt(st, |st| parse_ty_(st, conv));
let object_lifetime_default = parse_object_lifetime_default(st, conv);

Expand All @@ -831,7 +830,6 @@ fn parse_type_param_def_<'a, 'tcx, F>(st: &mut PState<'a, 'tcx>, conv: &mut F)
def_id: def_id,
space: space,
index: index,
bounds: bounds,
default: default,
object_lifetime_default: object_lifetime_default,
}
Expand Down Expand Up @@ -924,18 +922,18 @@ fn parse_bounds_<'a, 'tcx, F>(st: &mut PState<'a, 'tcx>, conv: &mut F)
{
let builtin_bounds = parse_builtin_bounds_(st, conv);

let region_bounds = parse_region_bounds_(st, conv);

let mut param_bounds = ty::ParamBounds {
region_bounds: Vec::new(),
region_bounds: region_bounds,
builtin_bounds: builtin_bounds,
trait_bounds: Vec::new(),
projection_bounds: Vec::new(),
};


loop {
match next(st) {
'R' => {
param_bounds.region_bounds.push(
parse_region_(st, conv));
}
'I' => {
param_bounds.trait_bounds.push(
ty::Binder(parse_trait_ref_(st, conv)));
Expand All @@ -953,3 +951,18 @@ fn parse_bounds_<'a, 'tcx, F>(st: &mut PState<'a, 'tcx>, conv: &mut F)
}
}
}

fn parse_region_bounds_<'a, 'tcx, F>(st: &mut PState<'a, 'tcx>, conv: &mut F)
-> Vec<ty::Region> where
F: FnMut(DefIdSource, ast::DefId) -> ast::DefId,
{
let mut region_bounds = Vec::new();
loop {
match next(st) {
'R' => { region_bounds.push(parse_region_(st, conv)); }
'.' => { return region_bounds; }
c => { panic!("parse_bounds: bad bounds ('{}')", c); }
}
}
}

17 changes: 12 additions & 5 deletions src/librustc/metadata/tyencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,7 @@ pub fn enc_bounds<'a, 'tcx>(w: &mut SeekableMemWriter, cx: &ctxt<'a, 'tcx>,
bs: &ty::ParamBounds<'tcx>) {
enc_builtin_bounds(w, cx, &bs.builtin_bounds);

for &r in &bs.region_bounds {
mywrite!(w, "R");
enc_region(w, cx, r);
}
enc_region_bounds(w, cx, &bs.region_bounds);

for tp in &bs.trait_bounds {
mywrite!(w, "I");
Expand All @@ -404,12 +401,22 @@ pub fn enc_bounds<'a, 'tcx>(w: &mut SeekableMemWriter, cx: &ctxt<'a, 'tcx>,
mywrite!(w, ".");
}

pub fn enc_region_bounds<'a, 'tcx>(w: &mut SeekableMemWriter,
cx: &ctxt<'a, 'tcx>,
rs: &[ty::Region]) {
for &r in rs {
mywrite!(w, "R");
enc_region(w, cx, r);
}

mywrite!(w, ".");
}

pub fn enc_type_param_def<'a, 'tcx>(w: &mut SeekableMemWriter, cx: &ctxt<'a, 'tcx>,
v: &ty::TypeParameterDef<'tcx>) {
mywrite!(w, "{}:{}|{}|{}|",
token::get_name(v.name), (cx.ds)(v.def_id),
v.space.to_uint(), v.index);
enc_bounds(w, cx, &v.bounds);
enc_opt(w, v.default, |w, t| enc_ty(w, cx, t));
enc_object_lifetime_default(w, cx, v.object_lifetime_default);
}
Expand Down
17 changes: 15 additions & 2 deletions src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use middle::region;
use middle::resolve_lifetime;
use middle::infer;
use middle::stability;
use middle::subst::{self, Subst, Substs, VecPerParamSpace};
use middle::subst::{self, ParamSpace, Subst, Substs, VecPerParamSpace};
use middle::traits;
use middle::ty;
use middle::ty_fold::{self, TypeFoldable, TypeFolder};
Expand Down Expand Up @@ -1750,7 +1750,6 @@ pub struct TypeParameterDef<'tcx> {
pub def_id: ast::DefId,
pub space: subst::ParamSpace,
pub index: u32,
pub bounds: ParamBounds<'tcx>,
pub default: Option<Ty<'tcx>>,
pub object_lifetime_default: Option<ObjectLifetimeDefault>,
}
Expand Down Expand Up @@ -2546,6 +2545,13 @@ impl<'tcx> ctxt<'tcx> {
{
self.closure_tys.borrow()[def_id].subst(self, substs)
}

pub fn type_parameter_def(&self,
node_id: ast::NodeId)
-> TypeParameterDef<'tcx>
{
self.ty_param_defs.borrow()[node_id].clone()
}
}

// Interns a type/name combination, stores the resulting box in cx.interner,
Expand Down Expand Up @@ -2996,6 +3002,13 @@ impl<'tcx> TyS<'tcx> {
_ => None,
}
}

pub fn is_param(&self, space: ParamSpace, index: u32) -> bool {
match self.sty {
ty::ty_param(ref data) => data.space == space && data.idx == index,
_ => false,
}
}
}

pub fn walk_ty<'tcx, F>(ty_root: Ty<'tcx>, mut f: F)
Expand Down
1 change: 0 additions & 1 deletion src/librustc/middle/ty_fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,6 @@ impl<'tcx> TypeFoldable<'tcx> for ty::TypeParameterDef<'tcx> {
def_id: self.def_id,
space: self.space,
index: self.index,
bounds: self.bounds.fold_with(folder),
default: self.default.fold_with(folder),
object_lifetime_default: self.object_lifetime_default.fold_with(folder),
}
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ pub struct Options {
pub test: bool,
pub parse_only: bool,
pub no_trans: bool,
pub treat_err_as_bug: bool,
pub no_analysis: bool,
pub debugging_opts: DebuggingOptions,
/// Whether to write dependency files. It's (enabled, optional filename).
Expand Down Expand Up @@ -223,6 +224,7 @@ pub fn basic_options() -> Options {
test: false,
parse_only: false,
no_trans: false,
treat_err_as_bug: false,
no_analysis: false,
debugging_opts: basic_debugging_options(),
write_dependency_info: (false, None),
Expand Down Expand Up @@ -573,6 +575,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"Parse only; do not compile, assemble, or link"),
no_trans: bool = (false, parse_bool,
"Run all passes except translation; no output"),
treat_err_as_bug: bool = (false, parse_bool,
"Treat all errors that occur as bugs"),
no_analysis: bool = (false, parse_bool,
"Parse and expand the source, but run no analysis"),
extra_plugins: Vec<String> = (Vec::new(), parse_list,
Expand Down Expand Up @@ -843,6 +847,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {

let parse_only = debugging_opts.parse_only;
let no_trans = debugging_opts.no_trans;
let treat_err_as_bug = debugging_opts.treat_err_as_bug;
let no_analysis = debugging_opts.no_analysis;

if debugging_opts.debug_llvm {
Expand Down Expand Up @@ -1030,6 +1035,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
test: test,
parse_only: parse_only,
no_trans: no_trans,
treat_err_as_bug: treat_err_as_bug,
no_analysis: no_analysis,
debugging_opts: debugging_opts,
write_dependency_info: write_dependency_info,
Expand Down
9 changes: 9 additions & 0 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,27 @@ impl Session {
self.diagnostic().handler().fatal(msg)
}
pub fn span_err(&self, sp: Span, msg: &str) {
if self.opts.treat_err_as_bug {
self.span_bug(sp, msg);
}
match split_msg_into_multilines(msg) {
Some(msg) => self.diagnostic().span_err(sp, &msg[..]),
None => self.diagnostic().span_err(sp, msg)
}
}
pub fn span_err_with_code(&self, sp: Span, msg: &str, code: &str) {
if self.opts.treat_err_as_bug {
self.span_bug(sp, msg);
}
match split_msg_into_multilines(msg) {
Some(msg) => self.diagnostic().span_err_with_code(sp, &msg[..], code),
None => self.diagnostic().span_err_with_code(sp, msg, code)
}
}
pub fn err(&self, msg: &str) {
if self.opts.treat_err_as_bug {
self.bug(msg);
}
self.diagnostic().handler().err(msg)
}
pub fn err_count(&self) -> uint {
Expand Down
52 changes: 33 additions & 19 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,14 @@ use syntax::print::pprust;
pub trait AstConv<'tcx> {
fn tcx<'a>(&'a self) -> &'a ty::ctxt<'tcx>;

fn get_item_type_scheme(&self, id: ast::DefId) -> ty::TypeScheme<'tcx>;
fn get_item_type_scheme(&self, span: Span, id: ast::DefId)
-> Result<ty::TypeScheme<'tcx>, ErrorReported>;

fn get_trait_def(&self, id: ast::DefId) -> Rc<ty::TraitDef<'tcx>>;
fn get_trait_def(&self, span: Span, id: ast::DefId)
-> Result<Rc<ty::TraitDef<'tcx>>, ErrorReported>;

fn get_type_parameter_bounds(&self, span: Span, def_id: ast::NodeId)
-> Result<Vec<ty::PolyTraitRef<'tcx>>, ErrorReported>;

/// Return an (optional) substitution to convert bound type parameters that
/// are in scope into free ones. This function should only return Some
Expand Down Expand Up @@ -683,7 +688,14 @@ fn ast_path_to_trait_ref<'a,'tcx>(
-> Rc<ty::TraitRef<'tcx>>
{
debug!("ast_path_to_trait_ref {:?}", trait_segment);
let trait_def = this.get_trait_def(trait_def_id);
let trait_def = match this.get_trait_def(span, trait_def_id) {
Ok(trait_def) => trait_def,
Err(ErrorReported) => {
// No convenient way to recover from a cycle here. Just bail. Sorry!
this.tcx().sess.abort_if_errors();
this.tcx().sess.bug("ErrorReported returned, but no errors reports?")
}
};

let (regions, types, assoc_bindings) = match trait_segment.parameters {
ast::AngleBracketedParameters(ref data) => {
Expand Down Expand Up @@ -860,10 +872,15 @@ fn ast_path_to_ty<'tcx>(
item_segment: &ast::PathSegment)
-> Ty<'tcx>
{
let ty::TypeScheme {
generics,
ty: decl_ty
} = this.get_item_type_scheme(did);
let tcx = this.tcx();
let (generics, decl_ty) = match this.get_item_type_scheme(span, did) {
Ok(ty::TypeScheme { generics, ty: decl_ty }) => {
(generics, decl_ty)
}
Err(ErrorReported) => {
return tcx.types.err;
}
};

let substs = ast_path_substs_for_ty(this, rscope,
span, param_mode,
Expand Down Expand Up @@ -1001,20 +1018,17 @@ fn associated_path_def_to_ty<'tcx>(this: &AstConv<'tcx>,
return (tcx.types.err, ty_path_def);
};

let mut suitable_bounds: Vec<_>;
let ty_param_name: ast::Name;
{ // contain scope of refcell:
let ty_param_defs = tcx.ty_param_defs.borrow();
let ty_param_def = &ty_param_defs[ty_param_node_id];
ty_param_name = ty_param_def.name;
let ty_param_name = tcx.ty_param_defs.borrow()[ty_param_node_id].name;

// FIXME(#20300) -- search where clauses, not bounds
let bounds =
this.get_type_parameter_bounds(span, ty_param_node_id)
.unwrap_or(Vec::new());

// FIXME(#20300) -- search where clauses, not bounds
suitable_bounds =
traits::transitive_bounds(tcx, &ty_param_def.bounds.trait_bounds)
.filter(|b| trait_defines_associated_type_named(this, b.def_id(), assoc_name))
.collect();
}
let mut suitable_bounds: Vec<_> =
traits::transitive_bounds(tcx, &bounds)
.filter(|b| trait_defines_associated_type_named(this, b.def_id(), assoc_name))
.collect();

if suitable_bounds.len() == 0 {
span_err!(tcx.sess, span, E0220,
Expand Down
44 changes: 37 additions & 7 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ use middle::subst::{self, Subst, Substs, VecPerParamSpace, ParamSpace, TypeSpace
use middle::traits;
use middle::ty::{FnSig, GenericPredicates, VariantInfo, TypeScheme};
use middle::ty::{Disr, ParamTy, ParameterEnvironment};
use middle::ty::{self, HasProjectionTypes, RegionEscape, Ty};
use middle::ty::{self, HasProjectionTypes, RegionEscape, ToPolyTraitRef, Ty};
use middle::ty::liberate_late_bound_regions;
use middle::ty::{MethodCall, MethodCallee, MethodMap, ObjectCastMap};
use middle::ty_fold::{TypeFolder, TypeFoldable};
Expand All @@ -106,7 +106,7 @@ use session::Session;
use {CrateCtxt, lookup_full_def, require_same_types};
use TypeAndSubsts;
use lint;
use util::common::{block_query, indenter, loop_query};
use util::common::{block_query, ErrorReported, indenter, loop_query};
use util::ppaux::{self, Repr};
use util::nodemap::{DefIdMap, FnvHashMap, NodeMap};
use util::lev_distance::lev_distance;
Expand Down Expand Up @@ -1206,18 +1206,48 @@ fn check_cast<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>,
impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> {
fn tcx(&self) -> &ty::ctxt<'tcx> { self.ccx.tcx }

fn get_item_type_scheme(&self, id: ast::DefId) -> ty::TypeScheme<'tcx> {
ty::lookup_item_type(self.tcx(), id)
fn get_item_type_scheme(&self, _: Span, id: ast::DefId)
-> Result<ty::TypeScheme<'tcx>, ErrorReported>
{
Ok(ty::lookup_item_type(self.tcx(), id))
}

fn get_trait_def(&self, id: ast::DefId) -> Rc<ty::TraitDef<'tcx>> {
ty::lookup_trait_def(self.tcx(), id)
fn get_trait_def(&self, _: Span, id: ast::DefId)
-> Result<Rc<ty::TraitDef<'tcx>>, ErrorReported>
{
Ok(ty::lookup_trait_def(self.tcx(), id))
}

fn get_free_substs(&self) -> Option<&Substs<'tcx>> {
Some(&self.inh.param_env.free_substs)
}

fn get_type_parameter_bounds(&self,
_: Span,
node_id: ast::NodeId)
-> Result<Vec<ty::PolyTraitRef<'tcx>>, ErrorReported>
{
let def = self.tcx().type_parameter_def(node_id);
let r = self.inh.param_env.caller_bounds
.iter()
.filter_map(|predicate| {
match *predicate {
ty::Predicate::Trait(ref data) => {
if data.0.self_ty().is_param(def.space, def.index) {
Some(data.to_poly_trait_ref())
} else {
None
}
}
_ => {
None
}
}
})
.collect();
Ok(r)
}

fn ty_infer(&self, _span: Span) -> Ty<'tcx> {
self.infcx().next_ty_var()
}
Expand Down Expand Up @@ -3607,7 +3637,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>,
}
} else {
tcx.sess.span_bug(expr.span,
&format!("unbound path {}", expr.repr(tcx))[])
&format!("unbound path {}", expr.repr(tcx)))
};

let mut def = path_res.base_def;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/coherence/overlap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OverlapChecker<'cx, 'tcx> {
None => {
self.tcx.sess.bug(
&format!("no default implementation recorded for `{:?}`",
item)[]);
item));
}
}
}
Expand Down
Loading

0 comments on commit 880fb89

Please sign in to comment.