Skip to content

Commit 4726bb4

Browse files
committed
Correct regression in type-inference caused by failing to reconfirm that
the object trait matches the required trait during trait selection. The existing code was checking that the object trait WOULD match (in a probe), but never executing the match outside of a probe. This corrects various regressions observed in the wild, including issue #26952. Fixes #26952.
1 parent 2e5b165 commit 4726bb4

File tree

4 files changed

+80
-31
lines changed

4 files changed

+80
-31
lines changed

src/librustc/middle/infer/mod.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -698,8 +698,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
698698
}
699699
}
700700

701-
fn rollback_to(&self, snapshot: CombinedSnapshot) {
702-
debug!("rollback!");
701+
fn rollback_to(&self, cause: &str, snapshot: CombinedSnapshot) {
702+
debug!("rollback_to(cause={})", cause);
703703
let CombinedSnapshot { type_snapshot,
704704
int_snapshot,
705705
float_snapshot,
@@ -759,7 +759,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
759759
debug!("commit_if_ok() -- r.is_ok() = {}", r.is_ok());
760760
match r {
761761
Ok(_) => { self.commit_from(snapshot); }
762-
Err(_) => { self.rollback_to(snapshot); }
762+
Err(_) => { self.rollback_to("commit_if_ok -- error", snapshot); }
763763
}
764764
r
765765
}
@@ -778,6 +778,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
778778

779779
let r = self.commit_if_ok(|_| f());
780780

781+
debug!("commit_regions_if_ok: rolling back everything but regions");
782+
781783
// Roll back any non-region bindings - they should be resolved
782784
// inside `f`, with, e.g. `resolve_type_vars_if_possible`.
783785
self.type_variables
@@ -804,7 +806,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
804806
debug!("probe()");
805807
let snapshot = self.start_snapshot();
806808
let r = f(&snapshot);
807-
self.rollback_to(snapshot);
809+
self.rollback_to("probe", snapshot);
808810
r
809811
}
810812

src/librustc/middle/traits/select.rs

+40-26
Original file line numberDiff line numberDiff line change
@@ -1367,11 +1367,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
13671367
// correct trait, but also the correct type parameters.
13681368
// For example, we may be trying to upcast `Foo` to `Bar<i32>`,
13691369
// but `Foo` is declared as `trait Foo : Bar<u32>`.
1370-
let upcast_trait_refs = util::supertraits(self.tcx(), poly_trait_ref)
1371-
.filter(|upcast_trait_ref| self.infcx.probe(|_| {
1372-
let upcast_trait_ref = upcast_trait_ref.clone();
1373-
self.match_poly_trait_ref(obligation, upcast_trait_ref).is_ok()
1374-
})).count();
1370+
let upcast_trait_refs =
1371+
util::supertraits(self.tcx(), poly_trait_ref)
1372+
.filter(|upcast_trait_ref| {
1373+
self.infcx.probe(|_| {
1374+
let upcast_trait_ref = upcast_trait_ref.clone();
1375+
self.match_poly_trait_ref(obligation, upcast_trait_ref).is_ok()
1376+
})
1377+
})
1378+
.count();
13751379

13761380
if upcast_trait_refs > 1 {
13771381
// can be upcast in many ways; need more type information
@@ -1643,9 +1647,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
16431647
let principal =
16441648
data.principal_trait_ref_with_self_ty(self.tcx(),
16451649
self.tcx().types.err);
1646-
let desired_def_id = obligation.predicate.def_id();
1650+
let copy_def_id = obligation.predicate.def_id();
16471651
for tr in util::supertraits(self.tcx(), principal) {
1648-
if tr.def_id() == desired_def_id {
1652+
if tr.def_id() == copy_def_id {
16491653
return ok_if(Vec::new())
16501654
}
16511655
}
@@ -2310,31 +2314,41 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
23102314
}
23112315
};
23122316

2313-
// Upcast the object type to the obligation type. There must
2314-
// be exactly one applicable trait-reference; if this were not
2315-
// the case, we would have reported an ambiguity error rather
2316-
// than successfully selecting one of the candidates.
2317-
let mut upcast_trait_refs = util::supertraits(self.tcx(), poly_trait_ref)
2318-
.map(|upcast_trait_ref| {
2319-
(upcast_trait_ref.clone(), self.infcx.probe(|_| {
2320-
self.match_poly_trait_ref(obligation, upcast_trait_ref)
2321-
}).is_ok())
2322-
});
23232317
let mut upcast_trait_ref = None;
2324-
let mut vtable_base = 0;
2318+
let vtable_base;
2319+
2320+
{
2321+
// We want to find the first supertrait in the list of
2322+
// supertraits that we can unify with, and do that
2323+
// unification. We know that there is exactly one in the list
2324+
// where we can unify because otherwise select would have
2325+
// reported an ambiguity. (When we do find a match, also
2326+
// record it for later.)
2327+
let nonmatching =
2328+
util::supertraits(self.tcx(), poly_trait_ref)
2329+
.take_while(|&t| {
2330+
match
2331+
self.infcx.commit_if_ok(
2332+
|_| self.match_poly_trait_ref(obligation, t))
2333+
{
2334+
Ok(_) => { upcast_trait_ref = Some(t); false }
2335+
Err(_) => { true }
2336+
}
2337+
});
2338+
2339+
// Additionally, for each of the nonmatching predicates that
2340+
// we pass over, we sum up the set of number of vtable
2341+
// entries, so that we can compute the offset for the selected
2342+
// trait.
2343+
vtable_base =
2344+
nonmatching.map(|t| util::count_own_vtable_entries(self.tcx(), t))
2345+
.sum();
23252346

2326-
while let Some((supertrait, matches)) = upcast_trait_refs.next() {
2327-
if matches {
2328-
upcast_trait_ref = Some(supertrait);
2329-
break;
2330-
}
2331-
vtable_base += util::count_own_vtable_entries(self.tcx(), supertrait);
23322347
}
2333-
assert!(upcast_trait_refs.all(|(_, matches)| !matches));
23342348

23352349
VtableObjectData {
23362350
upcast_trait_ref: upcast_trait_ref.unwrap(),
2337-
vtable_base: vtable_base
2351+
vtable_base: vtable_base,
23382352
}
23392353
}
23402354

src/librustc/middle/ty.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1898,7 +1898,7 @@ impl<'tcx> PolyTraitRef<'tcx> {
18981898
/// erase, or otherwise "discharge" these bound regions, we change the
18991899
/// type from `Binder<T>` to just `T` (see
19001900
/// e.g. `liberate_late_bound_regions`).
1901-
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
1901+
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
19021902
pub struct Binder<T>(pub T);
19031903

19041904
impl<T> Binder<T> {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright 2012 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 that when we match a trait reference like `Foo<A>: Foo<_#0t>`,
12+
// we unify with `_#0t` with `A`. In this code, if we failed to do
13+
// that, then you get an unconstrained type-variable in `call`.
14+
//
15+
// Also serves as a regression test for issue #26952, though the test
16+
// was derived from another reported regression with the same cause.
17+
18+
use std::marker::PhantomData;
19+
20+
trait Trait<A> { fn foo(&self); }
21+
22+
struct Type<A> { a: PhantomData<A> }
23+
24+
fn as_trait<A>(t: &Type<A>) -> &Trait<A> { loop { } }
25+
26+
fn want<A,T:Trait<A>+?Sized>(t: &T) { }
27+
28+
fn call<A>(p: Type<A>) {
29+
let q = as_trait(&p);
30+
want(q); // parameter A to `want` *would* be unconstrained
31+
}
32+
33+
fn main() { }

0 commit comments

Comments
 (0)