diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 30b2c55afa194..80beeafb072a8 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -42,6 +42,7 @@ pub struct OverlapError { pub trait_desc: String, pub self_desc: Option, pub intercrate_ambiguity_causes: Vec, + pub used_to_be_allowed: bool, } /// Given a subst for the requested impl, translate it to a subst @@ -324,56 +325,82 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx def_id.index.as_array_index()) }); + let mut overlaps = Vec::new(); + 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); - // Report error if there was one. - let (overlap, used_to_be_allowed) = match insert_result { - Err(overlap) => (Some(overlap), false), - Ok(opt_overlap) => (opt_overlap, true) + match insert_result { + Ok(Some(mut opt_overlaps)) => { + // record any overlap that occurs between two impl + // later those recordings are processed to establish + // if an intersection impl is present between two overlapping impls + // if no an overlap error is emitted + for opt_overlap in opt_overlaps { + overlaps.push((impl_def_id, opt_overlap)); + } + } + _ => {} }; + } else { + let parent = tcx.impl_parent(impl_def_id).unwrap_or(trait_id); + sg.record_impl_from_cstore(tcx, parent, impl_def_id) + } + } - if let Some(overlap) = overlap { - let msg = format!("conflicting implementations of trait `{}`{}:{}", + if overlaps.len() > 0 { + // Build the graph only if there is at least an overlap + let (graph, nodes_idx) = sg.build_graph(); + for (impl_def_id, overlap) in overlaps { + if !sg.check_overlap(&graph, &nodes_idx, impl_def_id, overlap.with_impl, tcx) { + let msg = format!( + "conflicting implementations of trait `{}`{}:{}", overlap.trait_desc, - overlap.self_desc.clone().map_or( - String::new(), |ty| { - format!(" for type `{}`", ty) - }), - if used_to_be_allowed { " (E0119)" } else { "" } - ); - let impl_span = tcx.sess.codemap().def_span( - tcx.span_of_impl(impl_def_id).unwrap() + overlap + .self_desc + .clone() + .map_or(String::new(), |ty| format!(" for type `{}`", ty)), + if overlap.used_to_be_allowed { + " (E0119)" + } else { + "" + } ); - let mut err = if used_to_be_allowed { + let impl_span = tcx.sess + .codemap() + .def_span(tcx.span_of_impl(impl_def_id).unwrap()); + let mut err = if overlap.used_to_be_allowed { tcx.struct_span_lint_node( lint::builtin::INCOHERENT_FUNDAMENTAL_IMPLS, tcx.hir.as_local_node_id(impl_def_id).unwrap(), impl_span, - &msg) + &msg, + ) } else { - struct_span_err!(tcx.sess, - impl_span, - E0119, - "{}", - msg) + struct_span_err!(tcx.sess, impl_span, E0119, "{}", msg) }; match tcx.span_of_impl(overlap.with_impl) { Ok(span) => { - err.span_label(tcx.sess.codemap().def_span(span), - format!("first implementation here")); - err.span_label(impl_span, - format!("conflicting implementation{}", - overlap.self_desc - .map_or(String::new(), - |ty| format!(" for `{}`", ty)))); + err.span_label( + tcx.sess.codemap().def_span(span), + format!("first implementation here"), + ); + err.span_label( + impl_span, + format!( + "conflicting implementation{}", + overlap + .self_desc + .map_or(String::new(), |ty| format!(" for `{}`", ty)) + ), + ); } Err(cname) => { let msg = match to_pretty_impl_header(tcx, overlap.with_impl) { - Some(s) => format!( - "conflicting implementation in crate `{}`:\n- {}", cname, s), + Some(s) => { + format!("conflicting implementation in crate `{}`:\n- {}", cname, s) + } None => format!("conflicting implementation in crate `{}`", cname), }; err.note(&msg); @@ -386,9 +413,6 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx 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) } } diff --git a/src/librustc/traits/specialize/specialization_graph.rs b/src/librustc/traits/specialize/specialization_graph.rs index e56a8662f3eb4..7d9dfa8d79626 100644 --- a/src/librustc/traits/specialize/specialization_graph.rs +++ b/src/librustc/traits/specialize/specialization_graph.rs @@ -14,13 +14,17 @@ use hir::def_id::DefId; use ich::{self, StableHashingContext}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult}; -use traits; +use rustc_data_structures::graph::{Graph as DataStructuresGraph, NodeIndex, INCOMING}; +use traits::{self, ObligationCause, TraitEngine}; use ty::{self, TyCtxt, TypeFoldable}; use ty::fast_reject::{self, SimplifiedType}; use rustc_data_structures::sync::Lrc; -use syntax::ast::Name; +use syntax::ast::{Name, DUMMY_NODE_ID}; use util::captures::Captures; use util::nodemap::{DefIdMap, FxHashMap}; +use super::FulfillmentContext; +use std::collections::HashSet; +use syntax_pos::DUMMY_SP; /// A per-trait graph of impls in specialization order. At the moment, this /// graph forms a tree rooted with the trait itself, with all other nodes @@ -39,9 +43,9 @@ use util::nodemap::{DefIdMap, FxHashMap}; /// has at most one parent. #[derive(RustcEncodable, RustcDecodable)] pub struct Graph { - // all impls have a parent; the "root" impls have as their parent the def_id + // all impls have one or more parents; the "root" impls have as their parent the def_id // of the trait - parent: DefIdMap, + parent: DefIdMap>, // the "root" impls are found by looking up the trait's def_id. children: DefIdMap, @@ -60,7 +64,6 @@ struct Children { // A similar division is used within `TraitDef`, but the lists there collect // together *all* the impls for a trait, and are populated prior to building // the specialization graph. - /// Impls of the trait. nonblanket_impls: FxHashMap>, @@ -106,17 +109,22 @@ impl<'a, 'gcx, 'tcx> Children { tcx: TyCtxt<'a, 'gcx, 'tcx>, impl_def_id: DefId, simplified_self: Option) - -> Result + -> Result, OverlapError> { let mut last_lint = None; + // Nodes could specialize more than one parent + // so every impl must visited in order to properly place it + let mut inserted_results = Vec::new(); + for slot in match simplified_self { Some(sty) => self.filtered_mut(sty), None => self.iter_mut(), } { let possible_sibling = *slot; - let overlap_error = |overlap: traits::coherence::OverlapResult| { + let overlap_error = |overlap: traits::coherence::OverlapResult, + used_to_be_allowed: bool| { // overlap, but no specialization; error out let trait_ref = overlap.impl_header.trait_ref.unwrap(); let self_ty = trait_ref.self_ty(); @@ -132,30 +140,31 @@ impl<'a, 'gcx, 'tcx> Children { None }, intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes, + used_to_be_allowed: used_to_be_allowed, } }; let tcx = tcx.global_tcx(); - let (le, ge) = traits::overlapping_impls( + let (le, ge, overlap_er) = traits::overlapping_impls( tcx, possible_sibling, impl_def_id, traits::IntercrateMode::Issue43355, |overlap| { if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) { - return Ok((false, false)); + return Ok((false, false, None)); } let le = tcx.specializes((impl_def_id, possible_sibling)); let ge = tcx.specializes((possible_sibling, impl_def_id)); if le == ge { - Err(overlap_error(overlap)) + Ok((true, true, Some(overlap_error(overlap, false)))) } else { - Ok((le, ge)) + Ok((le, ge, None)) } }, - || Ok((false, false)), + || Ok((false, false, None)), )?; if le && !ge { @@ -163,14 +172,16 @@ impl<'a, 'gcx, 'tcx> Children { tcx.impl_trait_ref(possible_sibling).unwrap()); // the impl specializes possible_sibling - return Ok(Inserted::ShouldRecurseOn(possible_sibling)); + inserted_results.push(Inserted::ShouldRecurseOn(possible_sibling)); } else if ge && !le { debug!("placing as parent of TraitRef {:?}", tcx.impl_trait_ref(possible_sibling).unwrap()); - // possible_sibling specializes the impl - *slot = impl_def_id; - return Ok(Inserted::Replaced(possible_sibling)); + // possible_sibling specializes the impl + *slot = impl_def_id; + inserted_results.push(Inserted::Replaced(possible_sibling)); + } else if ge && le { + last_lint = overlap_er; } else { if !tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) { traits::overlapping_impls( @@ -178,7 +189,7 @@ impl<'a, 'gcx, 'tcx> Children { possible_sibling, impl_def_id, traits::IntercrateMode::Fixed, - |overlap| last_lint = Some(overlap_error(overlap)), + |overlap| last_lint = Some(overlap_error(overlap, true)), || (), ); } @@ -187,10 +198,14 @@ impl<'a, 'gcx, 'tcx> Children { } } + if inserted_results.len() > 0 { + return Ok(inserted_results); + } + // no overlap with any potential siblings, so add as a new sibling debug!("placing as new sibling"); self.insert_blindly(tcx, impl_def_id); - Ok(Inserted::BecameNewSibling(last_lint)) + Ok(vec![Inserted::BecameNewSibling(last_lint)]) } fn iter_mut(&'a mut self) -> Box + 'a> { @@ -219,7 +234,7 @@ impl<'a, 'gcx, 'tcx> Graph { pub fn insert(&mut self, tcx: TyCtxt<'a, 'gcx, 'tcx>, impl_def_id: DefId) - -> Result, OverlapError> { + -> Result>, OverlapError> { assert!(impl_def_id.is_local()); let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); @@ -237,43 +252,72 @@ impl<'a, 'gcx, 'tcx> Graph { impl_def_id={:?}, trait_def_id={:?}", trait_ref, impl_def_id, trait_def_id); - self.parent.insert(impl_def_id, trait_def_id); + self.parent_insert(impl_def_id, trait_def_id); self.children.entry(trait_def_id).or_insert(Children::new()) .insert_blindly(tcx, impl_def_id); return Ok(None); } - let mut parent = trait_def_id; - let mut last_lint = None; + let parent = trait_def_id; let simplified = fast_reject::simplify_type(tcx, trait_ref.self_ty(), false); - // Descend the specialization tree, where `parent` is the current parent node - loop { - use self::Inserted::*; - - let insert_result = self.children.entry(parent).or_insert(Children::new()) - .insert(tcx, impl_def_id, simplified)?; + let mut last_lints = vec![]; + // Recusively descend the specialization tree, where `parent` is the current parent node + self.recursive_insert(parent, tcx, impl_def_id, simplified, &mut last_lints); + Ok(Some(last_lints)) + } - match insert_result { - BecameNewSibling(opt_lint) => { - last_lint = opt_lint; - break; - } - Replaced(new_child) => { - self.parent.insert(new_child, impl_def_id); - let mut new_children = Children::new(); - new_children.insert_blindly(tcx, new_child); - self.children.insert(impl_def_id, new_children); - break; - } - ShouldRecurseOn(new_parent) => { - parent = new_parent; + fn recursive_insert( + &mut self, + parent: DefId, + tcx: TyCtxt<'a, 'gcx, 'tcx>, + impl_def_id: DefId, + simplified: Option, + last_lints: &mut Vec, + ) { + use self::Inserted::*; + match self.children + .entry(parent) + .or_insert(Children::new()) + .insert(tcx, impl_def_id, simplified) + { + Ok(insert_results) => for insert_result in insert_results { + match insert_result { + BecameNewSibling(opt_lint) => { + if opt_lint.is_some() { + last_lints.push(opt_lint.unwrap()); + } + self.parent_insert(impl_def_id, parent); + } + Replaced(new_child) => { + self.parent_insert(new_child, impl_def_id); + let mut new_children = Children::new(); + new_children.insert_blindly(tcx, new_child); + self.children.insert(impl_def_id, new_children); + self.parent_insert(impl_def_id, parent); + } + ShouldRecurseOn(new_parent) => { + self.recursive_insert(new_parent, tcx, impl_def_id, simplified, last_lints); + } } - } + }, + _ => {} } + } - self.parent.insert(impl_def_id, parent); - Ok(last_lint) + fn parent_insert(&mut self, key: DefId, value: DefId) -> Option { + if self.parent.contains_key(&key) { + let mut impl_vec = self.parent.get(&key).unwrap().clone(); + impl_vec.push(value); + self.parent.insert(key, impl_vec); + Some(value) + } else { + if self.parent.insert(key, vec![value]).is_some() { + Some(value) + } else { + None + } + } } /// Insert cached metadata mapping from a child impl back to its parent. @@ -281,18 +325,135 @@ impl<'a, 'gcx, 'tcx> Graph { tcx: TyCtxt<'a, 'gcx, 'tcx>, parent: DefId, child: DefId) { - if self.parent.insert(child, parent).is_some() { - bug!("When recording an impl from the crate store, information about its parent \ - was already present."); + if self.parent_insert(child, parent).is_some() { + bug!( + "When recording an impl from the crate store, information about its parent \ + was already present." + ); } self.children.entry(parent).or_insert(Children::new()).insert_blindly(tcx, child); } - /// The parent of a given impl, which is the def id of the trait when the - /// impl is a "specialization root". - pub fn parent(&self, child: DefId) -> DefId { - *self.parent.get(&child).unwrap() + /// Returns the def-id of the parent impl(s) for a given impl. + /// An impl A has a parent impl B if A matches a strict subset of the types that B matches. + pub fn parents(&self, child: DefId) -> Vec { + self.parent.get(&child).unwrap().clone() + } + + pub fn build_graph(&self) -> (DataStructuresGraph, DefIdMap) { + let mut sg_graph: DataStructuresGraph = DataStructuresGraph::new(); + let mut nodes_idx = Default::default(); + for (key, val) in self.parent.iter() { + let idx = self.node_idx(&mut sg_graph, &mut nodes_idx, *key); + for parent in val { + let pidx = self.node_idx(&mut sg_graph, &mut nodes_idx, *parent); + sg_graph.add_edge(idx, pidx, format!("fromt {:?} to {:?}", key, parent)); + debug!("from {:?} to {:?}", key, parent); + } + } + + (sg_graph, nodes_idx) + } + + /// Return true if impl1 and impl2 are allowed to overlap: + /// They have an intersection impl + pub fn check_overlap( + &self, + sg_graph: &DataStructuresGraph, + nodes_idx: &DefIdMap, + impl1: DefId, + impl2: DefId, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + ) -> bool { + + let impl1_idx = *nodes_idx.get(&impl1).unwrap(); + let impl2_idx = *nodes_idx.get(&impl2).unwrap(); + + // two impls are allowed to overlap if they have a mutual postdominator in the graph. + // That postdominator is the specializing impl. + let impl1_incoming_nodes = sg_graph.nodes_in_postorder(INCOMING, impl1_idx); + let impl2_incoming_nodes = sg_graph.nodes_in_postorder(INCOMING, impl2_idx); + + if impl1_incoming_nodes[0] == impl2_incoming_nodes[0] { + + // a mutual postdominator has been found but the intersection impls + // could not be complete. + // The query below tries to answer to the following + // Does the exist a set of types such that + // - impl A applies and impl B applies + // - but impl C does not apply + let mut result = true; + let int_impl = sg_graph.node_data(impl1_incoming_nodes[0]); + tcx.infer_ctxt().enter(|infcx| { + + let param_env1 = tcx.param_env(impl1); + let param_env2 = tcx.param_env(impl2); + + let mut combined_param_envs_vec = + param_env1 + .caller_bounds + .iter() + .chain(param_env2.caller_bounds.iter()) + .map(|p| *p) + .collect::>(); + + let combined_param_envs: HashSet<_> = + combined_param_envs_vec + .drain(..) + .collect(); + + // Combine the param envs of the overlapping impls into a single param env. + let param_env = ty::ParamEnv::new( + tcx.intern_predicates( + combined_param_envs + .into_iter() + .collect::>() + .as_slice() + ), + param_env1.reveal + ); + + let predicates = tcx.predicates_of(*int_impl); + let ty = tcx.type_of(*int_impl); + + let mut fulfill_cx = FulfillmentContext::new(); + for predicate in predicates.predicates { + if let ty::Predicate::Trait(trait_predicate) = predicate { + fulfill_cx.register_bound( + &infcx, + param_env, + ty, + trait_predicate.skip_binder().trait_ref.def_id, + ObligationCause::misc(DUMMY_SP, DUMMY_NODE_ID), + ); + } + } + + fulfill_cx.select_all_or_error(&infcx).unwrap_or_else(|_| { + result = false; + }); + }); + + result + } else { + false + } + } + + fn node_idx( + &self, + sg_graph: &mut DataStructuresGraph, + nodes_idx: &mut DefIdMap, + node: DefId, + ) -> NodeIndex { + if nodes_idx.get(&node).is_some() { + *nodes_idx.get(&node).unwrap() + } else { + let idx = sg_graph.add_node(node); + nodes_idx.insert(node, idx); + idx + } } } @@ -332,22 +493,43 @@ impl<'a, 'gcx, 'tcx> Node { pub struct Ancestors { trait_def_id: DefId, specialization_graph: Lrc, - current_source: Option, + current_source: Option>, } impl Iterator for Ancestors { type Item = Node; fn next(&mut self) -> Option { + // Visit and return the graph nodes from bottom to top + // When multiple parents are found return each one of them + // prior to move up in the graph let cur = self.current_source.take(); - if let Some(Node::Impl(cur_impl)) = cur { - 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)); + match cur { + Some(mut cur_vec) => { + let next_value = cur_vec[0]; + if let Node::Impl(cur_impl) = next_value { + let parents = self.specialization_graph.parents(cur_impl); + + let mut ncur = vec![]; + ncur.append(&mut cur_vec[1..].to_vec()); + for parent in parents { + let node = if parent == self.trait_def_id { + Node::Trait(parent) + } else { + Node::Impl(parent) + }; + + if ncur.iter().find(|n| n.def_id() == node.def_id()).is_none() { + ncur.push(node); + } + } + + self.current_source = Some(ncur); + } + + Some(next_value) } + None => None, } - cur } } @@ -395,7 +577,7 @@ pub fn ancestors(tcx: TyCtxt, Ancestors { trait_def_id, specialization_graph, - current_source: Some(Node::Impl(start_from_impl)), + current_source: Some(vec![Node::Impl(start_from_impl)]), } } diff --git a/src/test/compile-fail/coherence-conflicting-negative-trait-impl.rs b/src/test/compile-fail/coherence-conflicting-negative-trait-impl.rs index 8e9d1eff34580..80e61d7b9b43a 100644 --- a/src/test/compile-fail/coherence-conflicting-negative-trait-impl.rs +++ b/src/test/compile-fail/coherence-conflicting-negative-trait-impl.rs @@ -21,6 +21,7 @@ impl !Send for TestType {} //~^ ERROR conflicting implementations of trait `std::marker::Send` unsafe impl Send for TestType {} +//~^ ERROR conflicting implementations of trait `std::marker::Send` impl !Send for TestType {} //~^ ERROR conflicting implementations of trait `std::marker::Send` diff --git a/src/test/compile-fail/specialization/intersection-impl-not-complete.rs b/src/test/compile-fail/specialization/intersection-impl-not-complete.rs new file mode 100644 index 0000000000000..59b774c6168ea --- /dev/null +++ b/src/test/compile-fail/specialization/intersection-impl-not-complete.rs @@ -0,0 +1,30 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(specialization)] + +// test if the intersecion impl is complete + +trait A { } +trait B { } +trait C { } + +trait MyTrait { } + +impl MyTrait for T { } +impl MyTrait for T { } +//~^ ERROR conflicting implementations of trait `MyTrait` + +// This would be OK: +//impl MyTrait for T { } +// But what about this: +impl MyTrait for T { } + +fn main() {} diff --git a/src/test/compile-fail/specialization/intersection-impl.rs b/src/test/compile-fail/specialization/intersection-impl.rs new file mode 100644 index 0000000000000..de1df63abf9af --- /dev/null +++ b/src/test/compile-fail/specialization/intersection-impl.rs @@ -0,0 +1,36 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(specialization)] + +// test conflicting implementation error +// should an intersection impl help be shown? + +trait MyClone { + fn clone(&self) -> Self; +} + +impl MyClone for T { + fn clone(&self) -> T { + *self + } +} + +impl MyClone for Option { + fn clone(&self) -> Option { + match *self { + Some(ref v) => Some(v.clone()), + None => None, + } + } +} +//~^^^^^^^^ ERROR conflicting implementations of trait `MyClone` for type `std::option::Option<_>` + +fn main() {} diff --git a/src/test/run-pass/specialization/intersection-impl.rs b/src/test/run-pass/specialization/intersection-impl.rs new file mode 100644 index 0000000000000..eb9f0cb643d5a --- /dev/null +++ b/src/test/run-pass/specialization/intersection-impl.rs @@ -0,0 +1,42 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(specialization)] + +// Test if two impls are allowed to overlap if a third +// impl is the intersection of them + +trait MyClone { + fn my_clone(&self) -> &'static str; +} + +impl MyClone for T { + default fn my_clone(&self) -> &'static str { + "impl_a" + } +} + +impl MyClone for Option { + default fn my_clone(&self) -> &'static str { + "impl_b" + } +} + +impl MyClone for Option { + fn my_clone(&self) -> &'static str { + "impl_c" + } +} + +fn main() { + assert!(42i32.my_clone() == "impl_a"); + assert!(Some(Box::new(42i32)).my_clone() == "impl_b"); + assert!(Some(42i32).my_clone() == "impl_c"); +} diff --git a/src/test/ui/e0119/issue-28981.stderr b/src/test/ui/e0119/issue-28981.stderr index afcbab4a5c6c0..9d600fcac07cf 100644 --- a/src/test/ui/e0119/issue-28981.stderr +++ b/src/test/ui/e0119/issue-28981.stderr @@ -1,11 +1,11 @@ -error[E0119]: conflicting implementations of trait `std::ops::Deref` for type `&_`: +error[E0119]: conflicting implementations of trait `std::ops::Deref` for type `std::cell::Ref<'_, _>`: --> $DIR/issue-28981.rs:15:1 | LL | impl Deref for Foo { } //~ ERROR must be used | ^^^^^^^^^^^^^^^^^^^^^^^ | = note: conflicting implementation in crate `core`: - - impl<'a, T> std::ops::Deref for &'a T + - impl<'b, T> std::ops::Deref for std::cell::Ref<'b, T> where T: ?Sized; error[E0210]: type parameter `Foo` must be used as the type parameter for some local type (e.g. `MyStruct`)