Check WF of predicates with defaults only if all params have defaults

5 files changed, 64 insertions, 54 deletions

diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index 2c538d2bba5..3ab2cd274b9 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -1040,13 +1040,6 @@ impl<'a, 'gcx, 'tcx> Predicate<'tcx> {
                 Predicate::ConstEvaluatable(def_id, const_substs.subst(tcx, substs)),
         }
     }
-
-    pub fn as_poly_trait_predicate(&self) -> Option<&PolyTraitPredicate<'tcx>> {
-        match self {
-            Predicate::Trait(trait_pred) => Some(trait_pred),
-            _ => None
-        }
-    }
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs
index 463cc2aa6b3..16115fa9be4 100644
--- a/src/librustc_typeck/check/wfcheck.rs
+++ b/src/librustc_typeck/check/wfcheck.rs
@@ -392,14 +392,13 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
         // Check that trait predicates are WF when params are substituted by their defaults.
         // We don't want to overly constrain the predicates that may be written but we
         // want to catch obviously wrong cases such as `struct Foo<T: Copy = String>`
-        // or cases that may cause backwards incompatibility such as a library going from
-        // `pub struct Foo<T>` to `pub struct Foo<T, U = i32>` where U: Trait<T>`
-        // which may break existing uses of Foo<T>.
-        // Therefore the check we do is: If if all params appearing in the LHS of the predicate
-        // have defaults then we verify that it is WF with all defaults substituted simultaneously.
+        // or cases where defaults don't work together such as:
+        // `struct Foo<T = i32, U = u8> where T: into<U>`
+        // Therefore we check if a predicate in which all type params are defaulted
+        // is WF with those defaults simultaneously substituted.
         // For more examples see tests `defaults-well-formedness.rs` and `type-check-defaults.rs`.
         //
-        // First, we build the defaulted substitution.
+        // First we build the defaulted substitution.
         let mut defaulted_params = Vec::new();
         let substs = ty::subst::Substs::for_item(fcx.tcx, def_id, |def, _| {
                 // All regions are identity.
@@ -414,33 +413,35 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
                     fcx.tcx.type_of(def.def_id)
                 }
             });
-        // In `trait Trait: Super`, checking `Self: Trait` or `Self: Super` is problematic.
-        // We avoid those by skipping any predicates in trait declarations that contain `Self`,
-        // which is excessive so we end up checking less than we could.
-        for pred in predicates.predicates.iter()
-                                         .filter_map(ty::Predicate::as_poly_trait_predicate)
-                                         .filter(|p| !(is_trait && p.has_self_ty())) {
-            let is_defaulted_param = |ty: ty::Ty| match ty.sty {
-                                            ty::TyParam(p) => defaulted_params.contains(&p.idx),
-                                            _ => false
-                                          };
-            // If there is a non-defaulted param in the LHS, don't check the substituted predicate.
-            // `skip_binder()` is ok, we're only inspecting the type params.
-            if !pred.skip_binder().self_ty().walk().all(is_defaulted_param) {
+        let defaulted_params = &defaulted_params;
+        // Now we build the substituted predicates.
+        for &pred in predicates.predicates.iter() {
+            struct HasNonDefaulted<'a> { defaulted_params: &'a Vec<u32> }
+            impl<'tcx, 'a> ty::fold::TypeVisitor<'tcx> for HasNonDefaulted<'a> {
+                fn visit_ty(&mut self, t: Ty<'tcx>) -> bool {
+                    match t.sty {
+                        ty::TyParam(p) => !self.defaulted_params.contains(&p.idx),
+                        _ => t.super_visit_with(self)
+                    }
+                }
+            }
+            // If there is a non-defaulted param in the predicate, don't check it.
+            if pred.visit_with(&mut HasNonDefaulted { defaulted_params }) {
                 continue;
             }
             let substituted_pred = pred.subst(fcx.tcx, substs);
-            // `skip_binder()` is ok, we're only inspecting for `has_self_ty()`.
-            let substituted_lhs = substituted_pred.skip_binder().self_ty();
             // In trait defs, don't check `Self: Sized` when `Self` is the default.
-            let pred_is_sized = Some(pred.def_id()) == fcx.tcx.lang_items().sized_trait();
-            if is_trait && substituted_lhs.has_self_ty() && pred_is_sized {
-                continue;
+            if let ty::Predicate::Trait(trait_pred) = substituted_pred {
+                // `skip_binder()` is ok, we're only inspecting for `has_self_ty()`.
+                let lhs_is_self = trait_pred.skip_binder().self_ty().has_self_ty();
+                let pred_sized = Some(trait_pred.def_id()) == fcx.tcx.lang_items().sized_trait();
+                if is_trait && lhs_is_self && pred_sized {
+                    continue;
+                }
             }
-            let pred = ty::Predicate::Trait(pred.subst(fcx.tcx, substs));
-            // Avoid duplicates.
-            if !predicates.predicates.contains(&pred) {
-                substituted_predicates.push(pred);
+            // Avoid duplication of predicates that contain no parameters, for example.
+            if !predicates.predicates.contains(&substituted_pred) {
+                substituted_predicates.push(substituted_pred);
             }
         }
 
diff --git a/src/test/run-pass/defaults-well-formedness.rs b/src/test/run-pass/defaults-well-formedness.rs
index abbd35b3909..6a7b1c51ccd 100644
--- a/src/test/run-pass/defaults-well-formedness.rs
+++ b/src/test/run-pass/defaults-well-formedness.rs
@@ -11,11 +11,13 @@
 trait Trait<T> {}
 struct Foo<U, V=i32>(U, V) where U: Trait<V>;
 
-trait Trait2 {}
+trait Marker {}
 struct TwoParams<T, U>(T, U);
-impl Trait2 for TwoParams<i32, i32> {}
+impl Marker for TwoParams<i32, i32> {}
 // Check that defaults are substituted simultaneously.
-struct IndividuallyBogus<T = i32, U = i32>(TwoParams<T, U>) where TwoParams<T, U>: Trait2;
+struct IndividuallyBogus<T = i32, U = i32>(TwoParams<T, U>) where TwoParams<T, U>: Marker;
 // Clauses with non-defaulted params are not checked.
-struct NonDefaultedInClause<T, U = i32>(TwoParams<T, U>) where TwoParams<T, U>: Trait2;
+struct NonDefaultedInClause<T, U = i32>(TwoParams<T, U>) where TwoParams<T, U>: Marker;
+struct DefaultedLhs<U, V=i32>(U, V) where V: Trait<U>;
+
 fn main() {}
diff --git a/src/test/ui/type-check-defaults.rs b/src/test/ui/type-check-defaults.rs
index cc20f81ae16..ff3fb44bb26 100644
--- a/src/test/ui/type-check-defaults.rs
+++ b/src/test/ui/type-check-defaults.rs
@@ -37,11 +37,14 @@ trait Super<T: Copy> { }
 trait Base<T = String>: Super<T> { }
 //~^ error: the trait bound `T: std::marker::Copy` is not satisfied [E0277]
 
-trait Trait<T> {}
-struct DefaultedLhs<U, V=i32>(U, V) where V: Trait<U>;
-//~^ error: the trait bound `i32: Trait<U>` is not satisfied [E0277]
+trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
+//~^ error: the trait bound `i32: std::ops::Add<u8>` is not satisfied [E0277]
+
+// Defaults must work together.
+struct TwoParams<T = u32, U = i32>(T, U) where T: Bar<U>;
+//~^ the trait bound `u32: Bar<i32>` is not satisfied [E0277]
+trait Bar<V> {}
+impl Bar<String> for u32 { }
+impl Bar<i32> for String { }
 
-// FIXME: Deal with projection predicates
-// trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
-// ~^ error: the trait bound `i32: std::ops::Add<u8>` is not satisfied [E0277]
 fn main() { }
diff --git a/src/test/ui/type-check-defaults.stderr b/src/test/ui/type-check-defaults.stderr
index ede1b417c7c..412a7966f0a 100644
--- a/src/test/ui/type-check-defaults.stderr
+++ b/src/test/ui/type-check-defaults.stderr
@@ -79,17 +79,28 @@ note: required by `Super`
 36 | trait Super<T: Copy> { }
    | ^^^^^^^^^^^^^^^^^^^^
 
-error[E0277]: the trait bound `i32: Trait<U>` is not satisfied
-  --> $DIR/type-check-defaults.rs:41:1
+error[E0277]: the trait bound `i32: std::ops::Add<u8>` is not satisfied
+  --> $DIR/type-check-defaults.rs:40:1
    |
-41 | struct DefaultedLhs<U, V=i32>(U, V) where V: Trait<U>;
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Trait<U>` is not implemented for `i32`
+40 | trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `i32 + u8`
    |
-note: required by `Trait`
-  --> $DIR/type-check-defaults.rs:40:1
+   = help: the trait `std::ops::Add<u8>` is not implemented for `i32`
+   = note: required by `std::ops::Add`
+
+error[E0277]: the trait bound `u32: Bar<i32>` is not satisfied
+  --> $DIR/type-check-defaults.rs:44:1
+   |
+44 | struct TwoParams<T = u32, U = i32>(T, U) where T: Bar<U>;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Bar<i32>` is not implemented for `u32`
+   |
+   = help: the following implementations were found:
+             <u32 as Bar<std::string::String>>
+note: required by `Bar`
+  --> $DIR/type-check-defaults.rs:46:1
    |
-40 | trait Trait<T> {}
-   | ^^^^^^^^^^^^^^
+46 | trait Bar<V> {}
+   | ^^^^^^^^^^^^
 
-error: aborting due to 9 previous errors
+error: aborting due to 10 previous errors