Address basic nits from initial review

Author: aturon

src/librustc/middle/traits/specialize.rs

@@ -52,10 +52,9 @@ pub struct SpecializationGraph {
 }
 
 /// Information pertinent to an overlapping impl error.
-pub struct Overlap<'tcx> {
+pub struct Overlap<'a, 'tcx: 'a> {
+    pub in_context: InferCtxt<'a, 'tcx>,
     pub with_impl: DefId,
-
-    /// NB: this TraitRef can contain inference variables!
     pub on_trait_ref: ty::TraitRef<'tcx>,
 }
 
@@ -70,13 +69,13 @@ impl SpecializationGraph {
     /// Insert a local impl into the specialization graph. If an existing impl
     /// conflicts with it (has overlap, but neither specializes the other),
     /// information about the area of overlap is returned in the `Err`.
-    pub fn insert<'tcx>(&mut self,
-                        tcx: &ty::ctxt<'tcx>,
-                        impl_def_id: DefId,
-                        trait_ref: ty::TraitRef)
-                        -> Result<(), Overlap<'tcx>> {
+    pub fn insert<'a, 'tcx>(&mut self,
+                            tcx: &'a ty::ctxt<'tcx>,
+                            impl_def_id: DefId)
+                            -> Result<(), Overlap<'a, 'tcx>> {
         assert!(impl_def_id.is_local());
 
+        let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
         let mut parent = trait_ref.def_id;
         let mut my_children = vec![];
 
@@ -98,6 +97,7 @@ impl SpecializationGraph {
                         return Err(Overlap {
                             with_impl: possible_sibling,
                             on_trait_ref: trait_ref,
+                            in_context: infcx,
                         });
                     }
 
@@ -118,6 +118,7 @@ impl SpecializationGraph {
                         return Err(Overlap {
                             with_impl: possible_sibling,
                             on_trait_ref: trait_ref,
+                            in_context: infcx,
                         });
                     }
 

src/librustc/middle/ty/trait_def.rs

@@ -125,7 +125,8 @@ impl<'tcx> TraitDef<'tcx> {
     fn record_impl(&self,
                    tcx: &TyCtxt<'tcx>,
                    impl_def_id: DefId,
-                   impl_trait_ref: TraitRef<'tcx>) -> bool {
+                   impl_trait_ref: TraitRef<'tcx>)
+                   -> bool {
         debug!("TraitDef::record_impl for {:?}, from {:?}",
                self, impl_trait_ref);
 
@@ -161,7 +162,9 @@ impl<'tcx> TraitDef<'tcx> {
                              tcx: &TyCtxt<'tcx>,
                              impl_def_id: DefId,
                              impl_trait_ref: TraitRef<'tcx>) {
-        self.record_impl(tcx, impl_def_id, impl_trait_ref);
+        assert!(impl_def_id.is_local());
+        let was_new = self.record_impl(tcx, impl_def_id, impl_trait_ref);
+        assert!(was_new);
     }
 
     /// Records a trait-to-implementation mapping for a non-local impl.
@@ -174,6 +177,8 @@ impl<'tcx> TraitDef<'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
@@ -186,15 +191,14 @@ impl<'tcx> TraitDef<'tcx> {
     /// 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: &ctxt<'tcx>,
-                                       impl_def_id: DefId,
-                                       impl_trait_ref: TraitRef<'tcx>)
-                                       -> Result<(), traits::Overlap<'tcx>> {
+    pub fn add_impl_for_specialization<'a>(&self,
+                                           tcx: &'a ctxt<'tcx>,
+                                           impl_def_id: DefId)
+                                           -> Result<(), traits::Overlap<'a, 'tcx>> {
         assert!(impl_def_id.is_local());
 
         self.specialization_graph.borrow_mut()
-            .insert(tcx, impl_def_id, impl_trait_ref)
+            .insert(tcx, impl_def_id)
     }
 
     /// Returns the immediately less specialized impl, if any.

src/librustc_typeck/coherence/overlap.rs

@@ -133,9 +133,7 @@ impl<'cx, 'tcx,'v> intravisit::Visitor<'v> for OverlapChecker<'cx, 'tcx> {
                 let def = self.tcx.lookup_trait_def(trait_def_id);
 
                 // attempt to insert into the specialization graph
-                let insert_result = def.add_impl_for_specialization(self.tcx,
-                                                                    impl_def_id,
-                                                                    trait_ref);
+                let insert_result = def.add_impl_for_specialization(self.tcx, impl_def_id);
 
                 // insertion failed due to overlap
                 if let Err(overlap) = insert_result {

src/libsyntax/feature_gate.rs

@@ -250,6 +250,7 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Option<u32>, Status
     ("question_mark", "1.9.0", Some(31436), Active)
 
     // impl specialization (RFC 1210)
+    // TODO: update with issue number (once it exists), before landing
     ("specialization", "1.7.0", None, Active),
 ];
 // (changing above list without updating src/doc/reference.md makes @cmr sad)

src/test/compile-fail/specialization-negative-impl.rs

@@ -13,6 +13,8 @@
 
 struct TestType<T>(T);
 
+// TODO: nail down the rules here with @nikomatsakis
+
 unsafe impl<T> Send for TestType<T> {}
 impl !Send for TestType<u8> {}
 

src/test/compile-fail/specialization-no-default.rs

@@ -10,6 +10,10 @@
 
 #![feature(specialization)]
 
+////////////////////////////////////////////////////////////////////////////////
+// Test 1: one layer of specialization, multiple methods, missing `default`
+////////////////////////////////////////////////////////////////////////////////
+
 trait Foo {
     fn foo(&self);
     fn bar(&self);
@@ -28,6 +32,10 @@ impl Foo for u32 {
     fn bar(&self) {} //~ ERROR E0520
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// Test 2: one layer of specialization, missing `default` on associated type
+////////////////////////////////////////////////////////////////////////////////
+
 trait Bar {
     type T;
 }
@@ -40,4 +48,45 @@ impl Bar for u8 {
     type T = (); //~ ERROR E0520
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// Test 3a: multiple layers of specialization, missing interior `default`
+////////////////////////////////////////////////////////////////////////////////
+
+trait Baz {
+    fn baz(&self);
+}
+
+impl<T> Baz for T {
+    default fn baz(&self) {}
+}
+
+impl<T: Clone> Baz for T {
+    fn baz(&self) {}
+}
+
+impl Baz for i32 {
+    fn baz(&self) {}
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 3b: multiple layers of specialization, missing interior `default`,
+// redundant `default` in bottom layer.
+////////////////////////////////////////////////////////////////////////////////
+
+trait Redundant {
+    fn redundant(&self);
+}
+
+impl<T> Redundant for T {
+    default fn redundant(&self) {}
+}
+
+impl<T: Clone> Redundant for T {
+    fn redundant(&self) {}
+}
+
+impl Redundant for i32 {
+    default fn redundant(&self) {}
+}
+
 fn main() {}

src/test/run-pass/specialization-default-methods.rs

@@ -16,6 +16,12 @@ trait Foo {
     fn foo(&self) -> bool;
 }
 
+// Specialization tree for Foo:
+//
+//        T
+//       / \
+//    i32   i64
+
 impl<T> Foo for T {
     default fn foo(&self) -> bool { false }
 }
@@ -38,6 +44,23 @@ trait Bar {
     fn bar(&self) -> i32 { 0 }
 }
 
+// Specialization tree for Bar.
+// Uses of $ designate that method is provided
+//
+//           $Bar   (the trait)
+//             |
+//             T
+//            /|\
+//           / | \
+//          /  |  \
+//         /   |   \
+//        /    |    \
+//       /     |     \
+//     $i32   &str  $Vec<T>
+//                    /\
+//                   /  \
+//            Vec<i32>  $Vec<i64>
+
 impl<T> Bar for T {} // use the provided method
 
 impl Bar for i32 {