From c2d9b4e3349552ec0bf53e3fc7a1f509d449b210 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 8 May 2017 23:36:37 +0200 Subject: [PATCH 01/10] ICH: Hash lists of local trait impls as part of the HIR. --- src/librustc/dep_graph/dep_node.rs | 3 + src/librustc/hir/map/mod.rs | 4 +- src/librustc/ich/fingerprint.rs | 8 +++ src/librustc/ich/hcx.rs | 24 +++++++- src/librustc/ich/mod.rs | 3 +- src/librustc_incremental/calculate_svh/mod.rs | 55 ++++++++++++++++++- src/librustc_metadata/creader.rs | 9 +++ src/librustc_metadata/cstore.rs | 2 + src/librustc_metadata/cstore_impl.rs | 4 +- src/librustc_metadata/decoder.rs | 18 +++--- src/librustc_metadata/schema.rs | 14 +++++ 11 files changed, 127 insertions(+), 17 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 25fc5b7a4f6..93dd76a6984 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -116,6 +116,8 @@ pub enum DepNode { // than changes in the impl body. TraitImpls(D), + AllLocalTraitImpls, + // Nodes representing caches. To properly handle a true cache, we // don't use a DepTrackingMap, but rather we push a task node. // Otherwise the write into the map would be incorrectly @@ -263,6 +265,7 @@ impl DepNode { ConstEval(ref d) => op(d).map(ConstEval), SymbolName(ref d) => op(d).map(SymbolName), TraitImpls(ref d) => op(d).map(TraitImpls), + AllLocalTraitImpls => Some(AllLocalTraitImpls), TraitItems(ref d) => op(d).map(TraitItems), ReprHints(ref d) => op(d).map(ReprHints), TraitSelect { ref trait_def_id, ref input_def_id } => { diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index c715484a934..868730edfed 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -497,7 +497,7 @@ impl<'hir> Map<'hir> { } pub fn trait_impls(&self, trait_did: DefId) -> &'hir [NodeId] { - self.dep_graph.read(DepNode::TraitImpls(trait_did)); + self.dep_graph.read(DepNode::AllLocalTraitImpls); // NB: intentionally bypass `self.forest.krate()` so that we // do not trigger a read of the whole krate here @@ -505,7 +505,7 @@ impl<'hir> Map<'hir> { } pub fn trait_default_impl(&self, trait_did: DefId) -> Option { - self.dep_graph.read(DepNode::TraitImpls(trait_did)); + self.dep_graph.read(DepNode::AllLocalTraitImpls); // NB: intentionally bypass `self.forest.krate()` so that we // do not trigger a read of the whole krate here diff --git a/src/librustc/ich/fingerprint.rs b/src/librustc/ich/fingerprint.rs index e760f7efc93..ccdbab88b8b 100644 --- a/src/librustc/ich/fingerprint.rs +++ b/src/librustc/ich/fingerprint.rs @@ -94,3 +94,11 @@ impl stable_hasher::StableHasherResult for Fingerprint { fingerprint } } + +impl stable_hasher::HashStable for Fingerprint { + fn hash_stable(&self, + _: &mut CTX, + hasher: &mut stable_hasher::StableHasher) { + ::std::hash::Hash::hash(&self.0, hasher); + } +} diff --git a/src/librustc/ich/hcx.rs b/src/librustc/ich/hcx.rs index 3a6367c353c..786d1c5035d 100644 --- a/src/librustc/ich/hcx.rs +++ b/src/librustc/ich/hcx.rs @@ -16,7 +16,7 @@ use ty; use util::nodemap::NodeMap; use std::hash as std_hash; -use std::collections::{HashMap, HashSet}; +use std::collections::{HashMap, HashSet, BTreeMap}; use syntax::ast; use syntax::attr; @@ -348,3 +348,25 @@ pub fn hash_stable_nodemap<'a, 'tcx, V, W>(hcx: &mut StableHashingContext<'a, 't hcx.tcx.hir.definitions().node_to_hir_id(*node_id).local_id }); } + + +pub fn hash_stable_btreemap<'a, 'tcx, K, V, SK, F, W>(hcx: &mut StableHashingContext<'a, 'tcx>, + hasher: &mut StableHasher, + map: &BTreeMap, + extract_stable_key: F) + where K: Eq + Ord, + V: HashStable>, + SK: HashStable> + Ord + Clone, + F: Fn(&mut StableHashingContext<'a, 'tcx>, &K) -> SK, + W: StableHasherResult, +{ + let mut keys: Vec<_> = map.keys() + .map(|k| (extract_stable_key(hcx, k), k)) + .collect(); + keys.sort_unstable_by_key(|&(ref stable_key, _)| stable_key.clone()); + keys.len().hash_stable(hcx, hasher); + for (stable_key, key) in keys { + stable_key.hash_stable(hcx, hasher); + map[key].hash_stable(hcx, hasher); + } +} diff --git a/src/librustc/ich/mod.rs b/src/librustc/ich/mod.rs index d881a1cc45a..5b238090850 100644 --- a/src/librustc/ich/mod.rs +++ b/src/librustc/ich/mod.rs @@ -13,7 +13,8 @@ pub use self::fingerprint::Fingerprint; pub use self::caching_codemap_view::CachingCodemapView; pub use self::hcx::{StableHashingContext, NodeIdHashingMode, hash_stable_hashmap, - hash_stable_hashset, hash_stable_nodemap}; + hash_stable_hashset, hash_stable_nodemap, + hash_stable_btreemap}; mod fingerprint; mod caching_codemap_view; mod hcx; diff --git a/src/librustc_incremental/calculate_svh/mod.rs b/src/librustc_incremental/calculate_svh/mod.rs index 6f5cc1f3f45..8cdabc1d894 100644 --- a/src/librustc_incremental/calculate_svh/mod.rs +++ b/src/librustc_incremental/calculate_svh/mod.rs @@ -36,9 +36,10 @@ use rustc::hir::def_id::{LOCAL_CRATE, CRATE_DEF_INDEX, DefId}; use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc::ich::{Fingerprint, StableHashingContext}; use rustc::ty::TyCtxt; +use rustc::util::common::record_time; use rustc_data_structures::stable_hasher::{StableHasher, HashStable}; use rustc_data_structures::fx::FxHashMap; -use rustc::util::common::record_time; +use rustc_data_structures::accumulate_vec::AccumulateVec; pub type IchHasher = StableHasher; @@ -159,6 +160,11 @@ impl<'a, 'tcx: 'a> ComputeItemHashesVisitor<'a, 'tcx> { // difference, filter them out. return None } + DepNode::AllLocalTraitImpls => { + // These are already covered by hashing + // the HIR. + return None + } ref other => { bug!("Found unexpected DepNode during \ SVH computation: {:?}", @@ -213,6 +219,49 @@ impl<'a, 'tcx: 'a> ComputeItemHashesVisitor<'a, 'tcx> { true, (module, (span, attrs))); } + + fn compute_and_store_ich_for_trait_impls(&mut self, krate: &'tcx hir::Crate) + { + let tcx = self.hcx.tcx(); + + let mut impls: Vec<(u64, Fingerprint)> = krate + .trait_impls + .iter() + .map(|(&trait_id, impls)| { + let trait_id = tcx.def_path_hash(trait_id); + let mut impls: AccumulateVec<[_; 32]> = impls + .iter() + .map(|&node_id| { + let def_id = tcx.hir.local_def_id(node_id); + tcx.def_path_hash(def_id) + }) + .collect(); + + impls.sort_unstable(); + let mut hasher = StableHasher::new(); + impls.hash_stable(&mut self.hcx, &mut hasher); + (trait_id, hasher.finish()) + }) + .collect(); + + impls.sort_unstable(); + + let mut default_impls: AccumulateVec<[_; 32]> = krate + .trait_default_impl + .iter() + .map(|(&trait_def_id, &impl_node_id)| { + let impl_def_id = tcx.hir.local_def_id(impl_node_id); + (tcx.def_path_hash(trait_def_id), tcx.def_path_hash(impl_def_id)) + }) + .collect(); + + default_impls.sort_unstable(); + + let mut hasher = StableHasher::new(); + impls.hash_stable(&mut self.hcx, &mut hasher); + + self.hashes.insert(DepNode::AllLocalTraitImpls, hasher.finish()); + } } impl<'a, 'tcx: 'a> ItemLikeVisitor<'tcx> for ComputeItemHashesVisitor<'a, 'tcx> { @@ -235,6 +284,8 @@ impl<'a, 'tcx: 'a> ItemLikeVisitor<'tcx> for ComputeItemHashesVisitor<'a, 'tcx> } } + + pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> IncrementalHashesMap { let _ignore = tcx.dep_graph.in_ignore(); @@ -272,6 +323,8 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) let fingerprint = hasher.finish(); visitor.hashes.insert(dep_node, fingerprint); } + + visitor.compute_and_store_ich_for_trait_impls(krate); }); tcx.sess.perf_stats.incr_comp_hashes_count.set(visitor.hashes.len() as u64); diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index d2874f16289..54f09a0f319 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -315,11 +315,20 @@ impl<'a> CrateLoader<'a> { let exported_symbols = crate_root.exported_symbols .map(|x| x.decode(&metadata).collect()); + let trait_impls = crate_root + .impls + .map(|impls| { + impls.decode(&metadata) + .map(|trait_impls| (trait_impls.trait_id, trait_impls.impls)) + .collect() + }); + let mut cmeta = cstore::CrateMetadata { name: name, extern_crate: Cell::new(None), def_path_table: def_path_table, exported_symbols: exported_symbols, + trait_impls: trait_impls, proc_macros: crate_root.macro_derive_registrar.map(|_| { self.load_derive_macros(&crate_root, dylib.clone().map(|p| p.0), span) }), diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs index c12b4209675..4e70387c8b0 100644 --- a/src/librustc_metadata/cstore.rs +++ b/src/librustc_metadata/cstore.rs @@ -85,6 +85,8 @@ pub struct CrateMetadata { pub exported_symbols: Tracked>, + pub trait_impls: Tracked>>, + pub dep_kind: Cell, pub source: CrateSource, diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index dbf3e94832f..d42aab60c6e 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -147,10 +147,8 @@ impl CrateStore for cstore::CStore { fn implementations_of_trait(&self, filter: Option) -> Vec { - if let Some(def_id) = filter { - self.dep_graph.read(DepNode::MetaData(def_id)); - } let mut result = vec![]; + self.iter_crate_data(|_, cdata| { cdata.get_implementations_for_trait(filter, &self.dep_graph, &mut result) }); diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 819095e2628..51822474254 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -961,17 +961,17 @@ impl<'a, 'tcx> CrateMetadata { None => None, }; - // FIXME(eddyb) Make this O(1) instead of O(n). let dep_node = self.metadata_dep_node(GlobalMetaDataKind::Impls); - for trait_impls in self.root.impls.get(dep_graph, dep_node).decode(self) { - if filter.is_some() && filter != Some(trait_impls.trait_id) { - continue; + + if let Some(filter) = filter { + if let Some(impls) = self.trait_impls + .get(dep_graph, dep_node) + .get(&filter) { + result.extend(impls.decode(self).map(|idx| self.local_def_id(idx))); } - - result.extend(trait_impls.impls.decode(self).map(|index| self.local_def_id(index))); - - if filter.is_some() { - break; + } else { + for impls in self.trait_impls.get(dep_graph, dep_node).values() { + result.extend(impls.decode(self).map(|idx| self.local_def_id(idx))); } } } diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index 5abe1adfb6f..91a22d92219 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -221,6 +221,20 @@ impl Tracked { } } +impl<'a, 'tcx, T> HashStable> for Tracked + where T: HashStable> +{ + fn hash_stable(&self, + hcx: &mut StableHashingContext<'a, 'tcx>, + hasher: &mut StableHasher) { + let Tracked { + ref state + } = *self; + + state.hash_stable(hcx, hasher); + } +} + #[derive(RustcEncodable, RustcDecodable)] pub struct CrateRoot { From 513cc6d538885959f504b2fba4f613d2b9690a4c Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 11 May 2017 15:46:22 +0200 Subject: [PATCH 02/10] Make incr. comp. test case dependent on specific ICH instead of SVH --- .../remapped_paths_cc/auxiliary/extern_crate.rs | 7 +++---- src/test/incremental/remapped_paths_cc/main.rs | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/test/incremental/remapped_paths_cc/auxiliary/extern_crate.rs b/src/test/incremental/remapped_paths_cc/auxiliary/extern_crate.rs index 09db90d618b..9feefb4c037 100644 --- a/src/test/incremental/remapped_paths_cc/auxiliary/extern_crate.rs +++ b/src/test/incremental/remapped_paths_cc/auxiliary/extern_crate.rs @@ -10,10 +10,9 @@ // ignore-tidy-linelength -// aux-build:extern_crate.rs -//[rpass1] compile-flags: -g -//[rpass2] compile-flags: -g -//[rpass3] compile-flags: -g -Zremap-path-prefix-from={{src-base}} -Zremap-path-prefix-to=/the/src +//[rpass1] compile-flags: -g -Zincremental-cc +//[rpass2] compile-flags: -g -Zincremental-cc +//[rpass3] compile-flags: -g -Zincremental-cc -Zremap-path-prefix-from={{src-base}} -Zremap-path-prefix-to=/the/src #![feature(rustc_attrs)] #![crate_type="rlib"] diff --git a/src/test/incremental/remapped_paths_cc/main.rs b/src/test/incremental/remapped_paths_cc/main.rs index 8a8c658accc..be4764c7d99 100644 --- a/src/test/incremental/remapped_paths_cc/main.rs +++ b/src/test/incremental/remapped_paths_cc/main.rs @@ -9,7 +9,7 @@ // except according to those terms. // revisions:rpass1 rpass2 rpass3 -// compile-flags: -Z query-dep-graph -g +// compile-flags: -Z query-dep-graph -g -Zincremental-cc // aux-build:extern_crate.rs From 77b7df33077aedfe8df903f95165872243f4acd4 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 11 May 2017 15:47:15 +0200 Subject: [PATCH 03/10] Fix instability in GlobalMetadata::Impls ICH. --- src/librustc_metadata/encoder.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index fa4ebed1618..93fcdc455e5 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -943,7 +943,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> { let trait_ref = tcx.impl_trait_ref(def_id); let parent = if let Some(trait_ref) = trait_ref { let trait_def = tcx.trait_def(trait_ref.def_id); - trait_def.ancestors(def_id).skip(1).next().and_then(|node| { + trait_def.ancestors(tcx, def_id).skip(1).next().and_then(|node| { match node { specialization_graph::Node::Impl(parent) => Some(parent), _ => None, @@ -1295,23 +1295,37 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> { /// Encodes an index, mapping each trait to its (local) implementations. fn encode_impls(&mut self, _: ()) -> LazySeq { + debug!("IsolatedEncoder::encode_impls()"); + let tcx = self.tcx; let mut visitor = ImplVisitor { - tcx: self.tcx, + tcx: tcx, impls: FxHashMap(), }; - self.tcx.hir.krate().visit_all_item_likes(&mut visitor); + tcx.hir.krate().visit_all_item_likes(&mut visitor); - let all_impls: Vec<_> = visitor.impls + let mut all_impls: Vec<_> = visitor.impls.into_iter().collect(); + + // Bring everything into deterministic order for hashing + all_impls.sort_unstable_by_key(|&(trait_def_id, _)| { + tcx.def_path_hash(trait_def_id) + }); + + let all_impls: Vec<_> = all_impls .into_iter() - .map(|(trait_def_id, impls)| { + .map(|(trait_def_id, mut impls)| { + // Bring everything into deterministic order for hashing + impls.sort_unstable_by_key(|&def_index| { + tcx.hir.definitions().def_path_hash(def_index) + }); + TraitImpls { trait_id: (trait_def_id.krate.as_u32(), trait_def_id.index), - impls: self.lazy_seq(impls), + impls: self.lazy_seq_from_slice(&impls[..]), } }) .collect(); - self.lazy_seq(all_impls) + self.lazy_seq_from_slice(&all_impls[..]) } // Encodes all symbols exported from this crate into the metadata. From 8da2fe8ed72cc9960f4043867fc55c2b4bc4a3e3 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 11 May 2017 16:01:19 +0200 Subject: [PATCH 04/10] Remove interior mutability from TraitDef by turning fields into queries. --- src/librustc/dep_graph/dep_node.rs | 4 + src/librustc/diagnostics.rs | 61 ++++ src/librustc/traits/mod.rs | 18 +- src/librustc/traits/object_safety.rs | 25 +- src/librustc/traits/project.rs | 21 +- src/librustc/traits/specialize/mod.rs | 50 ++- .../traits/specialize/specialization_graph.rs | 25 +- src/librustc/ty/maps.rs | 40 +++ src/librustc/ty/mod.rs | 38 +- src/librustc/ty/trait_def.rs | 336 ++++-------------- src/librustc_driver/driver.rs | 3 + src/librustc_metadata/decoder.rs | 15 +- src/librustc_metadata/lib.rs | 1 + src/librustc_typeck/check/mod.rs | 4 +- src/librustc_typeck/coherence/mod.rs | 4 - src/librustc_typeck/coherence/overlap.rs | 35 +- src/librustc_typeck/collect.rs | 12 +- src/librustc_typeck/diagnostics.rs | 61 ---- 18 files changed, 300 insertions(+), 453 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 93dd76a6984..15c4469b746 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -106,6 +106,8 @@ pub enum DepNode { UsedTraitImports(D), ConstEval(D), SymbolName(D), + SpecializationGraph(D), + ObjectSafety(D), // The set of impls for a given trait. Ultimately, it would be // nice to get more fine-grained here (e.g., to include a @@ -264,6 +266,8 @@ impl DepNode { UsedTraitImports(ref d) => op(d).map(UsedTraitImports), ConstEval(ref d) => op(d).map(ConstEval), SymbolName(ref d) => op(d).map(SymbolName), + SpecializationGraph(ref d) => op(d).map(SpecializationGraph), + ObjectSafety(ref d) => op(d).map(ObjectSafety), TraitImpls(ref d) => op(d).map(TraitImpls), AllLocalTraitImpls => Some(AllLocalTraitImpls), TraitItems(ref d) => op(d).map(TraitItems), diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 8ef42826fac..470dcb4bd61 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -409,6 +409,67 @@ RFC. It is, however, [currently unimplemented][iss15872]. [iss15872]: https://github.com/rust-lang/rust/issues/15872 "##, +E0119: r##" +There are conflicting trait implementations for the same type. +Example of erroneous code: + +```compile_fail,E0119 +trait MyTrait { + fn get(&self) -> usize; +} + +impl MyTrait for T { + fn get(&self) -> usize { 0 } +} + +struct Foo { + value: usize +} + +impl MyTrait for Foo { // error: conflicting implementations of trait + // `MyTrait` for type `Foo` + fn get(&self) -> usize { self.value } +} +``` + +When looking for the implementation for the trait, the compiler finds +both the `impl MyTrait for T` where T is all types and the `impl +MyTrait for Foo`. Since a trait cannot be implemented multiple times, +this is an error. So, when you write: + +``` +trait MyTrait { + fn get(&self) -> usize; +} + +impl MyTrait for T { + fn get(&self) -> usize { 0 } +} +``` + +This makes the trait implemented on all types in the scope. So if you +try to implement it on another one after that, the implementations will +conflict. Example: + +``` +trait MyTrait { + fn get(&self) -> usize; +} + +impl MyTrait for T { + fn get(&self) -> usize { 0 } +} + +struct Foo; + +fn main() { + let f = Foo; + + f.get(); // the trait is implemented so we can use it +} +``` +"##, + E0133: r##" Unsafe code was used outside of an unsafe function or block. diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 2f525e1b8b4..1823373348b 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -619,8 +619,6 @@ pub fn get_vtable_methods<'a, 'tcx>( debug!("get_vtable_methods({:?})", trait_ref); supertraits(tcx, trait_ref).flat_map(move |trait_ref| { - tcx.populate_implementations_for_trait_if_necessary(trait_ref.def_id()); - let trait_methods = tcx.associated_items(trait_ref.def_id()) .filter(|item| item.kind == ty::AssociatedKind::Method); @@ -782,3 +780,19 @@ impl<'tcx> TraitObligation<'tcx> { ty::Binder(self.predicate.skip_binder().self_ty()) } } + +pub fn provide(providers: &mut ty::maps::Providers) { + *providers = ty::maps::Providers { + is_object_safe: object_safety::is_object_safe_provider, + specialization_graph_of: specialize::specialization_graph_provider, + ..*providers + }; +} + +pub fn provide_extern(providers: &mut ty::maps::Providers) { + *providers = ty::maps::Providers { + is_object_safe: object_safety::is_object_safe_provider, + specialization_graph_of: specialize::specialization_graph_provider, + ..*providers + }; +} diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index 66e8e503be4..0e3a53129d1 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -77,25 +77,6 @@ pub enum MethodViolationCode { } impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { - pub fn is_object_safe(self, trait_def_id: DefId) -> bool { - // Because we query yes/no results frequently, we keep a cache: - let def = self.trait_def(trait_def_id); - - let result = def.object_safety().unwrap_or_else(|| { - let result = self.object_safety_violations(trait_def_id).is_empty(); - - // Record just a yes/no result in the cache; this is what is - // queried most frequently. Note that this may overwrite a - // previous result, but always with the same thing. - def.set_object_safety(result); - - result - }); - - debug!("is_object_safe({:?}) = {}", trait_def_id, result); - - result - } /// Returns the object safety violations that affect /// astconv - currently, Self in supertraits. This is needed @@ -391,3 +372,9 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { error } } + +pub(super) fn is_object_safe_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + trait_def_id: DefId) + -> bool { + tcx.object_safety_violations(trait_def_id).is_empty() +} diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index e01f97eb1f3..d6f6350373a 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -1320,23 +1320,10 @@ fn assoc_ty_def<'cx, 'gcx, 'tcx>( let trait_def_id = selcx.tcx().impl_trait_ref(impl_def_id).unwrap().def_id; let trait_def = selcx.tcx().trait_def(trait_def_id); - if !trait_def.is_complete(selcx.tcx()) { - let impl_node = specialization_graph::Node::Impl(impl_def_id); - for item in impl_node.items(selcx.tcx()) { - if item.kind == ty::AssociatedKind::Type && item.name == assoc_ty_name { - return Some(specialization_graph::NodeItem { - node: specialization_graph::Node::Impl(impl_def_id), - item: item, - }); - } - } - None - } else { - trait_def - .ancestors(impl_def_id) - .defs(selcx.tcx(), assoc_ty_name, ty::AssociatedKind::Type) - .next() - } + trait_def + .ancestors(selcx.tcx(), impl_def_id) + .defs(selcx.tcx(), assoc_ty_name, ty::AssociatedKind::Type) + .next() } // # Cache diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 3882e218241..b3956e813c4 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -27,6 +27,7 @@ use ty::subst::{Subst, Substs}; use traits::{self, Reveal, ObligationCause}; use ty::{self, TyCtxt, TypeFoldable}; use syntax_pos::DUMMY_SP; +use std::rc::Rc; pub mod specialization_graph; @@ -118,7 +119,7 @@ pub fn find_associated_item<'a, 'tcx>( let trait_def_id = tcx.trait_id_of_impl(impl_data.impl_def_id).unwrap(); let trait_def = tcx.trait_def(trait_def_id); - let ancestors = trait_def.ancestors(impl_data.impl_def_id); + let ancestors = trait_def.ancestors(tcx, impl_data.impl_def_id); match ancestors.defs(tcx, item.name, item.kind).next() { Some(node_item) => { let substs = tcx.infer_ctxt((), Reveal::All).enter(|infcx| { @@ -285,3 +286,50 @@ impl SpecializesCache { self.map.insert((a, b), result); } } + +// Query provider for `specialization_graph_of`. +pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + trait_id: DefId) + -> Rc { + let mut sg = specialization_graph::Graph::new(); + + for &impl_def_id in tcx.trait_impls_of(trait_id).iter() { + if impl_def_id.is_local() { + // This is where impl overlap checking happens: + let insert_result = sg.insert(tcx, impl_def_id); + // Report error if there was one. + if let Err(overlap) = insert_result { + let mut err = struct_span_err!(tcx.sess, + tcx.span_of_impl(impl_def_id).unwrap(), + E0119, + "conflicting implementations of trait `{}`{}:", + overlap.trait_desc, + overlap.self_desc.clone().map_or(String::new(), + |ty| { + format!(" for type `{}`", ty) + })); + + match tcx.span_of_impl(overlap.with_impl) { + Ok(span) => { + err.span_label(span, format!("first implementation here")); + err.span_label(tcx.span_of_impl(impl_def_id).unwrap(), + format!("conflicting implementation{}", + overlap.self_desc + .map_or(String::new(), + |ty| format!(" for `{}`", ty)))); + } + Err(cname) => { + err.note(&format!("conflicting implementation in crate `{}`", cname)); + } + } + + err.emit(); + } + } else { + let parent = tcx.impl_parent(impl_def_id).unwrap_or(trait_id); + sg.record_impl_from_cstore(tcx, parent, impl_def_id) + } + } + + Rc::new(sg) +} diff --git a/src/librustc/traits/specialize/specialization_graph.rs b/src/librustc/traits/specialize/specialization_graph.rs index 6e2c16c82ae..87c98a0ef0e 100644 --- a/src/librustc/traits/specialize/specialization_graph.rs +++ b/src/librustc/traits/specialize/specialization_graph.rs @@ -12,8 +12,9 @@ use super::{OverlapError, specializes}; use hir::def_id::DefId; use traits::{self, Reveal}; -use ty::{self, TyCtxt, TraitDef, TypeFoldable}; +use ty::{self, TyCtxt, TypeFoldable}; use ty::fast_reject::{self, SimplifiedType}; +use std::rc::Rc; use syntax::ast::Name; use util::nodemap::{DefIdMap, FxHashMap}; @@ -301,18 +302,19 @@ impl<'a, 'gcx, 'tcx> Node { } } -pub struct Ancestors<'a> { - trait_def: &'a TraitDef, +pub struct Ancestors { + trait_def_id: DefId, + specialization_graph: Rc, current_source: Option, } -impl<'a> Iterator for Ancestors<'a> { +impl Iterator for Ancestors { type Item = Node; fn next(&mut self) -> Option { let cur = self.current_source.take(); if let Some(Node::Impl(cur_impl)) = cur { - let parent = self.trait_def.specialization_graph.borrow().parent(cur_impl); - if parent == self.trait_def.def_id { + let parent = self.specialization_graph.parent(cur_impl); + if parent == self.trait_def_id { self.current_source = Some(Node::Trait(parent)); } else { self.current_source = Some(Node::Impl(parent)); @@ -336,7 +338,7 @@ impl NodeItem { } } -impl<'a, 'gcx, 'tcx> Ancestors<'a> { +impl<'a, 'gcx, 'tcx> Ancestors { /// Search the items from the given ancestors, returning each definition /// with the given name and the given kind. #[inline] // FIXME(#35870) Avoid closures being unexported due to impl Trait. @@ -351,9 +353,14 @@ impl<'a, 'gcx, 'tcx> Ancestors<'a> { /// Walk up the specialization ancestors of a given impl, starting with that /// impl itself. -pub fn ancestors<'a>(trait_def: &'a TraitDef, start_from_impl: DefId) -> Ancestors<'a> { +pub fn ancestors(tcx: TyCtxt, + trait_def_id: DefId, + start_from_impl: DefId) + -> Ancestors { + let specialization_graph = tcx.specialization_graph_of(trait_def_id); Ancestors { - trait_def: trait_def, + trait_def_id, + specialization_graph, current_source: Some(Node::Impl(start_from_impl)), } } diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 1fd9e8f7375..188ef289ba9 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -18,10 +18,12 @@ use middle::region::RegionMaps; use mir; use mir::transform::{MirSuite, MirPassIndex}; use session::CompileResult; +use traits::specialization_graph; use ty::{self, CrateInherentImpls, Ty, TyCtxt}; use ty::item_path; use ty::steal::Steal; use ty::subst::Substs; +use ty::fast_reject::SimplifiedType; use util::nodemap::{DefIdSet, NodeSet}; use rustc_data_structures::indexed_vec::IndexVec; @@ -98,6 +100,15 @@ impl Key for (CrateNum, DefId) { } } +impl Key for (DefId, SimplifiedType) { + fn map_crate(&self) -> CrateNum { + self.0.krate + } + fn default_span(&self, tcx: TyCtxt) -> Span { + self.0.default_span(tcx) + } +} + impl<'tcx> Key for (DefId, &'tcx Substs<'tcx>) { fn map_crate(&self) -> CrateNum { self.0.krate @@ -391,6 +402,24 @@ impl<'tcx> QueryDescription for queries::is_mir_available<'tcx> { } } +impl<'tcx> QueryDescription for queries::trait_impls_of<'tcx> { + fn describe(tcx: TyCtxt, def_id: DefId) -> String { + format!("trait impls of `{}`", tcx.item_path_str(def_id)) + } +} + +impl<'tcx> QueryDescription for queries::relevant_trait_impls_for<'tcx> { + fn describe(tcx: TyCtxt, (def_id, ty): (DefId, SimplifiedType)) -> String { + format!("relevant impls for: `({}, {:?})`", tcx.item_path_str(def_id), ty) + } +} + +impl<'tcx> QueryDescription for queries::is_object_safe<'tcx> { + fn describe(tcx: TyCtxt, def_id: DefId) -> String { + format!("determine object safety of trait `{}`", tcx.item_path_str(def_id)) + } +} + macro_rules! define_maps { (<$tcx:tt> $($(#[$attr:meta])* @@ -820,6 +849,13 @@ define_maps! { <'tcx> [] item_body_nested_bodies: ItemBodyNestedBodies(DefId) -> Rc>, [] const_is_rvalue_promotable_to_static: ConstIsRvaluePromotableToStatic(DefId) -> bool, [] is_mir_available: IsMirAvailable(DefId) -> bool, + + [] trait_impls_of: TraitImpls(DefId) -> Rc>, + // Note that TraitDef::for_each_relevant_impl() will do type simplication for you. + [] relevant_trait_impls_for: relevant_trait_impls_for((DefId, SimplifiedType)) + -> Rc>, + [] specialization_graph_of: SpecializationGraph(DefId) -> Rc, + [] is_object_safe: ObjectSafety(DefId) -> bool, } fn coherent_trait_dep_node((_, def_id): (CrateNum, DefId)) -> DepNode { @@ -859,3 +895,7 @@ fn mir_keys(_: CrateNum) -> DepNode { fn crate_variances(_: CrateNum) -> DepNode { DepNode::CrateVariances } + +fn relevant_trait_impls_for((def_id, _): (DefId, SimplifiedType)) -> DepNode { + DepNode::TraitImpls(def_id) +} diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 6ca401d27ac..a86d7351ef4 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -80,7 +80,7 @@ pub use self::context::{Lift, TypeckTables}; pub use self::instance::{Instance, InstanceDef}; -pub use self::trait_def::{TraitDef, TraitFlags}; +pub use self::trait_def::TraitDef; pub use self::maps::queries; @@ -2324,37 +2324,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } pub fn trait_has_default_impl(self, trait_def_id: DefId) -> bool { - let def = self.trait_def(trait_def_id); - def.flags.get().intersects(TraitFlags::HAS_DEFAULT_IMPL) - } - - /// Populates the type context with all the implementations for the given - /// trait if necessary. - pub fn populate_implementations_for_trait_if_necessary(self, trait_id: DefId) { - if trait_id.is_local() { - return - } - - // The type is not local, hence we are reading this out of - // metadata and don't need to track edges. - let _ignore = self.dep_graph.in_ignore(); - - let def = self.trait_def(trait_id); - if def.flags.get().intersects(TraitFlags::HAS_REMOTE_IMPLS) { - return; - } - - debug!("populate_implementations_for_trait_if_necessary: searching for {:?}", def); - - for impl_def_id in self.sess.cstore.implementations_of_trait(Some(trait_id)) { - let trait_ref = self.impl_trait_ref(impl_def_id).unwrap(); - - // Record the trait->implementation mapping. - let parent = self.impl_parent(impl_def_id).unwrap_or(trait_id); - def.record_remote_impl(self, impl_def_id, trait_ref, parent); - } - - def.flags.set(def.flags.get() | TraitFlags::HAS_REMOTE_IMPLS); + self.trait_def(trait_def_id).has_default_impl } /// Given the def_id of an impl, return the def_id of the trait it implements. @@ -2603,6 +2573,8 @@ pub fn provide(providers: &mut ty::maps::Providers) { adt_dtorck_constraint, def_span, trait_of_item, + trait_impls_of: trait_def::trait_impls_of_provider, + relevant_trait_impls_for: trait_def::relevant_trait_impls_provider, ..*providers }; } @@ -2611,6 +2583,8 @@ pub fn provide_extern(providers: &mut ty::maps::Providers) { *providers = ty::maps::Providers { adt_sized_constraint, adt_dtorck_constraint, + trait_impls_of: trait_def::trait_impls_of_provider, + relevant_trait_impls_for: trait_def::relevant_trait_impls_provider, ..*providers }; } diff --git a/src/librustc/ty/trait_def.rs b/src/librustc/ty/trait_def.rs index 097b596c5eb..b0357a35b83 100644 --- a/src/librustc/ty/trait_def.rs +++ b/src/librustc/ty/trait_def.rs @@ -8,18 +8,13 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use dep_graph::DepNode; -use hir::def_id::{DefId, LOCAL_CRATE}; -use traits::{self, specialization_graph}; -use ty; +use hir::def_id::DefId; +use traits::specialization_graph; use ty::fast_reject; -use ty::{Ty, TyCtxt, TraitRef}; -use std::cell::{Cell, RefCell}; +use ty::fold::TypeFoldable; +use ty::{Ty, TyCtxt}; +use std::rc::Rc; use hir; -use util::nodemap::FxHashMap; - -use syntax::ast; -use syntax_pos::DUMMY_SP; /// A trait's definition with type information. pub struct TraitDef { @@ -33,40 +28,7 @@ pub struct TraitDef { /// be usable with the sugar (or without it). pub paren_sugar: bool, - // Impls of a trait. To allow for quicker lookup, the impls are indexed by a - // simplified version of their `Self` type: impls with a simplifiable `Self` - // are stored in `nonblanket_impls` keyed by it, while all other impls are - // stored in `blanket_impls`. - // - // A similar division is used within `specialization_graph`, but the ones - // here are (1) stored as a flat list for the trait and (2) populated prior - // to -- and used while -- determining specialization order. - // - // FIXME: solve the reentrancy issues and remove these lists in favor of the - // ones in `specialization_graph`. - // - // These lists are tracked by `DepNode::TraitImpls`; we don't use - // a DepTrackingMap but instead have the `TraitDef` insert the - // required reads/writes. - - /// Impls of the trait. - nonblanket_impls: RefCell< - FxHashMap> - >, - - /// Blanket impls associated with the trait. - blanket_impls: RefCell>, - - /// The specialization order for impls of this trait. - pub specialization_graph: RefCell, - - /// Various flags - pub flags: Cell, - - /// The number of impls we've added from the local crate. - /// When this number matches up the list in the HIR map, - /// we're done, and the specialization graph is correct. - local_impl_count: Cell, + pub has_default_impl: bool, /// The ICH of this trait's DefPath, cached here so it doesn't have to be /// recomputed all the time. @@ -77,193 +39,28 @@ impl<'a, 'gcx, 'tcx> TraitDef { pub fn new(def_id: DefId, unsafety: hir::Unsafety, paren_sugar: bool, + has_default_impl: bool, def_path_hash: u64) -> TraitDef { TraitDef { - def_id: def_id, - paren_sugar: paren_sugar, - unsafety: unsafety, - nonblanket_impls: RefCell::new(FxHashMap()), - blanket_impls: RefCell::new(vec![]), - flags: Cell::new(ty::TraitFlags::NO_TRAIT_FLAGS), - local_impl_count: Cell::new(0), - specialization_graph: RefCell::new(traits::specialization_graph::Graph::new()), - def_path_hash: def_path_hash, + def_id, + paren_sugar, + unsafety, + has_default_impl, + def_path_hash, } } - // returns None if not yet calculated - pub fn object_safety(&self) -> Option { - if self.flags.get().intersects(TraitFlags::OBJECT_SAFETY_VALID) { - Some(self.flags.get().intersects(TraitFlags::IS_OBJECT_SAFE)) - } else { - None - } - } - - pub fn set_object_safety(&self, is_safe: bool) { - assert!(self.object_safety().map(|cs| cs == is_safe).unwrap_or(true)); - self.flags.set( - self.flags.get() | if is_safe { - TraitFlags::OBJECT_SAFETY_VALID | TraitFlags::IS_OBJECT_SAFE - } else { - TraitFlags::OBJECT_SAFETY_VALID - } - ); - } - - fn write_trait_impls(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) { - tcx.dep_graph.write(DepNode::TraitImpls(self.def_id)); - } - - fn read_trait_impls(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) { - tcx.dep_graph.read(DepNode::TraitImpls(self.def_id)); - } - - /// Records a basic trait-to-implementation mapping. - /// - /// Returns `true` iff the impl has not previously been recorded. - fn record_impl(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, - impl_def_id: DefId, - impl_trait_ref: TraitRef<'tcx>) - -> bool { - debug!("TraitDef::record_impl for {:?}, from {:?}", - self, impl_trait_ref); - - // Record the write into the impl set, but only for local - // impls: external impls are handled differently. - if impl_def_id.is_local() { - self.write_trait_impls(tcx); - } - - // We don't want to borrow_mut after we already populated all impls, - // so check if an impl is present with an immutable borrow first. - if let Some(sty) = fast_reject::simplify_type(tcx, - impl_trait_ref.self_ty(), false) { - if let Some(is) = self.nonblanket_impls.borrow().get(&sty) { - if is.contains(&impl_def_id) { - return false; // duplicate - skip - } - } - - self.nonblanket_impls.borrow_mut().entry(sty).or_insert(vec![]).push(impl_def_id) - } else { - if self.blanket_impls.borrow().contains(&impl_def_id) { - return false; // duplicate - skip - } - self.blanket_impls.borrow_mut().push(impl_def_id) - } - - true - } - - /// Records a trait-to-implementation mapping for a crate-local impl. - pub fn record_local_impl(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, - impl_def_id: DefId, - impl_trait_ref: TraitRef<'tcx>) { - assert!(impl_def_id.is_local()); - let was_new = self.record_impl(tcx, impl_def_id, impl_trait_ref); - assert!(was_new); - - self.local_impl_count.set(self.local_impl_count.get() + 1); - } - - /// Records a trait-to-implementation mapping. - pub fn record_has_default_impl(&self) { - self.flags.set(self.flags.get() | TraitFlags::HAS_DEFAULT_IMPL); - } - - /// Records a trait-to-implementation mapping for a non-local impl. - /// - /// The `parent_impl` is the immediately-less-specialized impl, or the - /// trait's def ID if the impl is not a specialization -- information that - /// should be pulled from the metadata. - pub fn record_remote_impl(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, - impl_def_id: DefId, - impl_trait_ref: TraitRef<'tcx>, - parent_impl: DefId) { - assert!(!impl_def_id.is_local()); - - // if the impl has not previously been recorded - if self.record_impl(tcx, impl_def_id, impl_trait_ref) { - // if the impl is non-local, it's placed directly into the - // specialization graph using parent information drawn from metadata. - self.specialization_graph.borrow_mut() - .record_impl_from_cstore(tcx, parent_impl, impl_def_id) - } - } - - /// Adds a local impl into the specialization graph, returning an error with - /// overlap information if the impl overlaps but does not specialize an - /// existing impl. - pub fn add_impl_for_specialization(&self, - tcx: TyCtxt<'a, 'gcx, 'tcx>, - impl_def_id: DefId) - -> Result<(), traits::OverlapError> { - assert!(impl_def_id.is_local()); - - self.specialization_graph.borrow_mut() - .insert(tcx, impl_def_id) - } - - pub fn ancestors(&'a self, of_impl: DefId) -> specialization_graph::Ancestors<'a> { - specialization_graph::ancestors(self, of_impl) - } - - /// Whether the impl set and specialization graphs are complete. - pub fn is_complete(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> bool { - tcx.populate_implementations_for_trait_if_necessary(self.def_id); - ty::queries::coherent_trait::try_get(tcx, DUMMY_SP, (LOCAL_CRATE, self.def_id)).is_ok() - } - - /// If any local impls haven't been added yet, returns - /// Some(list of local impls for this trait). - fn missing_local_impls(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) - -> Option<&'gcx [ast::NodeId]> { - if self.flags.get().intersects(TraitFlags::HAS_LOCAL_IMPLS) { - return None; - } - - if self.is_complete(tcx) { - self.flags.set(self.flags.get() | TraitFlags::HAS_LOCAL_IMPLS); - return None; - } - - let impls = tcx.hir.trait_impls(self.def_id); - assert!(self.local_impl_count.get() <= impls.len()); - if self.local_impl_count.get() == impls.len() { - self.flags.set(self.flags.get() | TraitFlags::HAS_LOCAL_IMPLS); - return None; - } - - Some(impls) + pub fn ancestors(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, + of_impl: DefId) + -> specialization_graph::Ancestors { + specialization_graph::ancestors(tcx, self.def_id, of_impl) } pub fn for_each_impl(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, mut f: F) { - self.read_trait_impls(tcx); - tcx.populate_implementations_for_trait_if_necessary(self.def_id); - - let local_impls = self.missing_local_impls(tcx); - if let Some(impls) = local_impls { - for &id in impls { - f(tcx.hir.local_def_id(id)); - } - } - let mut f = |def_id: DefId| { - if !(local_impls.is_some() && def_id.is_local()) { - f(def_id); - } - }; - - for &impl_def_id in self.blanket_impls.borrow().iter() { + for &impl_def_id in tcx.trait_impls_of(self.def_id).iter() { f(impl_def_id); } - - for v in self.nonblanket_impls.borrow().values() { - for &impl_def_id in v { - f(impl_def_id); - } - } } /// Iterate over every impl that could possibly match the @@ -273,25 +70,6 @@ impl<'a, 'gcx, 'tcx> TraitDef { self_ty: Ty<'tcx>, mut f: F) { - self.read_trait_impls(tcx); - tcx.populate_implementations_for_trait_if_necessary(self.def_id); - - let local_impls = self.missing_local_impls(tcx); - if let Some(impls) = local_impls { - for &id in impls { - f(tcx.hir.local_def_id(id)); - } - } - let mut f = |def_id: DefId| { - if !(local_impls.is_some() && def_id.is_local()) { - f(def_id); - } - }; - - for &impl_def_id in self.blanket_impls.borrow().iter() { - f(impl_def_id); - } - // simplify_type(.., false) basically replaces type parameters and // projections with infer-variables. This is, of course, done on // the impl trait-ref when it is instantiated, but not on the @@ -304,29 +82,71 @@ impl<'a, 'gcx, 'tcx> TraitDef { // replace `S` with anything - this impl of course can't be // selected, and as there are hundreds of similar impls, // considering them would significantly harm performance. - if let Some(simp) = fast_reject::simplify_type(tcx, self_ty, true) { - if let Some(impls) = self.nonblanket_impls.borrow().get(&simp) { - for &impl_def_id in impls { - f(impl_def_id); - } - } + let relevant_impls = if let Some(simplified_self_ty) = + fast_reject::simplify_type(tcx, self_ty, true) { + tcx.relevant_trait_impls_for((self.def_id, simplified_self_ty)) } else { - for v in self.nonblanket_impls.borrow().values() { - for &impl_def_id in v { - f(impl_def_id); - } - } + tcx.trait_impls_of(self.def_id) + }; + + for &impl_def_id in relevant_impls.iter() { + f(impl_def_id); } } } -bitflags! { - flags TraitFlags: u32 { - const NO_TRAIT_FLAGS = 0, - const HAS_DEFAULT_IMPL = 1 << 0, - const IS_OBJECT_SAFE = 1 << 1, - const OBJECT_SAFETY_VALID = 1 << 2, - const HAS_REMOTE_IMPLS = 1 << 3, - const HAS_LOCAL_IMPLS = 1 << 4, +// Query provider for `trait_impls_of`. +pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + trait_id: DefId) + -> Rc> { + let mut impls = if trait_id.is_local() { + // Traits defined in the current crate can't have impls in upstream + // crates, so we don't bother querying the cstore. + Vec::new() + } else { + tcx.sess.cstore.implementations_of_trait(Some(trait_id)) + }; + + impls.extend(tcx.hir + .trait_impls(trait_id) + .iter() + .map(|&node_id| tcx.hir.local_def_id(node_id)) + .filter(|&impl_def_id| { + let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); + !trait_ref.references_error() + })); + Rc::new(impls) +} + +// Query provider for `relevant_trait_impls_for`. +pub(super) fn relevant_trait_impls_provider<'a, 'tcx>( + tcx: TyCtxt<'a, 'tcx, 'tcx>, + (trait_id, self_ty): (DefId, fast_reject::SimplifiedType)) + -> Rc> +{ + let all_trait_impls = tcx.trait_impls_of(trait_id); + + let relevant: Vec = all_trait_impls + .iter() + .map(|&impl_def_id| impl_def_id) + .filter(|&impl_def_id| { + let impl_trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); + let impl_simple_self_ty = fast_reject::simplify_type(tcx, + impl_trait_ref.self_ty(), + false); + if let Some(impl_simple_self_ty) = impl_simple_self_ty { + impl_simple_self_ty == self_ty + } else { + // blanket impl (?) + true + } + }) + .collect(); + + if all_trait_impls.len() == relevant.len() { + // If we didn't filter anything out, re-use the existing vec. + all_trait_impls + } else { + Rc::new(relevant) } } diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 8fddbe110b0..c23563e13be 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -22,6 +22,7 @@ use rustc::middle::{self, dependency_format, stability, reachable}; use rustc::middle::privacy::AccessLevels; use rustc::mir::transform::{MIR_CONST, MIR_VALIDATED, MIR_OPTIMIZED, Passes}; use rustc::ty::{self, TyCtxt, Resolutions, GlobalArenas}; +use rustc::traits; use rustc::util::common::time; use rustc::util::nodemap::NodeSet; use rustc::util::fs::rename_or_copy_remove; @@ -892,6 +893,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, trans::provide(&mut local_providers); typeck::provide(&mut local_providers); ty::provide(&mut local_providers); + traits::provide(&mut local_providers); reachable::provide(&mut local_providers); rustc_const_eval::provide(&mut local_providers); middle::region::provide(&mut local_providers); @@ -900,6 +902,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, cstore::provide(&mut extern_providers); trans::provide(&mut extern_providers); ty::provide_extern(&mut extern_providers); + traits::provide_extern(&mut extern_providers); // FIXME(eddyb) get rid of this once we replace const_eval with miri. rustc_const_eval::provide(&mut extern_providers); diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 51822474254..941b965d9a4 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -505,16 +505,11 @@ impl<'a, 'tcx> CrateMetadata { _ => bug!(), }; - let def = ty::TraitDef::new(self.local_def_id(item_id), - data.unsafety, - data.paren_sugar, - self.def_path_table.def_path_hash(item_id)); - - if data.has_default_impl { - def.record_has_default_impl(); - } - - def + ty::TraitDef::new(self.local_def_id(item_id), + data.unsafety, + data.paren_sugar, + data.has_default_impl, + self.def_path_table.def_path_hash(item_id)) } fn get_variant(&self, item: &Entry, index: DefIndex) -> ty::VariantDef { diff --git a/src/librustc_metadata/lib.rs b/src/librustc_metadata/lib.rs index 27555f49e57..6a40174fa02 100644 --- a/src/librustc_metadata/lib.rs +++ b/src/librustc_metadata/lib.rs @@ -29,6 +29,7 @@ #![cfg_attr(stage0, unstable(feature = "rustc_private", issue = "27812"))] #![cfg_attr(stage0, feature(staged_api))] +#![feature(sort_unstable)] #[macro_use] extern crate log; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 70d2867c08c..d304d79bc52 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1200,7 +1200,7 @@ fn check_specialization_validity<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_id: DefId, impl_item: &hir::ImplItem) { - let ancestors = trait_def.ancestors(impl_id); + let ancestors = trait_def.ancestors(tcx, impl_id); let kind = match impl_item.node { hir::ImplItemKind::Const(..) => ty::AssociatedKind::Const, @@ -1330,7 +1330,7 @@ fn check_impl_items_against_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let mut invalidated_items = Vec::new(); let associated_type_overridden = overridden_associated_type.is_some(); for trait_item in tcx.associated_items(impl_trait_ref.def_id) { - let is_implemented = trait_def.ancestors(impl_id) + let is_implemented = trait_def.ancestors(tcx, impl_id) .defs(tcx, trait_item.name, trait_item.kind) .next() .map(|node_item| !node_item.node.is_from_trait()) diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 8b9dc20315d..165be49f760 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -46,8 +46,6 @@ fn check_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, node_id: ast::NodeId) { } enforce_trait_manually_implementable(tcx, impl_def_id, trait_ref.def_id); - let trait_def = tcx.trait_def(trait_ref.def_id); - trait_def.record_local_impl(tcx, impl_def_id, trait_ref); } } @@ -117,8 +115,6 @@ pub fn provide(providers: &mut Providers) { fn coherent_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, (_, def_id): (CrateNum, DefId)) { - tcx.populate_implementations_for_trait_if_necessary(def_id); - let impls = tcx.hir.trait_impls(def_id); for &impl_id in impls { check_impl(tcx, impl_id); diff --git a/src/librustc_typeck/coherence/overlap.rs b/src/librustc_typeck/coherence/overlap.rs index f479dc2e6ab..ba1d7b18e8c 100644 --- a/src/librustc_typeck/coherence/overlap.rs +++ b/src/librustc_typeck/coherence/overlap.rs @@ -41,39 +41,10 @@ pub fn check_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, node_id: ast::NodeId) { let _task = tcx.dep_graph.in_task(DepNode::CoherenceOverlapCheck(trait_def_id)); - let def = tcx.trait_def(trait_def_id); + // Trigger building the specialization graph for the trait of this impl. + // This will detect any overlap errors. + tcx.specialization_graph_of(trait_def_id); - // attempt to insert into the specialization graph - let insert_result = def.add_impl_for_specialization(tcx, impl_def_id); - - // insertion failed due to overlap - if let Err(overlap) = insert_result { - let mut err = struct_span_err!(tcx.sess, - tcx.span_of_impl(impl_def_id).unwrap(), - E0119, - "conflicting implementations of trait `{}`{}:", - overlap.trait_desc, - overlap.self_desc.clone().map_or(String::new(), - |ty| { - format!(" for type `{}`", ty) - })); - - match tcx.span_of_impl(overlap.with_impl) { - Ok(span) => { - err.span_label(span, "first implementation here"); - err.span_label(tcx.span_of_impl(impl_def_id).unwrap(), - format!("conflicting implementation{}", - overlap.self_desc - .map_or(String::new(), - |ty| format!(" for `{}`", ty)))); - } - Err(cname) => { - err.note(&format!("conflicting implementation in crate `{}`", cname)); - } - } - - err.emit(); - } // check for overlap with the automatic `impl Trait for Trait` if let ty::TyDynamic(ref data, ..) = trait_ref.self_ty().sty { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 7c6c70024ce..cb1bd3e099d 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -749,12 +749,12 @@ fn trait_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } let def_path_hash = tcx.def_path_hash(def_id); - let def = ty::TraitDef::new(def_id, unsafety, paren_sugar, def_path_hash); - - if tcx.hir.trait_is_auto(def_id) { - def.record_has_default_impl(); - } - + let has_default_impl = tcx.hir.trait_is_auto(def_id); + let def = ty::TraitDef::new(def_id, + unsafety, + paren_sugar, + has_default_impl, + def_path_hash); tcx.alloc_trait_def(def) } diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 0f42ee15ecf..f9ebe3fff5b 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -1524,67 +1524,6 @@ impl TypeWrapper { ``` "##, -E0119: r##" -There are conflicting trait implementations for the same type. -Example of erroneous code: - -```compile_fail,E0119 -trait MyTrait { - fn get(&self) -> usize; -} - -impl MyTrait for T { - fn get(&self) -> usize { 0 } -} - -struct Foo { - value: usize -} - -impl MyTrait for Foo { // error: conflicting implementations of trait - // `MyTrait` for type `Foo` - fn get(&self) -> usize { self.value } -} -``` - -When looking for the implementation for the trait, the compiler finds -both the `impl MyTrait for T` where T is all types and the `impl -MyTrait for Foo`. Since a trait cannot be implemented multiple times, -this is an error. So, when you write: - -``` -trait MyTrait { - fn get(&self) -> usize; -} - -impl MyTrait for T { - fn get(&self) -> usize { 0 } -} -``` - -This makes the trait implemented on all types in the scope. So if you -try to implement it on another one after that, the implementations will -conflict. Example: - -``` -trait MyTrait { - fn get(&self) -> usize; -} - -impl MyTrait for T { - fn get(&self) -> usize { 0 } -} - -struct Foo; - -fn main() { - let f = Foo; - - f.get(); // the trait is implemented so we can use it -} -``` -"##, - E0120: r##" An attempt was made to implement Drop on a trait, which is not allowed: only structs and enums can implement Drop. An example causing this error: From 40a6734ae1a1ec9ad96b8f5b58a64d10d0cdbe76 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 15 May 2017 12:21:28 +0200 Subject: [PATCH 05/10] Re-introduce cycle-check in assoc. item resolution. --- src/librustc/traits/project.rs | 38 ++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index d6f6350373a..e119ead7473 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -29,6 +29,7 @@ use infer::type_variable::TypeVariableOrigin; use rustc_data_structures::snapshot_map::{Snapshot, SnapshotMap}; use syntax::ast; use syntax::symbol::Symbol; +use syntax_pos::DUMMY_SP; use ty::subst::Subst; use ty::{self, ToPredicate, ToPolyTraitRef, Ty, TyCtxt}; use ty::fold::{TypeFoldable, TypeFolder}; @@ -1317,13 +1318,38 @@ fn assoc_ty_def<'cx, 'gcx, 'tcx>( assoc_ty_name: ast::Name) -> Option> { - let trait_def_id = selcx.tcx().impl_trait_ref(impl_def_id).unwrap().def_id; - let trait_def = selcx.tcx().trait_def(trait_def_id); + let tcx = selcx.tcx(); + let trait_def_id = tcx.impl_trait_ref(impl_def_id).unwrap().def_id; + let trait_def = tcx.trait_def(trait_def_id); - trait_def - .ancestors(selcx.tcx(), impl_def_id) - .defs(selcx.tcx(), assoc_ty_name, ty::AssociatedKind::Type) - .next() + // This function may be called while we are still building the + // specialization graph that is queried below (via TraidDef::ancestors()), + // so, in order to avoid infinite recursion, we detect this case by + // seeing if a query of the specialization graph fails with a cycle error. + // If we are in cycle, and thus still building the graph, we perform a + // reduced version of the associated item lookup that does not need the + // specialization graph. + let specialization_graph_complete = + ty::queries::specialization_graph_of::try_get(tcx, + DUMMY_SP, + trait_def_id).is_ok(); + if !specialization_graph_complete { + let impl_node = specialization_graph::Node::Impl(impl_def_id); + for item in impl_node.items(tcx) { + if item.kind == ty::AssociatedKind::Type && item.name == assoc_ty_name { + return Some(specialization_graph::NodeItem { + node: specialization_graph::Node::Impl(impl_def_id), + item: item, + }); + } + } + None + } else { + trait_def + .ancestors(tcx, impl_def_id) + .defs(tcx, assoc_ty_name, ty::AssociatedKind::Type) + .next() + } } // # Cache From 742ebc17ff4d033f6b5c6348ddb1c94d730c56b8 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 15 May 2017 15:09:30 +0200 Subject: [PATCH 06/10] Share lists of blanket impls in results of relevant_impls_for() query. --- src/librustc/traits/specialize/mod.rs | 14 ++- src/librustc/ty/maps.rs | 4 +- src/librustc/ty/trait_def.rs | 117 ++++++++++++++++++++------ 3 files changed, 108 insertions(+), 27 deletions(-) diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index b3956e813c4..0e5779f9d17 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -293,7 +293,19 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx -> Rc { let mut sg = specialization_graph::Graph::new(); - for &impl_def_id in tcx.trait_impls_of(trait_id).iter() { + let mut trait_impls: Vec = tcx.trait_impls_of(trait_id).iter().collect(); + + // The coherence checking implementation seems to rely on impls being + // iterated over (roughly) in definition order, so we are sorting by + // negated CrateNum (so remote definitions are visited first) and then + // by a flattend version of the DefIndex. + trait_impls.sort_unstable_by_key(|def_id| { + (-(def_id.krate.as_u32() as i64), + def_id.index.address_space().index(), + def_id.index.as_array_index()) + }); + + for impl_def_id in trait_impls { if impl_def_id.is_local() { // This is where impl overlap checking happens: let insert_result = sg.insert(tcx, impl_def_id); diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 188ef289ba9..0926da2005d 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -850,10 +850,10 @@ define_maps! { <'tcx> [] const_is_rvalue_promotable_to_static: ConstIsRvaluePromotableToStatic(DefId) -> bool, [] is_mir_available: IsMirAvailable(DefId) -> bool, - [] trait_impls_of: TraitImpls(DefId) -> Rc>, + [] trait_impls_of: TraitImpls(DefId) -> ty::trait_def::TraitImpls, // Note that TraitDef::for_each_relevant_impl() will do type simplication for you. [] relevant_trait_impls_for: relevant_trait_impls_for((DefId, SimplifiedType)) - -> Rc>, + -> ty::trait_def::TraitImpls, [] specialization_graph_of: SpecializationGraph(DefId) -> Rc, [] is_object_safe: ObjectSafety(DefId) -> bool, } diff --git a/src/librustc/ty/trait_def.rs b/src/librustc/ty/trait_def.rs index b0357a35b83..eb60f9d1010 100644 --- a/src/librustc/ty/trait_def.rs +++ b/src/librustc/ty/trait_def.rs @@ -35,6 +35,60 @@ pub struct TraitDef { pub def_path_hash: u64, } +// We don't store the list of impls in a flat list because each cached list of +// `relevant_impls_for` we would then duplicate all blanket impls. By keeping +// blanket and non-blanket impls separate, we can share the list of blanket +// impls. +#[derive(Clone)] +pub struct TraitImpls { + blanket_impls: Rc>, + non_blanket_impls: Rc>, +} + +impl TraitImpls { + pub fn iter(&self) -> TraitImplsIter { + TraitImplsIter { + blanket_impls: self.blanket_impls.clone(), + non_blanket_impls: self.non_blanket_impls.clone(), + index: 0 + } + } +} + +#[derive(Clone)] +pub struct TraitImplsIter { + blanket_impls: Rc>, + non_blanket_impls: Rc>, + index: usize, +} + +impl Iterator for TraitImplsIter { + type Item = DefId; + + fn next(&mut self) -> Option { + if self.index < self.blanket_impls.len() { + let bi_index = self.index; + self.index += 1; + Some(self.blanket_impls[bi_index]) + } else { + let nbi_index = self.index - self.blanket_impls.len(); + if nbi_index < self.non_blanket_impls.len() { + self.index += 1; + Some(self.non_blanket_impls[nbi_index]) + } else { + None + } + } + } + + fn size_hint(&self) -> (usize, Option) { + let items_left = (self.blanket_impls.len() + self.non_blanket_impls.len()) - self.index; + (items_left, Some(items_left)) + } +} + +impl ExactSizeIterator for TraitImplsIter {} + impl<'a, 'gcx, 'tcx> TraitDef { pub fn new(def_id: DefId, unsafety: hir::Unsafety, @@ -58,7 +112,7 @@ impl<'a, 'gcx, 'tcx> TraitDef { } pub fn for_each_impl(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, mut f: F) { - for &impl_def_id in tcx.trait_impls_of(self.def_id).iter() { + for impl_def_id in tcx.trait_impls_of(self.def_id).iter() { f(impl_def_id); } } @@ -89,7 +143,7 @@ impl<'a, 'gcx, 'tcx> TraitDef { tcx.trait_impls_of(self.def_id) }; - for &impl_def_id in relevant_impls.iter() { + for impl_def_id in relevant_impls.iter() { f(impl_def_id); } } @@ -98,8 +152,8 @@ impl<'a, 'gcx, 'tcx> TraitDef { // Query provider for `trait_impls_of`. pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trait_id: DefId) - -> Rc> { - let mut impls = if trait_id.is_local() { + -> TraitImpls { + let remote_impls = if trait_id.is_local() { // Traits defined in the current crate can't have impls in upstream // crates, so we don't bother querying the cstore. Vec::new() @@ -107,46 +161,61 @@ pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tcx.sess.cstore.implementations_of_trait(Some(trait_id)) }; - impls.extend(tcx.hir - .trait_impls(trait_id) - .iter() - .map(|&node_id| tcx.hir.local_def_id(node_id)) - .filter(|&impl_def_id| { - let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); - !trait_ref.references_error() - })); - Rc::new(impls) + let mut blanket_impls = Vec::new(); + let mut non_blanket_impls = Vec::new(); + + let local_impls = tcx.hir + .trait_impls(trait_id) + .into_iter() + .map(|&node_id| tcx.hir.local_def_id(node_id)); + + for impl_def_id in local_impls.chain(remote_impls.into_iter()) { + let impl_trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); + if impl_def_id.is_local() && impl_trait_ref.references_error() { + continue + } + + if fast_reject::simplify_type(tcx, impl_trait_ref.self_ty(), false).is_some() { + non_blanket_impls.push(impl_def_id); + } else { + blanket_impls.push(impl_def_id); + } + } + + TraitImpls { + blanket_impls: Rc::new(blanket_impls), + non_blanket_impls: Rc::new(non_blanket_impls), + } } // Query provider for `relevant_trait_impls_for`. pub(super) fn relevant_trait_impls_provider<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, (trait_id, self_ty): (DefId, fast_reject::SimplifiedType)) - -> Rc> + -> TraitImpls { let all_trait_impls = tcx.trait_impls_of(trait_id); let relevant: Vec = all_trait_impls + .non_blanket_impls .iter() - .map(|&impl_def_id| impl_def_id) + .cloned() .filter(|&impl_def_id| { let impl_trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); let impl_simple_self_ty = fast_reject::simplify_type(tcx, impl_trait_ref.self_ty(), - false); - if let Some(impl_simple_self_ty) = impl_simple_self_ty { - impl_simple_self_ty == self_ty - } else { - // blanket impl (?) - true - } + false).unwrap(); + impl_simple_self_ty == self_ty }) .collect(); - if all_trait_impls.len() == relevant.len() { + if all_trait_impls.non_blanket_impls.len() == relevant.len() { // If we didn't filter anything out, re-use the existing vec. all_trait_impls } else { - Rc::new(relevant) + TraitImpls { + blanket_impls: all_trait_impls.blanket_impls.clone(), + non_blanket_impls: Rc::new(relevant), + } } } From d731b04af11e212a9132a86f986ccebb3372aef5 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 16 May 2017 15:03:20 +0200 Subject: [PATCH 07/10] Don't use queries::try_get() in assoc_ty projection --- src/librustc/traits/project.rs | 41 +++++++++++++--------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index e119ead7473..0b12531f60a 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -29,7 +29,6 @@ use infer::type_variable::TypeVariableOrigin; use rustc_data_structures::snapshot_map::{Snapshot, SnapshotMap}; use syntax::ast; use syntax::symbol::Symbol; -use syntax_pos::DUMMY_SP; use ty::subst::Subst; use ty::{self, ToPredicate, ToPolyTraitRef, Ty, TyCtxt}; use ty::fold::{TypeFoldable, TypeFolder}; @@ -1324,32 +1323,24 @@ fn assoc_ty_def<'cx, 'gcx, 'tcx>( // This function may be called while we are still building the // specialization graph that is queried below (via TraidDef::ancestors()), - // so, in order to avoid infinite recursion, we detect this case by - // seeing if a query of the specialization graph fails with a cycle error. - // If we are in cycle, and thus still building the graph, we perform a - // reduced version of the associated item lookup that does not need the - // specialization graph. - let specialization_graph_complete = - ty::queries::specialization_graph_of::try_get(tcx, - DUMMY_SP, - trait_def_id).is_ok(); - if !specialization_graph_complete { - let impl_node = specialization_graph::Node::Impl(impl_def_id); - for item in impl_node.items(tcx) { - if item.kind == ty::AssociatedKind::Type && item.name == assoc_ty_name { - return Some(specialization_graph::NodeItem { - node: specialization_graph::Node::Impl(impl_def_id), - item: item, - }); - } + // so, in order to avoid unnecessary infinite recursion, we manually look + // for the associated item at the given impl. + // If there is no such item in that impl, this function will fail with a + // cycle error if the specialization graph is currently being built. + let impl_node = specialization_graph::Node::Impl(impl_def_id); + for item in impl_node.items(tcx) { + if item.kind == ty::AssociatedKind::Type && item.name == assoc_ty_name { + return Some(specialization_graph::NodeItem { + node: specialization_graph::Node::Impl(impl_def_id), + item: item, + }); } - None - } else { - trait_def - .ancestors(tcx, impl_def_id) - .defs(tcx, assoc_ty_name, ty::AssociatedKind::Type) - .next() } + + trait_def + .ancestors(tcx, impl_def_id) + .defs(tcx, assoc_ty_name, ty::AssociatedKind::Type) + .next() } // # Cache From 0a77a588578b69497caac7a507f875c8f258c9aa Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 16 May 2017 15:04:32 +0200 Subject: [PATCH 08/10] Add test cases for cyclic specialization graph construction --- .../coherence-inherited-assoc-ty-cycle-err.rs | 34 +++++++++++++++++++ .../specialization/assoc-ty-graph-cycle.rs | 33 ++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 src/test/compile-fail/coherence-inherited-assoc-ty-cycle-err.rs create mode 100644 src/test/run-pass/specialization/assoc-ty-graph-cycle.rs diff --git a/src/test/compile-fail/coherence-inherited-assoc-ty-cycle-err.rs b/src/test/compile-fail/coherence-inherited-assoc-ty-cycle-err.rs new file mode 100644 index 00000000000..5d7f3396740 --- /dev/null +++ b/src/test/compile-fail/coherence-inherited-assoc-ty-cycle-err.rs @@ -0,0 +1,34 @@ +// Copyright 2017 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Formerly this ICEd with the following message: +// Tried to project an inherited associated type during coherence checking, +// which is currently not supported. +// +// No we expect to run into a more user-friendly cycle error instead. + +#![feature(specialization)] + +trait Trait { type Assoc; } +//~^ unsupported cyclic reference between types/traits detected [E0391] + +impl Trait for Vec { + type Assoc = (); +} + +impl Trait for Vec {} + +impl Trait for String { + type Assoc = (); +} + +impl Trait< as Trait>::Assoc> for String {} + +fn main() {} diff --git a/src/test/run-pass/specialization/assoc-ty-graph-cycle.rs b/src/test/run-pass/specialization/assoc-ty-graph-cycle.rs new file mode 100644 index 00000000000..a65dcf33d85 --- /dev/null +++ b/src/test/run-pass/specialization/assoc-ty-graph-cycle.rs @@ -0,0 +1,33 @@ +// Copyright 2017 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Make sure we don't crash with a cycle error during coherence. + +#![feature(specialization)] + +trait Trait { + type Assoc; +} + +impl Trait for Vec { + default type Assoc = (); +} + +impl Trait for Vec { + type Assoc = u8; +} + +impl Trait for String { + type Assoc = (); +} + +impl Trait< as Trait>::Assoc> for String {} + +fn main() {} From 42051ceb162b8e69bcc9df580974ccff8f22f3c8 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 16 May 2017 17:12:00 +0200 Subject: [PATCH 09/10] Use tcx.type_of(impl) instead of TraitRef::self_ty() for getting Self in relevant_impls_for(). --- src/librustc/ty/trait_def.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc/ty/trait_def.rs b/src/librustc/ty/trait_def.rs index eb60f9d1010..865297c7ecb 100644 --- a/src/librustc/ty/trait_def.rs +++ b/src/librustc/ty/trait_def.rs @@ -170,12 +170,12 @@ pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, .map(|&node_id| tcx.hir.local_def_id(node_id)); for impl_def_id in local_impls.chain(remote_impls.into_iter()) { - let impl_trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); - if impl_def_id.is_local() && impl_trait_ref.references_error() { + let impl_self_ty = tcx.type_of(impl_def_id); + if impl_def_id.is_local() && impl_self_ty.references_error() { continue } - if fast_reject::simplify_type(tcx, impl_trait_ref.self_ty(), false).is_some() { + if fast_reject::simplify_type(tcx, impl_self_ty, false).is_some() { non_blanket_impls.push(impl_def_id); } else { blanket_impls.push(impl_def_id); @@ -201,9 +201,9 @@ pub(super) fn relevant_trait_impls_provider<'a, 'tcx>( .iter() .cloned() .filter(|&impl_def_id| { - let impl_trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); + let impl_self_ty = tcx.type_of(impl_def_id); let impl_simple_self_ty = fast_reject::simplify_type(tcx, - impl_trait_ref.self_ty(), + impl_self_ty, false).unwrap(); impl_simple_self_ty == self_ty }) From 08660afe90d6c2257c59408116c718929af58963 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 16 May 2017 17:31:18 +0200 Subject: [PATCH 10/10] Remove unreachable branches in traits::project --- src/librustc/traits/project.rs | 186 +++++++++++++-------------------- 1 file changed, 71 insertions(+), 115 deletions(-) diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index 0b12531f60a..d7911870f39 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -900,96 +900,50 @@ fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>( // In either case, we handle this by not adding a // candidate for an impl if it contains a `default` // type. - let opt_node_item = assoc_ty_def(selcx, - impl_data.impl_def_id, - obligation.predicate.item_name); - let new_candidate = if let Some(node_item) = opt_node_item { - let is_default = if node_item.node.is_from_trait() { - // If true, the impl inherited a `type Foo = Bar` - // given in the trait, which is implicitly default. - // Otherwise, the impl did not specify `type` and - // neither did the trait: - // - // ```rust - // trait Foo { type T; } - // impl Foo for Bar { } - // ``` - // - // This is an error, but it will be - // reported in `check_impl_items_against_trait`. - // We accept it here but will flag it as - // an error when we confirm the candidate - // (which will ultimately lead to `normalize_to_error` - // being invoked). - node_item.item.defaultness.has_value() - } else { - node_item.item.defaultness.is_default() || - selcx.tcx().impl_is_default(node_item.node.def_id()) - }; + let node_item = assoc_ty_def(selcx, + impl_data.impl_def_id, + obligation.predicate.item_name); - // Only reveal a specializable default if we're past type-checking - // and the obligations is monomorphic, otherwise passes such as - // transmute checking and polymorphic MIR optimizations could - // get a result which isn't correct for all monomorphizations. - if !is_default { + let is_default = if node_item.node.is_from_trait() { + // If true, the impl inherited a `type Foo = Bar` + // given in the trait, which is implicitly default. + // Otherwise, the impl did not specify `type` and + // neither did the trait: + // + // ```rust + // trait Foo { type T; } + // impl Foo for Bar { } + // ``` + // + // This is an error, but it will be + // reported in `check_impl_items_against_trait`. + // We accept it here but will flag it as + // an error when we confirm the candidate + // (which will ultimately lead to `normalize_to_error` + // being invoked). + node_item.item.defaultness.has_value() + } else { + node_item.item.defaultness.is_default() || + selcx.tcx().impl_is_default(node_item.node.def_id()) + }; + + // Only reveal a specializable default if we're past type-checking + // and the obligations is monomorphic, otherwise passes such as + // transmute checking and polymorphic MIR optimizations could + // get a result which isn't correct for all monomorphizations. + let new_candidate = if !is_default { + Some(ProjectionTyCandidate::Select) + } else if selcx.projection_mode() == Reveal::All { + assert!(!poly_trait_ref.needs_infer()); + if !poly_trait_ref.needs_subst() { Some(ProjectionTyCandidate::Select) - } else if selcx.projection_mode() == Reveal::All { - assert!(!poly_trait_ref.needs_infer()); - if !poly_trait_ref.needs_subst() { - Some(ProjectionTyCandidate::Select) - } else { - None - } } else { None } } else { - // This is saying that neither the trait nor - // the impl contain a definition for this - // associated type. Normally this situation - // could only arise through a compiler bug -- - // if the user wrote a bad item name, it - // should have failed in astconv. **However**, - // at coherence-checking time, we only look at - // the topmost impl (we don't even consider - // the trait itself) for the definition -- and - // so in that case it may be that the trait - // *DOES* have a declaration, but we don't see - // it, and we end up in this branch. - // - // This is kind of tricky to handle actually. - // For now, we just unconditionally ICE, - // because otherwise, examples like the - // following will succeed: - // - // ``` - // trait Assoc { - // type Output; - // } - // - // impl Assoc for T { - // default type Output = bool; - // } - // - // impl Assoc for u8 {} - // impl Assoc for u16 {} - // - // trait Foo {} - // impl Foo for ::Output {} - // impl Foo for ::Output {} - // return None; - // } - // ``` - // - // The essential problem here is that the - // projection fails, leaving two unnormalized - // types, which appear not to unify -- so the - // overlap check succeeds, when it should - // fail. - span_bug!(obligation.cause.span, - "Tried to project an inherited associated type during \ - coherence checking, which is currently not supported."); + None }; + candidate_set.vec.extend(new_candidate); } super::VtableParam(..) => { @@ -1274,35 +1228,25 @@ fn confirm_impl_candidate<'cx, 'gcx, 'tcx>( let VtableImplData { substs, nested, impl_def_id } = impl_vtable; let tcx = selcx.tcx(); - let trait_ref = obligation.predicate.trait_ref; let assoc_ty = assoc_ty_def(selcx, impl_def_id, obligation.predicate.item_name); - match assoc_ty { - Some(node_item) => { - let ty = if !node_item.item.defaultness.has_value() { - // This means that the impl is missing a definition for the - // associated type. This error will be reported by the type - // checker method `check_impl_items_against_trait`, so here we - // just return TyError. - debug!("confirm_impl_candidate: no associated type {:?} for {:?}", - node_item.item.name, - obligation.predicate.trait_ref); - tcx.types.err - } else { - tcx.type_of(node_item.item.def_id) - }; - let substs = translate_substs(selcx.infcx(), impl_def_id, substs, node_item.node); - Progress { - ty: ty.subst(tcx, substs), - obligations: nested, - cacheable: true - } - } - None => { - span_bug!(obligation.cause.span, - "No associated type for {:?}", - trait_ref); - } + let ty = if !assoc_ty.item.defaultness.has_value() { + // This means that the impl is missing a definition for the + // associated type. This error will be reported by the type + // checker method `check_impl_items_against_trait`, so here we + // just return TyError. + debug!("confirm_impl_candidate: no associated type {:?} for {:?}", + assoc_ty.item.name, + obligation.predicate.trait_ref); + tcx.types.err + } else { + tcx.type_of(assoc_ty.item.def_id) + }; + let substs = translate_substs(selcx.infcx(), impl_def_id, substs, assoc_ty.node); + Progress { + ty: ty.subst(tcx, substs), + obligations: nested, + cacheable: true } } @@ -1315,7 +1259,7 @@ fn assoc_ty_def<'cx, 'gcx, 'tcx>( selcx: &SelectionContext<'cx, 'gcx, 'tcx>, impl_def_id: DefId, assoc_ty_name: ast::Name) - -> Option> + -> specialization_graph::NodeItem { let tcx = selcx.tcx(); let trait_def_id = tcx.impl_trait_ref(impl_def_id).unwrap().def_id; @@ -1330,17 +1274,29 @@ fn assoc_ty_def<'cx, 'gcx, 'tcx>( let impl_node = specialization_graph::Node::Impl(impl_def_id); for item in impl_node.items(tcx) { if item.kind == ty::AssociatedKind::Type && item.name == assoc_ty_name { - return Some(specialization_graph::NodeItem { + return specialization_graph::NodeItem { node: specialization_graph::Node::Impl(impl_def_id), item: item, - }); + }; } } - trait_def + if let Some(assoc_item) = trait_def .ancestors(tcx, impl_def_id) .defs(tcx, assoc_ty_name, ty::AssociatedKind::Type) - .next() + .next() { + assoc_item + } else { + // This is saying that neither the trait nor + // the impl contain a definition for this + // associated type. Normally this situation + // could only arise through a compiler bug -- + // if the user wrote a bad item name, it + // should have failed in astconv. + bug!("No associated type `{}` for {}", + assoc_ty_name, + tcx.item_path_str(impl_def_id)) + } } // # Cache