Skip to content

Commit 6a749c7

Browse files
committed
fix corner case around top of stack
When deciding on a coinductive match, we were examining the new obligation and the backtrace, but not the *current* obligation that goes in between the two. Refactoring the code to just have the cycle given as input also made things a lot simpler.
1 parent 944723b commit 6a749c7

File tree

2 files changed

+64
-39
lines changed

2 files changed

+64
-39
lines changed

src/librustc/traits/fulfill.rs

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -407,17 +407,17 @@ fn process_child_obligations<'a,'tcx>(
407407
// ~~~ (*) see above
408408
debug!("process_child_obligations: cycle index = {}", index);
409409

410-
if coinductive_match(selcx, &obligation, &backtrace) {
410+
let backtrace = backtrace.clone();
411+
let cycle: Vec<_> =
412+
iter::once(&obligation)
413+
.chain(Some(pending_obligation))
414+
.chain(backtrace.take(index + 1).map(|p| &p.obligation))
415+
.cloned()
416+
.collect();
417+
if coinductive_match(selcx, &cycle) {
411418
debug!("process_child_obligations: coinductive match");
412419
None
413420
} else {
414-
let backtrace = backtrace.clone();
415-
let cycle: Vec<_> =
416-
iter::once(&obligation)
417-
.chain(Some(pending_obligation))
418-
.chain(backtrace.take(index + 1).map(|p| &p.obligation))
419-
.cloned()
420-
.collect();
421421
report_overflow_error_cycle(selcx.infcx(), &cycle);
422422
}
423423
} else {
@@ -663,47 +663,40 @@ fn process_predicate1<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>,
663663

664664
/// For defaulted traits, we use a co-inductive strategy to solve, so
665665
/// that recursion is ok. This routine returns true if the top of the
666-
/// stack (`top_obligation` and `top_data`):
666+
/// stack (`cycle[0]`):
667667
/// - is a defaulted trait, and
668668
/// - it also appears in the backtrace at some position `X`; and,
669669
/// - all the predicates at positions `X..` between `X` an the top are
670670
/// also defaulted traits.
671671
fn coinductive_match<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>,
672-
top_obligation: &PredicateObligation<'tcx>,
673-
backtrace: &Backtrace<PendingPredicateObligation<'tcx>>)
672+
cycle: &[PredicateObligation<'tcx>])
674673
-> bool
675674
{
676-
// only trait predicates can be coinductive matches
677-
let top_data = match top_obligation.predicate {
678-
ty::Predicate::Trait(ref data) => data,
679-
_ => return false
680-
};
681-
682-
if selcx.tcx().trait_has_default_impl(top_data.def_id()) {
683-
debug!("coinductive_match: top_data={:?}", top_data);
684-
for bt_obligation in backtrace.clone() {
685-
debug!("coinductive_match: bt_obligation={:?}", bt_obligation);
686-
687-
// *Everything* in the backtrace must be a defaulted trait.
688-
match bt_obligation.obligation.predicate {
689-
ty::Predicate::Trait(ref data) => {
690-
if !selcx.tcx().trait_has_default_impl(data.def_id()) {
691-
debug!("coinductive_match: trait does not have default impl");
692-
break;
693-
}
694-
}
695-
_ => { break; }
696-
}
675+
let len = cycle.len();
697676

698-
// And we must find a recursive match.
699-
if bt_obligation.obligation.predicate == top_obligation.predicate {
700-
debug!("coinductive_match: found a match in the backtrace");
701-
return true;
702-
}
677+
assert_eq!(cycle[0].predicate, cycle[len - 1].predicate);
678+
679+
cycle[0..len-1]
680+
.iter()
681+
.all(|bt_obligation| {
682+
let result = coinductive_obligation(selcx, bt_obligation);
683+
debug!("coinductive_match: bt_obligation={:?} coinductive={}",
684+
bt_obligation, result);
685+
result
686+
})
687+
}
688+
689+
fn coinductive_obligation<'a, 'tcx>(selcx: &SelectionContext<'a, 'tcx>,
690+
obligation: &PredicateObligation<'tcx>)
691+
-> bool {
692+
match obligation.predicate {
693+
ty::Predicate::Trait(ref data) => {
694+
selcx.tcx().trait_has_default_impl(data.def_id())
695+
}
696+
_ => {
697+
false
703698
}
704699
}
705-
706-
false
707700
}
708701

709702
fn register_region_obligation<'tcx>(t_a: Ty<'tcx>,
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Test for a potential corner case in current impl where you have an
12+
// auto trait (Magic1) that depends on a normal trait (Magic2) which
13+
// in turn depends on the auto trait (Magic1). This was incorrectly
14+
// being considered coinductive, but because of the normal trait
15+
// interfering, it should not be.
16+
17+
#![feature(optin_builtin_traits)]
18+
19+
trait Magic1: Magic2 { }
20+
impl Magic1 for .. {}
21+
22+
trait Magic2 { }
23+
impl<T: Magic1> Magic2 for T { }
24+
25+
fn is_magic1<T: Magic1>() { }
26+
27+
#[derive(Debug)]
28+
struct NoClone;
29+
30+
fn main() {
31+
is_magic1::<NoClone>();
32+
}

0 commit comments

Comments
 (0)