Generalize the code so we can handle multiple supertraits.

nikomatsakis · nikomatsakis · commit cd50b4e0b14b · 2015-03-04T15:06:33.000-05:00
Fixes #10596. Fixes #22279.
diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs
@@ -280,7 +280,11 @@ pub struct VtableBuiltinData<N> {
 /// for the object type `Foo`.
 #[derive(PartialEq,Eq,Clone)]
 pub struct VtableObjectData<'tcx> {
+    /// the object type `Foo`.
     pub object_ty: Ty<'tcx>,
+
+    /// `Foo` upcast to the obligation trait. This will be some supertrait of `Foo`.
+    pub upcast_trait_ref: ty::PolyTraitRef<'tcx>,
 }
 
 /// Creates predicate obligations from the generic bounds.
diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs
@@ -1260,19 +1260,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                poly_trait_ref.repr(self.tcx()));
 
         // see whether the object trait can be upcast to the trait we are looking for
-        let obligation_def_id = obligation.predicate.def_id();
-        let upcast_trait_ref = match util::upcast(self.tcx(), poly_trait_ref, obligation_def_id) {
-            Some(r) => r,
-            None => { return; }
-        };
-
-        debug!("assemble_candidates_from_object_ty: upcast_trait_ref={}",
-               upcast_trait_ref.repr(self.tcx()));
-
-        // check whether the upcast version of the trait-ref matches what we are looking for
-        if let Ok(()) = self.infcx.probe(|_| self.match_poly_trait_ref(obligation,
-                                                                       upcast_trait_ref.clone())) {
-            debug!("assemble_candidates_from_object_ty: matched, pushing candidate");
+        let upcast_trait_refs = self.upcast(poly_trait_ref, obligation);
+        if upcast_trait_refs.len() > 1 {
+            // can be upcast in many ways; need more type information
+            candidates.ambiguous = true;
+        } else if upcast_trait_refs.len() == 1 {
             candidates.vec.push(ObjectCandidate);
         }
     }
@@ -2063,28 +2055,24 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             }
         };
 
-        let obligation_def_id = obligation.predicate.def_id();
-        let upcast_trait_ref = match util::upcast(self.tcx(),
-                                                  poly_trait_ref.clone(),
-                                                  obligation_def_id) {
-            Some(r) => r,
-            None => {
-                self.tcx().sess.span_bug(obligation.cause.span,
-                                         &format!("unable to upcast from {} to {}",
-                                                  poly_trait_ref.repr(self.tcx()),
-                                                  obligation_def_id.repr(self.tcx())));
-            }
-        };
+        // Upcast the object type to the obligation type. There must
+        // be exactly one applicable trait-reference; if this were not
+        // the case, we would have reported an ambiguity error rather
+        // than successfully selecting one of the candidates.
+        let upcast_trait_refs = self.upcast(poly_trait_ref.clone(), obligation);
+        assert_eq!(upcast_trait_refs.len(), 1);
+        let upcast_trait_ref = upcast_trait_refs.into_iter().next().unwrap();
 
-        match self.match_poly_trait_ref(obligation, upcast_trait_ref) {
+        match self.match_poly_trait_ref(obligation, upcast_trait_ref.clone()) {
             Ok(()) => { }
             Err(()) => {
                 self.tcx().sess.span_bug(obligation.cause.span,
                                          "failed to match trait refs");
             }
         }
 
-        VtableObjectData { object_ty: self_ty }
+        VtableObjectData { object_ty: self_ty,
+                           upcast_trait_ref: upcast_trait_ref }
     }
 
     fn confirm_fn_pointer_candidate(&mut self,
@@ -2501,6 +2489,27 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             obligation.cause.clone()
         }
     }
+
+    /// Upcasts an object trait-reference into those that match the obligation.
+    fn upcast(&mut self, obj_trait_ref: ty::PolyTraitRef<'tcx>, obligation: &TraitObligation<'tcx>)
+              -> Vec<ty::PolyTraitRef<'tcx>>
+    {
+        debug!("upcast(obj_trait_ref={}, obligation={})",
+               obj_trait_ref.repr(self.tcx()),
+               obligation.repr(self.tcx()));
+
+        let obligation_def_id = obligation.predicate.def_id();
+        let mut upcast_trait_refs = util::upcast(self.tcx(), obj_trait_ref, obligation_def_id);
+
+        // retain only those upcast versions that match the trait-ref we are looking for
+        upcast_trait_refs.retain(|upcast_trait_ref| {
+            let upcast_trait_ref = upcast_trait_ref.clone();
+            self.infcx.probe(|_| self.match_poly_trait_ref(obligation, upcast_trait_ref)).is_ok()
+        });
+
+        debug!("upcast: upcast_trait_refs={}", upcast_trait_refs.repr(self.tcx()));
+        upcast_trait_refs
+    }
 }
 
 impl<'tcx> Repr<'tcx> for SelectionCandidate<'tcx> {
diff --git a/src/librustc/middle/traits/util.rs b/src/librustc/middle/traits/util.rs
@@ -359,19 +359,15 @@ pub fn predicate_for_builtin_bound<'tcx>(
 pub fn upcast<'tcx>(tcx: &ty::ctxt<'tcx>,
                     source_trait_ref: ty::PolyTraitRef<'tcx>,
                     target_trait_def_id: ast::DefId)
-                    -> Option<ty::PolyTraitRef<'tcx>>
+                    -> Vec<ty::PolyTraitRef<'tcx>>
 {
     if source_trait_ref.def_id() == target_trait_def_id {
-        return Some(source_trait_ref); // shorcut the most common case
+        return vec![source_trait_ref]; // shorcut the most common case
     }
 
-    for super_trait_ref in supertraits(tcx, source_trait_ref) {
-        if super_trait_ref.def_id() == target_trait_def_id {
-            return Some(super_trait_ref);
-        }
-    }
-
-    None
+    supertraits(tcx, source_trait_ref)
+        .filter(|r| r.def_id() == target_trait_def_id)
+        .collect()
 }
 
 /// Given an object of type `object_trait_ref`, returns the index of
diff --git a/src/librustc/middle/ty_fold.rs b/src/librustc/middle/ty_fold.rs
@@ -544,7 +544,8 @@ impl<'tcx, N: TypeFoldable<'tcx>> TypeFoldable<'tcx> for traits::Vtable<'tcx, N>
 impl<'tcx> TypeFoldable<'tcx> for traits::VtableObjectData<'tcx> {
     fn fold_with<F:TypeFolder<'tcx>>(&self, folder: &mut F) -> traits::VtableObjectData<'tcx> {
         traits::VtableObjectData {
-            object_ty: self.object_ty.fold_with(folder)
+            object_ty: self.object_ty.fold_with(folder),
+            upcast_trait_ref: self.upcast_trait_ref.fold_with(folder),
         }
     }
 }
diff --git a/src/librustc_trans/trans/meth.rs b/src/librustc_trans/trans/meth.rs
@@ -300,7 +300,8 @@ pub fn trans_static_method_callee<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
                                   .position(|item| item.def_id() == method_id)
                                   .unwrap();
             let (llfn, ty) =
-                trans_object_shim(ccx, data.object_ty, trait_id, method_offset_in_trait);
+                trans_object_shim(ccx, data.object_ty, data.upcast_trait_ref.clone(),
+                                  method_offset_in_trait);
             immediate_rvalue(llfn, ty)
         }
         _ => {
@@ -386,7 +387,8 @@ fn trans_monomorphized_callee<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
             Callee { bcx: bcx, data: Fn(llfn) }
         }
         traits::VtableObject(ref data) => {
-            let (llfn, _) = trans_object_shim(bcx.ccx(), data.object_ty, trait_id, n_method);
+            let (llfn, _) = trans_object_shim(bcx.ccx(), data.object_ty,
+                                              data.upcast_trait_ref.clone(), n_method);
             Callee { bcx: bcx, data: Fn(llfn) }
         }
         traits::VtableBuiltin(..) |
@@ -551,16 +553,17 @@ pub fn trans_trait_callee_from_llval<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 pub fn trans_object_shim<'a, 'tcx>(
     ccx: &'a CrateContext<'a, 'tcx>,
     object_ty: Ty<'tcx>,
-    trait_id: ast::DefId,
+    upcast_trait_ref: ty::PolyTraitRef<'tcx>,
     method_offset_in_trait: uint)
     -> (ValueRef, Ty<'tcx>)
 {
     let _icx = push_ctxt("trans_object_shim");
     let tcx = ccx.tcx();
+    let trait_id = upcast_trait_ref.def_id();
 
-    debug!("trans_object_shim(object_ty={}, trait_id={}, method_offset_in_trait={})",
+    debug!("trans_object_shim(object_ty={}, upcast_trait_ref={}, method_offset_in_trait={})",
            object_ty.repr(tcx),
-           trait_id.repr(tcx),
+           upcast_trait_ref.repr(tcx),
            method_offset_in_trait);
 
     let object_trait_ref =
@@ -575,7 +578,6 @@ pub fn trans_object_shim<'a, 'tcx>(
         };
 
     // Upcast to the trait in question and extract out the substitutions.
-    let upcast_trait_ref = traits::upcast(ccx.tcx(), object_trait_ref.clone(), trait_id).unwrap();
     let upcast_trait_ref = ty::erase_late_bound_regions(tcx, &upcast_trait_ref);
     let object_substs = upcast_trait_ref.substs.clone().erase_regions();
     debug!("trans_object_shim: object_substs={}", object_substs.repr(tcx));
diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs
@@ -634,16 +634,21 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
               target_trait_def_id: ast::DefId)
               -> ty::PolyTraitRef<'tcx>
     {
-        match traits::upcast(self.tcx(), source_trait_ref.clone(), target_trait_def_id) {
-            Some(super_trait_ref) => super_trait_ref,
-            None => {
-                self.tcx().sess.span_bug(
-                    self.span,
-                    &format!("cannot upcast `{}` to `{}`",
-                             source_trait_ref.repr(self.tcx()),
-                             target_trait_def_id.repr(self.tcx())));
-            }
+        let upcast_trait_refs = traits::upcast(self.tcx(),
+                                               source_trait_ref.clone(),
+                                               target_trait_def_id);
+
+        // must be exactly one trait ref or we'd get an ambig error etc
+        if upcast_trait_refs.len() != 1 {
+            self.tcx().sess.span_bug(
+                self.span,
+                &format!("cannot uniquely upcast `{}` to `{}`: `{}`",
+                         source_trait_ref.repr(self.tcx()),
+                         target_trait_def_id.repr(self.tcx()),
+                         upcast_trait_refs.repr(self.tcx())));
         }
+
+        upcast_trait_refs.into_iter().next().unwrap()
     }
 
     fn replace_late_bound_regions_with_fresh_var<T>(&self, value: &ty::Binder<T>) -> T
diff --git a/src/test/compile-fail/issue-3953.rs b/src/test/compile-fail/issue-3953.rs
diff --git a/src/test/compile-fail/traits-repeated-supertrait-ambig.rs b/src/test/compile-fail/traits-repeated-supertrait-ambig.rs
@@ -0,0 +1,53 @@
+// Copyright 2015 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.
+
+// Test a case of a trait which extends the same supertrait twice, but
+// with difference type parameters. Test then that when we don't give
+// enough information to pick between these, no selection is made. In
+// this particular case, the two choices are i64/u64 -- so when we use
+// an integer literal, we wind up falling this literal back to i32.
+// See also `run-pass/trait-repeated-supertrait.rs`.
+
+trait CompareTo<T> {
+    fn same_as(&self, t: T) -> bool;
+}
+
+trait CompareToInts : CompareTo<i64> + CompareTo<u64> {
+}
+
+impl CompareTo<i64> for i64 {
+    fn same_as(&self, t: i64) -> bool { *self == t }
+}
+
+impl CompareTo<u64> for i64 {
+    fn same_as(&self, t: u64) -> bool { *self == (t as i64) }
+}
+
+impl CompareToInts for i64 { }
+
+fn with_obj(c: &CompareToInts) -> bool {
+    c.same_as(22) //~ ERROR `CompareTo<i32>` is not implemented
+}
+
+fn with_trait<C:CompareToInts>(c: &C) -> bool {
+    c.same_as(22) //~ ERROR `CompareTo<i32>` is not implemented
+}
+
+fn with_ufcs1<C:CompareToInts>(c: &C) -> bool {
+    CompareToInts::same_as(c, 22) //~ ERROR `CompareTo<i32>` is not implemented
+}
+
+fn with_ufcs2<C:CompareToInts>(c: &C) -> bool {
+    CompareTo::same_as(c, 22) //~ ERROR `CompareTo<i32>` is not implemented
+}
+
+fn main() {
+    assert_eq!(22_i64.same_as(22), true); //~ ERROR `CompareTo<i32>` is not implemented
+}
diff --git a/src/test/compile-fail/unsized4.rs b/src/test/compile-fail/unsized4.rs
diff --git a/src/test/run-pass/traits-repeated-supertrait.rs b/src/test/run-pass/traits-repeated-supertrait.rs
@@ -0,0 +1,56 @@
+// Copyright 2015 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.
+
+// Test a case of a trait which extends the same supertrait twice, but
+// with difference type parameters. Test that we can invoke the
+// various methods in various ways successfully.
+// See also `compile-fail/trait-repeated-supertrait-ambig.rs`.
+
+trait CompareTo<T> {
+    fn same_as(&self, t: T) -> bool;
+}
+
+trait CompareToInts : CompareTo<i64> + CompareTo<u64> {
+}
+
+impl CompareTo<i64> for i64 {
+    fn same_as(&self, t: i64) -> bool { *self == t }
+}
+
+impl CompareTo<u64> for i64 {
+    fn same_as(&self, t: u64) -> bool { *self == (t as i64) }
+}
+
+impl CompareToInts for i64 { }
+
+fn with_obj(c: &CompareToInts) -> bool {
+    c.same_as(22_i64) && c.same_as(22_u64)
+}
+
+fn with_trait<C:CompareToInts>(c: &C) -> bool {
+    c.same_as(22_i64) && c.same_as(22_u64)
+}
+
+fn with_ufcs1<C:CompareToInts>(c: &C) -> bool {
+    CompareToInts::same_as(c, 22_i64) && CompareToInts::same_as(c, 22_u64)
+}
+
+fn with_ufcs2<C:CompareToInts>(c: &C) -> bool {
+    CompareTo::same_as(c, 22_i64) && CompareTo::same_as(c, 22_u64)
+}
+
+fn main() {
+    assert_eq!(22_i64.same_as(22_i64), true);
+    assert_eq!(22_i64.same_as(22_u64), true);
+    assert_eq!(with_trait(&22), true);
+    assert_eq!(with_obj(&22), true);
+    assert_eq!(with_ufcs1(&22), true);
+    assert_eq!(with_ufcs2(&22), true);
+}

Original file line number	Diff line number	Diff line change
`@@ -544,7 +544,8 @@ impl<'tcx, N: TypeFoldable<'tcx>> TypeFoldable<'tcx> for traits::Vtable<'tcx, N>`
`544`	`544`	`impl<'tcx> TypeFoldable<'tcx> for traits::VtableObjectData<'tcx> {`
`545`	`545`	`fn fold_with<F:TypeFolder<'tcx>>(&self, folder: &mut F) -> traits::VtableObjectData<'tcx> {`
`546`	`546`	`traits::VtableObjectData {`
`547`		`- object_ty: self.object_ty.fold_with(folder)`
	`547`	`+ object_ty: self.object_ty.fold_with(folder),`
	`548`	`+ upcast_trait_ref: self.upcast_trait_ref.fold_with(folder),`
`548`	`549`	`}`
`549`	`550`	`}`
`550`	`551`	`}`