Skip to content

Commit 0d9e473

Browse files
nikomatsakisnikomatsakis
committed
Comprehence cycle detection in collect. In some cases, the cycles we
report are not *necessary* cycles, but we'll work on refactoring them over time. This overlaps with the cycle detection that astconv already does: I left that code in because it gives a more targeted error message, though perhaps less helpful in that it doesn't give the full details of the cycle.
1 parent 15ef2c2 commit 0d9e473

File tree

8 files changed

+282
-68
lines changed

8 files changed

+282
-68
lines changed

‎src/librustc/middle/ty.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2546,6 +2546,13 @@ impl<'tcx> ctxt<'tcx> {
25462546
{
25472547
self.closure_tys.borrow()[def_id].subst(self, substs)
25482548
}
2549+
2550+
pub fn type_parameter_def(&self,
2551+
node_id: ast::NodeId)
2552+
-> TypeParameterDef<'tcx>
2553+
{
2554+
self.ty_param_defs.borrow()[node_id].clone()
2555+
}
25492556
}
25502557

25512558
// Interns a type/name combination, stores the resulting box in cx.interner,

‎src/librustc_typeck/astconv.rs

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ use middle::const_eval;
5353
use middle::def;
5454
use middle::resolve_lifetime as rl;
5555
use middle::privacy::{AllPublic, LastMod};
56-
use middle::subst::{FnSpace, ParamSpace, TypeSpace, SelfSpace, Subst, Substs};
56+
use middle::subst::{FnSpace, TypeSpace, SelfSpace, Subst, Substs};
5757
use middle::traits;
5858
use middle::ty::{self, RegionEscape, ToPolyTraitRef, Ty};
5959
use rscope::{self, UnelidableRscope, RegionScope, ElidableRscope,
@@ -73,11 +73,14 @@ use syntax::print::pprust;
7373
pub trait AstConv<'tcx> {
7474
fn tcx<'a>(&'a self) -> &'a ty::ctxt<'tcx>;
7575

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

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

80-
fn get_type_parameter_bounds(&self, space: ParamSpace, index: u32) -> Vec<ty::PolyTraitRef<'tcx>>;
82+
fn get_type_parameter_bounds(&self, span: Span, def_id: ast::NodeId)
83+
-> Result<Vec<ty::PolyTraitRef<'tcx>>, ErrorReported>;
8184

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

690700
let (regions, types, assoc_bindings) = match trait_segment.parameters {
691701
ast::AngleBracketedParameters(ref data) => {
@@ -862,14 +872,17 @@ fn ast_path_to_ty<'tcx>(
862872
item_segment: &ast::PathSegment)
863873
-> Ty<'tcx>
864874
{
865-
let ty::TypeScheme {
866-
generics,
867-
ty: decl_ty
868-
} = this.get_item_type_scheme(did);
869-
870-
let substs = ast_path_substs_for_ty(this, rscope,
871-
span, param_mode,
872-
&generics, item_segment);
875+
let tcx = this.tcx();
876+
let substs = match this.get_item_type_scheme(path.span, did) {
877+
Ok(ty::TypeScheme { generics, ty: decl_ty }) => {
878+
ast_path_substs_for_ty(this, rscope,
879+
span, param_mode,
880+
&generics, item_segment)
881+
}
882+
Err(ErrorReported) => {
883+
return TypeAndSubsts { substs: Substs::empty(), ty: tcx.types.err };
884+
}
885+
};
873886

874887
// FIXME(#12938): This is a hack until we have full support for DST.
875888
if Some(did) == this.tcx().lang_items.owned_box() {
@@ -1003,22 +1016,17 @@ fn associated_path_def_to_ty<'tcx>(this: &AstConv<'tcx>,
10031016
return (tcx.types.err, ty_path_def);
10041017
};
10051018

1006-
let mut suitable_bounds: Vec<_>;
1007-
let ty_param_name: ast::Name;
1008-
{ // contain scope of refcell:
1009-
let ty_param_defs = tcx.ty_param_defs.borrow();
1010-
let ty_param_def = &ty_param_defs[ty_param_node_id];
1011-
ty_param_name = ty_param_def.name;
1012-
1013-
1014-
// FIXME(#20300) -- search where clauses, not bounds
1015-
suitable_bounds =
1016-
traits::transitive_bounds(tcx,
1017-
&this.get_type_parameter_bounds(ty_param_def.space,
1018-
ty_param_def.index))
1019-
.filter(|b| trait_defines_associated_type_named(this, b.def_id(), assoc_name))
1020-
.collect();
1021-
}
1019+
let ty_param_name = tcx.ty_param_defs.borrow()[ty_param_node_id].name;
1020+
1021+
// FIXME(#20300) -- search where clauses, not bounds
1022+
let bounds =
1023+
this.get_type_parameter_bounds(ast_ty.span, ty_param_ndoe_id)
1024+
.unwrap_or(Vec::new());
1025+
1026+
let mut suitable_bounds: Vec<_> =
1027+
traits::transitive_bounds(tcx, &bounds)
1028+
.filter(|b| trait_defines_associated_type_named(this, b.def_id(), assoc_name))
1029+
.collect();
10221030

10231031
if suitable_bounds.len() == 0 {
10241032
span_err!(tcx.sess, span, E0220,

‎src/librustc_typeck/check/mod.rs

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ use session::Session;
106106
use {CrateCtxt, lookup_full_def, require_same_types};
107107
use TypeAndSubsts;
108108
use lint;
109-
use util::common::{block_query, indenter, loop_query};
109+
use util::common::{block_query, ErrorReported, indenter, loop_query};
110110
use util::ppaux::{self, Repr};
111111
use util::nodemap::{DefIdMap, FnvHashMap, NodeMap};
112112
use util::lev_distance::lev_distance;
@@ -1206,40 +1206,46 @@ fn check_cast<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>,
12061206
impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> {
12071207
fn tcx(&self) -> &ty::ctxt<'tcx> { self.ccx.tcx }
12081208

1209-
fn get_item_type_scheme(&self, id: ast::DefId) -> ty::TypeScheme<'tcx> {
1210-
ty::lookup_item_type(self.tcx(), id)
1209+
fn get_item_type_scheme(&self, _: Span, id: ast::DefId)
1210+
-> Result<ty::TypeScheme<'tcx>, ErrorReported>
1211+
{
1212+
Ok(ty::lookup_item_type(self.tcx(), id))
12111213
}
12121214

1213-
fn get_trait_def(&self, id: ast::DefId) -> Rc<ty::TraitDef<'tcx>> {
1214-
ty::lookup_trait_def(self.tcx(), id)
1215+
fn get_trait_def(&self, _: Span, id: ast::DefId)
1216+
-> Result<Rc<ty::TraitDef<'tcx>>, ErrorReported>
1217+
{
1218+
Ok(ty::lookup_trait_def(self.tcx(), id))
12151219
}
12161220

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

12211225
fn get_type_parameter_bounds(&self,
1222-
space: ParamSpace,
1223-
index: u32)
1224-
-> Vec<ty::PolyTraitRef<'tcx>>
1226+
_: Span,
1227+
node_id: ast::NodeId)
1228+
-> Result<Vec<ty::PolyTraitRef<'tcx>>, ErrorReported>
12251229
{
1226-
self.inh.param_env.caller_bounds
1227-
.iter()
1228-
.filter_map(|predicate| {
1229-
match *predicate {
1230-
ty::Predicate::Trait(ref data) => {
1231-
if data.0.self_ty().is_param(space, index) {
1232-
Some(data.to_poly_trait_ref())
1233-
} else {
1234-
None
1230+
let def = self.tcx().type_parameter_def(node_id);
1231+
let r = self.inh.param_env.caller_bounds
1232+
.iter()
1233+
.filter_map(|predicate| {
1234+
match *predicate {
1235+
ty::Predicate::Trait(ref data) => {
1236+
if data.0.self_ty().is_param(def.space, def.index) {
1237+
Some(data.to_poly_trait_ref())
1238+
} else {
1239+
None
1240+
}
1241+
}
1242+
_ => {
1243+
None
1244+
}
12351245
}
1236-
}
1237-
_ => {
1238-
None
1239-
}
1240-
}
1241-
})
1242-
.collect()
1246+
})
1247+
.collect();
1248+
Ok(r)
12431249
}
12441250

12451251
fn ty_infer(&self, _span: Span) -> Ty<'tcx> {

‎src/librustc_typeck/collect.rs

Lines changed: 127 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,13 @@ use middle::ty::{self, RegionEscape, Ty, TypeScheme};
9898
use middle::ty_fold::{self, TypeFolder, TypeFoldable};
9999
use middle::infer;
100100
use rscope::*;
101-
use util::common::memoized;
101+
use util::common::{ErrorReported, memoized};
102102
use util::nodemap::{FnvHashMap, FnvHashSet};
103103
use util::ppaux;
104104
use util::ppaux::{Repr,UserString};
105105
use write_ty_to_tcx;
106106

107+
use std::cell::RefCell;
107108
use std::collections::HashSet;
108109
use std::rc::Rc;
109110

@@ -121,7 +122,7 @@ use syntax::visit;
121122
// Main entry point
122123

123124
pub fn collect_item_types(tcx: &ty::ctxt) {
124-
let ccx = &CrateCtxt { tcx: tcx };
125+
let ccx = &CrateCtxt { tcx: tcx, stack: RefCell::new(Vec::new()) };
125126

126127
match ccx.tcx.lang_items.ty_desc() {
127128
Some(id) => { collect_intrinsic_type(ccx, id); }
@@ -143,13 +144,24 @@ pub fn collect_item_types(tcx: &ty::ctxt) {
143144

144145
struct CrateCtxt<'a,'tcx:'a> {
145146
tcx: &'a ty::ctxt<'tcx>,
147+
148+
// This stack is used to identify cycles in the user's source.
149+
// Note that these cycles can cross multiple items.
150+
stack: RefCell<Vec<AstConvRequest>>,
146151
}
147152

148153
struct ItemCtxt<'a,'tcx:'a> {
149154
ccx: &'a CrateCtxt<'a,'tcx>,
150155
generics: &'a ty::Generics<'tcx>,
151156
}
152157

158+
#[derive(Copy, PartialEq, Eq)]
159+
enum AstConvRequest {
160+
GetItemTypeScheme(ast::DefId),
161+
GetTraitDef(ast::DefId),
162+
GetTypeParameterBounds(ast::NodeId),
163+
}
164+
153165
///////////////////////////////////////////////////////////////////////////
154166
// Zeroth phase: collect types of intrinsics
155167

@@ -217,6 +229,94 @@ impl<'a,'tcx> CrateCtxt<'a,'tcx> {
217229
}
218230
}
219231
}
232+
233+
fn cycle_check<F,R>(&self,
234+
span: Span,
235+
request: AstConvRequest,
236+
code: F)
237+
-> Result<R,ErrorReported>
238+
where F: FnOnce() -> R
239+
{
240+
{
241+
let mut stack = self.stack.borrow_mut();
242+
match stack.iter().enumerate().rev().find(|&(_, r)| *r == request) {
243+
None => { }
244+
Some((i, _)) => {
245+
let cycle = &stack[i..];
246+
self.report_cycle(span, cycle);
247+
return Err(ErrorReported);
248+
}
249+
}
250+
stack.push(request);
251+
}
252+
253+
let result = code();
254+
255+
self.stack.borrow_mut().pop();
256+
Ok(result)
257+
}
258+
259+
fn report_cycle(&self,
260+
span: Span,
261+
cycle: &[AstConvRequest])
262+
{
263+
assert!(!cycle.is_empty());
264+
let tcx = self.tcx;
265+
266+
tcx.sess.span_err(
267+
span,
268+
&format!("unsupported cyclic reference between types/traits detected"));
269+
270+
match cycle[0] {
271+
AstConvRequest::GetItemTypeScheme(def_id) |
272+
AstConvRequest::GetTraitDef(def_id) => {
273+
tcx.sess.note(
274+
&format!("the cycle begins when processing `{}`...",
275+
ty::item_path_str(tcx, def_id)));
276+
}
277+
AstConvRequest::GetTypeParameterBounds(id) => {
278+
let def = tcx.type_parameter_def(id);
279+
tcx.sess.note(
280+
&format!("the cycle begins when computing the bounds \
281+
for type parameter `{}`...",
282+
def.name.user_string(tcx)));
283+
}
284+
}
285+
286+
for request in cycle[1..].iter() {
287+
match *request {
288+
AstConvRequest::GetItemTypeScheme(def_id) |
289+
AstConvRequest::GetTraitDef(def_id) => {
290+
tcx.sess.note(
291+
&format!("...which then requires processing `{}`...",
292+
ty::item_path_str(tcx, def_id)));
293+
}
294+
AstConvRequest::GetTypeParameterBounds(id) => {
295+
let def = tcx.type_parameter_def(id);
296+
tcx.sess.note(
297+
&format!("...which then requires computing the bounds \
298+
for type parameter `{}`...",
299+
def.name.user_string(tcx)));
300+
}
301+
}
302+
}
303+
304+
match cycle[0] {
305+
AstConvRequest::GetItemTypeScheme(def_id) |
306+
AstConvRequest::GetTraitDef(def_id) => {
307+
tcx.sess.note(
308+
&format!("...which then again requires processing `{}`, completing the cycle.",
309+
ty::item_path_str(tcx, def_id)));
310+
}
311+
AstConvRequest::GetTypeParameterBounds(id) => {
312+
let def = tcx.type_parameter_def(id);
313+
tcx.sess.note(
314+
&format!("...which then again requires computing the bounds \
315+
for type parameter `{}`, completing the cycle.",
316+
def.name.user_string(tcx)));
317+
}
318+
}
319+
}
220320
}
221321

222322
pub trait ToTy<'tcx> {
@@ -232,25 +332,37 @@ impl<'a,'tcx> ToTy<'tcx> for ItemCtxt<'a,'tcx> {
232332
impl<'a, 'tcx> AstConv<'tcx> for ItemCtxt<'a, 'tcx> {
233333
fn tcx(&self) -> &ty::ctxt<'tcx> { self.ccx.tcx }
234334

235-
fn get_item_type_scheme(&self, id: ast::DefId) -> ty::TypeScheme<'tcx> {
236-
type_scheme_of_def_id(self.ccx, id)
335+
fn get_item_type_scheme(&self, span: Span, id: ast::DefId)
336+
-> Result<ty::TypeScheme<'tcx>, ErrorReported>
337+
{
338+
self.ccx.cycle_check(span, AstConvRequest::GetItemTypeScheme(id), || {
339+
type_scheme_of_def_id(self.ccx, id)
340+
})
237341
}
238342

239-
fn get_trait_def(&self, id: ast::DefId) -> Rc<ty::TraitDef<'tcx>> {
240-
get_trait_def(self.ccx, id)
343+
fn get_trait_def(&self, span: Span, id: ast::DefId)
344+
-> Result<Rc<ty::TraitDef<'tcx>>, ErrorReported>
345+
{
346+
self.ccx.cycle_check(span, AstConvRequest::GetTraitDef(id), || {
347+
get_trait_def(self.ccx, id)
348+
})
241349
}
242350

243351
fn get_type_parameter_bounds(&self,
244-
param: subst::ParamSpace,
245-
index: u32)
246-
-> Vec<ty::PolyTraitRef<'tcx>>
352+
span: Span,
353+
node_id: ast::NodeId)
354+
-> Result<Vec<ty::PolyTraitRef<'tcx>>, ErrorReported>
247355
{
248-
// TODO out of range indices can occur when you have something
249-
// like fn foo<T:U::X,U>() { }
250-
match self.generics.types.opt_get(param, index as usize) {
251-
Some(def) => def.bounds.trait_bounds.clone(),
252-
None => Vec::new(),
253-
}
356+
self.ccx.cycle_check(span, AstConvRequest::GetTypeParameterBounds(node_id), || {
357+
let def = self.tcx().type_parameter_def(node_id);
358+
359+
// TODO out of range indices can occur when you have something
360+
// like fn foo<T:U::X,U>() { }
361+
match self.generics.types.opt_get(def.space, def.index as usize) {
362+
Some(def) => def.bounds.trait_bounds.clone(),
363+
None => Vec::new(),
364+
}
365+
})
254366
}
255367

256368
fn ty_infer(&self, span: Span) -> Ty<'tcx> {

0 commit comments

Comments
 (0)