Rollup merge of rust-lang#36425 - michaelwoerister:stable-projection-bounds, r=eddyb

Fix indeterminism in ty::TraitObject representation.

Make sure that projection bounds in `ty::TraitObject` are sorted in a way that is stable across compilation sessions and crate boundaries.

This PR
+  moves `DefPathHashes` up into `librustc` so it can be used there to create a stable sort key for `DefId`s,
+ changes `PolyExistentialProjection::sort_key()` to take advantage of the above,
+ and removes the unused `PolyProjectionPredicate::sort_key()` and `ProjectionTy::sort_key()` methods.

Fixes rust-lang#36155

Author: Manishearth

src/librustc/ty/context.rs

@@ -1303,7 +1303,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
     }
 
     pub fn mk_trait(self, mut obj: TraitObject<'tcx>) -> Ty<'tcx> {
-        obj.projection_bounds.sort_by(|a, b| a.sort_key().cmp(&b.sort_key()));
+        obj.projection_bounds.sort_by_key(|b| b.sort_key(self));
         self.mk_ty(TyTrait(box obj))
     }
 

src/librustc/ty/mod.rs

@@ -1018,10 +1018,6 @@ impl<'tcx> PolyProjectionPredicate<'tcx> {
     pub fn item_name(&self) -> Name {
         self.0.projection_ty.item_name // safe to skip the binder to access a name
     }
-
-    pub fn sort_key(&self) -> (DefId, Name) {
-        self.0.projection_ty.sort_key()
-    }
 }
 
 pub trait ToPolyTraitRef<'tcx> {

src/librustc/ty/sty.rs

@@ -23,7 +23,7 @@ use std::mem;
 use std::ops;
 use syntax::abi;
 use syntax::ast::{self, Name};
-use syntax::parse::token::keywords;
+use syntax::parse::token::{keywords, InternedString};
 
 use serialize::{Decodable, Decoder, Encodable, Encoder};
 
@@ -440,12 +440,6 @@ pub struct ProjectionTy<'tcx> {
     pub item_name: Name,
 }
 
-impl<'tcx> ProjectionTy<'tcx> {
-    pub fn sort_key(&self) -> (DefId, Name) {
-        (self.trait_ref.def_id, self.item_name)
-    }
-}
-
 #[derive(Clone, PartialEq, Eq, Hash, Debug)]
 pub struct BareFnTy<'tcx> {
     pub unsafety: hir::Unsafety,
@@ -738,8 +732,17 @@ impl<'a, 'tcx, 'gcx> PolyExistentialProjection<'tcx> {
         self.0.item_name // safe to skip the binder to access a name
     }
 
-    pub fn sort_key(&self) -> (DefId, Name) {
-        (self.0.trait_ref.def_id, self.0.item_name)
+    pub fn sort_key(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> (u64, InternedString) {
+        // We want something here that is stable across crate boundaries.
+        // The DefId isn't but the `deterministic_hash` of the corresponding
+        // DefPath is.
+        let trait_def = tcx.lookup_trait_def(self.0.trait_ref.def_id);
+        let def_path_hash = trait_def.def_path_hash;
+
+        // An `ast::Name` is also not stable (it's just an index into an
+        // interning table), so map to the corresponding `InternedString`.
+        let item_name = self.0.item_name.as_str();
+        (def_path_hash, item_name)
     }
 
     pub fn with_self_ty(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>,

src/librustc/ty/trait_def.rs

@@ -70,15 +70,20 @@ pub struct TraitDef<'tcx> {
     pub specialization_graph: RefCell<traits::specialization_graph::Graph>,
 
     /// Various flags
-    pub flags: Cell<TraitFlags>
+    pub flags: Cell<TraitFlags>,
+
+    /// The ICH of this trait's DefPath, cached here so it doesn't have to be
+    /// recomputed all the time.
+    pub def_path_hash: u64,
 }
 
 impl<'a, 'gcx, 'tcx> TraitDef<'tcx> {
     pub fn new(unsafety: hir::Unsafety,
                paren_sugar: bool,
                generics: &'tcx ty::Generics<'tcx>,
                trait_ref: ty::TraitRef<'tcx>,
-               associated_type_names: Vec<Name>)
+               associated_type_names: Vec<Name>,
+               def_path_hash: u64)
                -> TraitDef<'tcx> {
         TraitDef {
             paren_sugar: paren_sugar,
@@ -90,6 +95,7 @@ impl<'a, 'gcx, 'tcx> TraitDef<'tcx> {
             blanket_impls: RefCell::new(vec![]),
             flags: Cell::new(ty::TraitFlags::NO_TRAIT_FLAGS),
             specialization_graph: RefCell::new(traits::specialization_graph::Graph::new()),
+            def_path_hash: def_path_hash,
         }
     }
 

src/librustc/ty/util.rs

@@ -411,15 +411,11 @@ impl<'a, 'gcx, 'tcx> TypeIdHasher<'a, 'gcx, 'tcx> {
     }
 
     fn def_id(&mut self, did: DefId) {
-        // Hash the crate identification information.
-        let name = self.tcx.crate_name(did.krate);
-        let disambiguator = self.tcx.crate_disambiguator(did.krate);
-        self.hash((name, disambiguator));
-
-        // Hash the item path within that crate.
-        // FIXME(#35379) This should use a deterministic
-        // DefPath hashing mechanism, not the DefIndex.
-        self.hash(did.index);
+        // Hash the DefPath corresponding to the DefId, which is independent
+        // of compiler internal state.
+        let tcx = self.tcx;
+        let def_path = tcx.def_path(did);
+        def_path.deterministic_hash_to(tcx, &mut self.state);
     }
 }
 
@@ -445,33 +441,8 @@ impl<'a, 'gcx, 'tcx> TypeVisitor<'tcx> for TypeIdHasher<'a, 'gcx, 'tcx> {
                 self.hash(f.sig.variadic());
             }
             TyTrait(ref data) => {
-                // Trait objects have a list of projection bounds
-                // that are not guaranteed to be sorted in an order
-                // that gets preserved across crates, so we need
-                // to sort them again by the name, in string form.
-
-                // Hash the whole principal trait ref.
                 self.def_id(data.principal.def_id());
-                data.principal.visit_with(self);
-
-                // Hash region and builtin bounds.
-                data.region_bound.visit_with(self);
                 self.hash(data.builtin_bounds);
-
-                // Only projection bounds are left, sort and hash them.
-                let mut projection_bounds: Vec<_> = data.projection_bounds
-                                                        .iter()
-                                                        .map(|b| (b.item_name().as_str(), b))
-                                                        .collect();
-                projection_bounds.sort_by_key(|&(ref name, _)| name.clone());
-                for (name, bound) in projection_bounds {
-                    self.def_id(bound.0.trait_ref.def_id);
-                    self.hash(name);
-                    bound.visit_with(self);
-                }
-
-                // Bypass super_visit_with, we've visited everything.
-                return false;
             }
             TyTuple(tys) => {
                 self.hash(tys.len());

src/librustc_metadata/decoder.rs

@@ -385,12 +385,14 @@ pub fn get_trait_def<'a, 'tcx>(cdata: Cmd,
     let unsafety = parse_unsafety(item_doc);
     let associated_type_names = parse_associated_type_names(item_doc);
     let paren_sugar = parse_paren_sugar(item_doc);
+    let def_path = def_path(cdata, item_id).unwrap();
 
     ty::TraitDef::new(unsafety,
                       paren_sugar,
                       generics,
                       item_trait_ref(item_doc, tcx, cdata),
-                      associated_type_names)
+                      associated_type_names,
+                      def_path.deterministic_hash(tcx))
 }
 
 pub fn get_adt_def<'a, 'tcx>(cdata: Cmd,

src/librustc_metadata/tyencode.rs

@@ -104,14 +104,7 @@ pub fn enc_ty<'a, 'tcx>(w: &mut Cursor<Vec<u8>>, cx: &ctxt<'a, 'tcx>, t: Ty<'tcx
 
             enc_region(w, cx, obj.region_bound);
 
-            // Encode projection_bounds in a stable order
-            let mut projection_bounds: Vec<_> = obj.projection_bounds
-                                                .iter()
-                                                .map(|b| (b.item_name().as_str(), b))
-                                                .collect();
-            projection_bounds.sort_by_key(|&(ref name, _)| name.clone());
-
-            for tp in projection_bounds.iter().map(|&(_, tp)| tp) {
+            for tp in &obj.projection_bounds {
                 write!(w, "P");
                 enc_existential_projection(w, cx, &tp.0);
             }

src/librustc_typeck/collect.rs

@@ -1290,12 +1290,15 @@ fn trait_def_of_item<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
         }
     }).collect();
 
+    let def_path_hash = tcx.def_path(def_id).deterministic_hash(tcx);
+
     let trait_ref = ty::TraitRef::new(def_id, substs);
     let trait_def = ty::TraitDef::new(unsafety,
                                       paren_sugar,
                                       ty_generics,
                                       trait_ref,
-                                      associated_type_names);
+                                      associated_type_names,
+                                      def_path_hash);
 
     tcx.intern_trait_def(trait_def)
 }

src/test/run-pass/auxiliary/typeid-intrinsic-aux1.rs

@@ -22,6 +22,8 @@ pub type F = Option<isize>;
 pub type G = usize;
 pub type H = &'static str;
 pub type I = Box<Fn()>;
+pub type I32Iterator = Iterator<Item=i32>;
+pub type U32Iterator = Iterator<Item=u32>;
 
 pub fn id_A() -> TypeId { TypeId::of::<A>() }
 pub fn id_B() -> TypeId { TypeId::of::<B>() }
@@ -34,3 +36,6 @@ pub fn id_H() -> TypeId { TypeId::of::<H>() }
 pub fn id_I() -> TypeId { TypeId::of::<I>() }
 
 pub fn foo<T: Any>() -> TypeId { TypeId::of::<T>() }
+
+pub fn id_i32_iterator() -> TypeId { TypeId::of::<I32Iterator>() }
+pub fn id_u32_iterator() -> TypeId { TypeId::of::<U32Iterator>() }

src/test/run-pass/auxiliary/typeid-intrinsic-aux2.rs

@@ -22,6 +22,8 @@ pub type F = Option<isize>;
 pub type G = usize;
 pub type H = &'static str;
 pub type I = Box<Fn()>;
+pub type I32Iterator = Iterator<Item=i32>;
+pub type U32Iterator = Iterator<Item=u32>;
 
 pub fn id_A() -> TypeId { TypeId::of::<A>() }
 pub fn id_B() -> TypeId { TypeId::of::<B>() }
@@ -34,3 +36,6 @@ pub fn id_H() -> TypeId { TypeId::of::<H>() }
 pub fn id_I() -> TypeId { TypeId::of::<I>() }
 
 pub fn foo<T: Any>() -> TypeId { TypeId::of::<T>() }
+
+pub fn id_i32_iterator() -> TypeId { TypeId::of::<I32Iterator>() }
+pub fn id_u32_iterator() -> TypeId { TypeId::of::<U32Iterator>() }

src/test/run-pass/typeid-intrinsic.rs

@@ -78,4 +78,13 @@ pub fn main() {
     b.hash(&mut s2);
 
     assert_eq!(s1.finish(), s2.finish());
+
+    // Check projections
+
+    assert_eq!(TypeId::of::<other1::I32Iterator>(), other1::id_i32_iterator());
+    assert_eq!(TypeId::of::<other1::U32Iterator>(), other1::id_u32_iterator());
+    assert_eq!(other1::id_i32_iterator(), other2::id_i32_iterator());
+    assert_eq!(other1::id_u32_iterator(), other2::id_u32_iterator());
+    assert!(other1::id_i32_iterator() != other1::id_u32_iterator());
+    assert!(TypeId::of::<other1::I32Iterator>() != TypeId::of::<other1::U32Iterator>());
 }