Coherence for default trait implementations

- Don't allow multiple default trait implementations
- Allow positive trait implementations just for structs and enums when
  there's a default implementation for such trait.

Author: flaper87

src/librustc/middle/traits/select.rs

@@ -1626,6 +1626,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 Vec::new()
             }
 
+            ty::ty_trait(..) |
             ty::ty_projection(..) |
             ty::ty_param(..) |
             ty::ty_infer(..) => {
@@ -1639,18 +1640,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 vec![referent_ty]
             }
 
-
-            ty::ty_trait(ref data) => {
-                // Recursively check all supertraits to find out if any further
-                // bounds are required and thus we must fulfill.
-                let principal =
-                    data.principal_trait_ref_with_self_ty(self.tcx(),
-                                                          self.tcx().types.err);
-
-
-                util::supertraits(self.tcx(), principal).map(|tr| tr.self_ty()).collect()
-            }
-
             ty::ty_open(element_ty) => {vec![element_ty]},
 
             ty::ty_ptr(ty::mt { ty: element_ty, ..}) |

src/librustc/middle/ty.rs

@@ -758,7 +758,7 @@ pub struct ctxt<'tcx> {
     pub trait_impls: RefCell<DefIdMap<Rc<RefCell<Vec<ast::DefId>>>>>,
 
     /// Maps a trait onto a list of *default* trait implementations
-    default_trait_impls: RefCell<DefIdSet>,
+    default_trait_impls: RefCell<DefIdMap<ast::DefId>>,
 
     /// Maps a DefId of a type to a list of its inherent impls.
     /// Contains implementations of methods that are inherent to a type.
@@ -2496,7 +2496,7 @@ pub fn mk_ctxt<'tcx>(s: Session,
         destructor_for_type: RefCell::new(DefIdMap()),
         destructors: RefCell::new(DefIdSet()),
         trait_impls: RefCell::new(DefIdMap()),
-        default_trait_impls: RefCell::new(DefIdSet()),
+        default_trait_impls: RefCell::new(DefIdMap()),
         inherent_impls: RefCell::new(DefIdMap()),
         impl_items: RefCell::new(DefIdMap()),
         used_unsafe: RefCell::new(NodeSet()),
@@ -5999,18 +5999,26 @@ pub fn item_variances(tcx: &ctxt, item_id: ast::DefId) -> Rc<ItemVariances> {
         || Rc::new(csearch::get_item_variances(&tcx.sess.cstore, item_id)))
 }
 
+pub fn trait_default_impl(tcx: &ctxt, trait_def_id: DefId) -> Option<ast::DefId> {
+    match tcx.default_trait_impls.borrow().get(&trait_def_id) {
+        Some(id) => Some(*id),
+        None => None
+    }
+}
+
 pub fn trait_has_default_impl(tcx: &ctxt, trait_def_id: DefId) -> bool {
-    tcx.default_trait_impls.borrow().contains(&trait_def_id)
+    tcx.default_trait_impls.borrow().contains_key(&trait_def_id)
 }
 
 /// Records a trait-to-implementation mapping.
-pub fn record_default_trait_implementation(tcx: &ctxt, trait_def_id: DefId) {
-
-    if tcx.default_trait_impls.borrow().contains(&trait_def_id) {
-        return;
-    }
+pub fn record_default_trait_implementation(tcx: &ctxt,
+                                           trait_def_id: DefId,
+                                           impl_def_id: DefId) {
 
-    tcx.default_trait_impls.borrow_mut().insert(trait_def_id);
+    // We're using the latest implementation found as the reference one.
+    // Duplicated implementations are caught and reported in the coherence
+    // step.
+    tcx.default_trait_impls.borrow_mut().insert(trait_def_id, impl_def_id);
 }
 
 
@@ -6100,7 +6108,8 @@ pub fn populate_implementations_for_trait_if_necessary(
         let impl_items = csearch::get_impl_items(&tcx.sess.cstore, implementation_def_id);
 
         if csearch::is_default_trait(&tcx.sess.cstore, implementation_def_id) {
-            record_default_trait_implementation(tcx, trait_id);
+            record_default_trait_implementation(tcx, trait_id,
+                                                implementation_def_id);
             tcx.populated_external_traits.borrow_mut().insert(trait_id);
 
             // Nothing else to do for default trait implementations since

src/librustc_typeck/coherence/overlap.rs

@@ -17,16 +17,19 @@ use middle::infer::{self, new_infer_ctxt};
 use syntax::ast::{DefId};
 use syntax::ast::{LOCAL_CRATE};
 use syntax::ast;
-use syntax::codemap::{Span};
+use syntax::ast_util;
+use syntax::visit;
+use syntax::codemap::Span;
 use util::ppaux::Repr;
 
 pub fn check(tcx: &ty::ctxt) {
-    let overlap = OverlapChecker { tcx: tcx };
+    let mut overlap = OverlapChecker { tcx: tcx };
     overlap.check_for_overlapping_impls();
+    visit::walk_crate(&mut overlap, tcx.map.krate());
 }
 
 struct OverlapChecker<'cx, 'tcx:'cx> {
-    tcx: &'cx ty::ctxt<'tcx>
+    tcx: &'cx ty::ctxt<'tcx>,
 }
 
 impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> {
@@ -90,17 +93,28 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> {
             return;
         }
 
-        span_err!(self.tcx.sess, self.span_of_impl(impl1_def_id), E0119,
+        self.report_overlap_error(trait_def_id, impl1_def_id, impl2_def_id);
+    }
+
+    fn report_overlap_error(&self, trait_def_id: ast::DefId,
+                            impl1: ast::DefId, impl2: ast::DefId) {
+
+        span_err!(self.tcx.sess, self.span_of_impl(impl1), E0119,
                   "conflicting implementations for trait `{}`",
                   ty::item_path_str(self.tcx, trait_def_id));
 
-        if impl2_def_id.krate == ast::LOCAL_CRATE {
-            span_note!(self.tcx.sess, self.span_of_impl(impl2_def_id),
+        self.report_overlap_note(impl1, impl2);
+    }
+
+    fn report_overlap_note(&self, impl1: ast::DefId, impl2: ast::DefId) {
+
+        if impl2.krate == ast::LOCAL_CRATE {
+            span_note!(self.tcx.sess, self.span_of_impl(impl2),
                        "note conflicting implementation here");
         } else {
             let crate_store = &self.tcx.sess.cstore;
-            let cdata = crate_store.get_crate_data(impl2_def_id.krate);
-            span_note!(self.tcx.sess, self.span_of_impl(impl1_def_id),
+            let cdata = crate_store.get_crate_data(impl2.krate);
+            span_note!(self.tcx.sess, self.span_of_impl(impl1),
                        "conflicting implementation in crate `{}`",
                        cdata.name);
         }
@@ -111,3 +125,52 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> {
         self.tcx.map.span(impl_did.node)
     }
 }
+
+
+impl<'cx, 'tcx,'v> visit::Visitor<'v> for OverlapChecker<'cx, 'tcx> {
+    fn visit_item(&mut self, item: &'v ast::Item) {
+        match item.node {
+            ast::ItemImpl(_, ast::ImplPolarity::Positive, _, Some(ref ast_trait_ref), _, _) => {
+                let trait_ref = ty::node_id_to_trait_ref(self.tcx, ast_trait_ref.ref_id);
+                match ty::trait_default_impl(self.tcx, trait_ref.def_id) {
+                    Some(default_impl) => {
+                        match trait_ref.self_ty().sty {
+                            ty::ty_struct(..) | ty::ty_enum(..) => {},
+                            _ => {
+                                let impl_def_id = ast_util::local_def(item.id);
+                                span_err!(self.tcx.sess, self.span_of_impl(impl_def_id), E0317,
+                                          "implementations for traits providing default \
+                                           implementations are only allowed on structs and enums");
+
+                                self.report_overlap_note(impl_def_id, default_impl);
+                            }
+                        }
+                    }
+                    None => {}
+                }
+            }
+            ast::ItemDefaultImpl(_, _) => {
+                let impl_def_id = ast_util::local_def(item.id);
+                match ty::impl_trait_ref(self.tcx, impl_def_id) {
+                    Some(ref trait_ref) => {
+                        match ty::trait_default_impl(self.tcx, trait_ref.def_id) {
+                            Some(other_impl) if other_impl != impl_def_id => {
+                                self.report_overlap_error(trait_ref.def_id,
+                                                          other_impl,
+                                                          impl_def_id);
+                            }
+                            Some(_) => {}
+                            None => {
+                                self.tcx.sess.bug(
+                                          &format!("no default implementation recorded for `{:?}`",
+                                          item)[]);
+                            }
+                        }
+                    }
+                    _ => {}
+                }
+            }
+            _ => {}
+        }
+    }
+}

src/librustc_typeck/collect.rs

@@ -652,7 +652,7 @@ fn convert_item(ccx: &CollectCtxt, it: &ast::Item) {
             let trait_ref = astconv::instantiate_trait_ref(ccx, &ExplicitRscope,
                                                            ast_trait_ref, None, None);
 
-            ty::record_default_trait_implementation(tcx, trait_ref.def_id)
+            ty::record_default_trait_implementation(tcx, trait_ref.def_id, local_def(it.id))
         }
         ast::ItemImpl(_, _,
                       ref generics,
@@ -1190,6 +1190,7 @@ fn convert_typed_item<'a, 'tcx>(ccx: &CollectCtxt<'a, 'tcx>,
         ast::ItemStruct(_, ref generics) => {
             ty_generic_bounds_for_type_or_impl(ccx, &scheme.generics, generics)
         }
+        ast::ItemDefaultImpl(..) |
         ast::ItemTrait(..) |
         ast::ItemExternCrate(..) |
         ast::ItemUse(..) |

src/librustc_typeck/diagnostics.rs

@@ -172,7 +172,8 @@ register_diagnostics! {
     E0248, // found value name used as a type
     E0249, // expected constant expr for array length
     E0250, // expected constant expr for array length
-    E0316  // can't create default impls for traits outside their crates
+    E0316,  // can't create default impls for traits outside their crates
+    E0317
 }
 
 __build_diagnostic_array! { DIAGNOSTICS }