Skip to content

Fix indeterminism in ty::TraitObject representation. #36425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 15, 2016
2 changes: 1 addition & 1 deletion src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
4 changes: 0 additions & 4 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
21 changes: 12 additions & 9 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
Expand Down
10 changes: 8 additions & 2 deletions src/librustc/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
}
}

Expand Down
39 changes: 5 additions & 34 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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());
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 1 addition & 8 deletions src/librustc_metadata/tyencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 5 additions & 0 deletions src/test/run-pass/auxiliary/typeid-intrinsic-aux1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>() }
Expand All @@ -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>() }
5 changes: 5 additions & 0 deletions src/test/run-pass/auxiliary/typeid-intrinsic-aux2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>() }
Expand All @@ -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>() }
9 changes: 9 additions & 0 deletions src/test/run-pass/typeid-intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
}