Auto merge of #46785 - leodasvacas:type-check-defaults-at-declaration, r=nikomatsakis

[Underspecified semantics] Type check defaults at declaration.

Fixes  #46669. See the test for code that compiles on stable but will no longer compile. This falls under a "Underspecified language semantics" fix. **Needs crater**.

On type and trait declarations, we currently allow anything that name checks as a type parameter default. That allows the user to write a default that can never be applied, or even a default that may conditionally be applied depending on the type of another parameter. Mostly this just defers the error to use sites, but also allows clever hacks such as `Foo<T, U = <T as Iterator>::Item>` where `U` will be able to apply it's default only when `T: Iterator`. Maybe that means this bug is a feature, but it's a fiddly behaviour that seems undesirable.

This PR validates defaults at declaration sites by ensuring all predicates on the parameter are valid for the default. With the exception of `Self: Sized` which we don't want to check to allow things like `trait Add<RHS = Self>`.

5 files changed, 231 insertions, 23 deletions

diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs
index 3668fc46ddc..39e757c52ff 100644
--- a/src/librustc_typeck/check/wfcheck.rs
+++ b/src/librustc_typeck/check/wfcheck.rs
@@ -185,10 +185,7 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
                     reject_shadowing_type_parameters(fcx.tcx, item.def_id);
                     let sig = fcx.tcx.fn_sig(item.def_id);
                     let sig = fcx.normalize_associated_types_in(span, &sig);
-                    let predicates = fcx.tcx.predicates_of(item.def_id)
-                        .instantiate_identity(fcx.tcx);
-                    let predicates = fcx.normalize_associated_types_in(span, &predicates);
-                    this.check_fn_or_method(fcx, span, sig, &predicates,
+                    this.check_fn_or_method(fcx, span, sig,
                                             item.def_id, &mut implied_bounds);
                     let sig_if_method = sig_if_method.expect("bad signature for method");
                     this.check_method_receiver(fcx, sig_if_method, &item, self_ty);
@@ -272,9 +269,7 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
                 }
             }
 
-            let predicates = fcx.tcx.predicates_of(def_id).instantiate_identity(fcx.tcx);
-            let predicates = fcx.normalize_associated_types_in(item.span, &predicates);
-            this.check_where_clauses(fcx, item.span, &predicates);
+            self.check_where_clauses(fcx, item.span, def_id);
 
             vec![] // no implied bounds in a struct def'n
         });
@@ -282,10 +277,8 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
 
     fn check_trait(&mut self, item: &hir::Item) {
         let trait_def_id = self.tcx.hir.local_def_id(item.id);
-        self.for_item(item).with_fcx(|fcx, this| {
-            let predicates = fcx.tcx.predicates_of(trait_def_id).instantiate_identity(fcx.tcx);
-            let predicates = fcx.normalize_associated_types_in(item.span, &predicates);
-            this.check_where_clauses(fcx, item.span, &predicates);
+        self.for_item(item).with_fcx(|fcx, _| {
+            self.check_where_clauses(fcx, item.span, trait_def_id);
             vec![]
         });
     }
@@ -295,12 +288,8 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
             let def_id = fcx.tcx.hir.local_def_id(item.id);
             let sig = fcx.tcx.fn_sig(def_id);
             let sig = fcx.normalize_associated_types_in(item.span, &sig);
-
-            let predicates = fcx.tcx.predicates_of(def_id).instantiate_identity(fcx.tcx);
-            let predicates = fcx.normalize_associated_types_in(item.span, &predicates);
-
             let mut implied_bounds = vec![];
-            this.check_fn_or_method(fcx, item.span, sig, &predicates,
+            this.check_fn_or_method(fcx, item.span, sig,
                                     def_id, &mut implied_bounds);
             implied_bounds
         })
@@ -354,19 +343,96 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
                 }
             }
 
-            let predicates = fcx.tcx.predicates_of(item_def_id).instantiate_identity(fcx.tcx);
-            let predicates = fcx.normalize_associated_types_in(item.span, &predicates);
-            this.check_where_clauses(fcx, item.span, &predicates);
+            this.check_where_clauses(fcx, item.span, item_def_id);
 
             fcx.impl_implied_bounds(item_def_id, item.span)
         });
     }
 
+    /// Checks where clauses and inline bounds that are declared on def_id.
     fn check_where_clauses<'fcx, 'tcx>(&mut self,
                                        fcx: &FnCtxt<'fcx, 'gcx, 'tcx>,
                                        span: Span,
-                                       predicates: &ty::InstantiatedPredicates<'tcx>)
-    {
+                                       def_id: DefId) {
+        use ty::subst::Subst;
+        use rustc::ty::TypeFoldable;
+
+        let mut predicates = fcx.tcx.predicates_of(def_id);
+        let mut substituted_predicates = Vec::new();
+
+        let generics = self.tcx.generics_of(def_id);
+        let is_our_default = |def: &ty::TypeParameterDef|
+                                def.has_default && def.index >= generics.parent_count() as u32;
+
+        // Check that concrete defaults are well-formed. See test `type-check-defaults.rs`.
+        // For example this forbids the declaration:
+        // struct Foo<T = Vec<[u32]>> { .. }
+        // Here the default `Vec<[u32]>` is not WF because `[u32]: Sized` does not hold.
+        for d in generics.types.iter().cloned().filter(is_our_default).map(|p| p.def_id) {
+            let ty = fcx.tcx.type_of(d);
+            // ignore dependent defaults -- that is, where the default of one type
+            // parameter includes another (e.g., <T, U = T>). In those cases, we can't
+            // be sure if it will error or not as user might always specify the other.
+            if !ty.needs_subst() {
+                fcx.register_wf_obligation(ty, fcx.tcx.def_span(d), self.code.clone());
+            }
+        }
+
+        // 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 cases where a default my never be applied such as `struct Foo<T: Copy = String>`.
+        // Therefore we check if a predicate which contains a single type param
+        // with a concrete default is WF with that default substituted.
+        // For more examples see tests `defaults-well-formedness.rs` and `type-check-defaults.rs`.
+        //
+        // First we build the defaulted substitution.
+        let substs = ty::subst::Substs::for_item(fcx.tcx, def_id, |def, _| {
+                // All regions are identity.
+                fcx.tcx.mk_region(ty::ReEarlyBound(def.to_early_bound_region_data()))
+            }, |def, _| {
+                // If the param has a default,
+                if is_our_default(def) {
+                    let default_ty = fcx.tcx.type_of(def.def_id);
+                    // and it's not a dependent default
+                    if !default_ty.needs_subst() {
+                        // then substitute with the default.
+                        return default_ty;
+                    }
+                }
+                // Mark unwanted params as err.
+                fcx.tcx.types.err
+            });
+        // Now we build the substituted predicates.
+        for &pred in predicates.predicates.iter() {
+            struct CountParams { params: FxHashSet<u32> }
+            impl<'tcx> ty::fold::TypeVisitor<'tcx> for CountParams {
+                fn visit_ty(&mut self, t: Ty<'tcx>) -> bool {
+                    match t.sty {
+                        ty::TyParam(p) => {
+                            self.params.insert(p.idx);
+                            t.super_visit_with(self)
+                        }
+                        _ => t.super_visit_with(self)
+                    }
+                }
+            }
+            let mut param_count = CountParams { params: FxHashSet() };
+            pred.visit_with(&mut param_count);
+            let substituted_pred = pred.subst(fcx.tcx, substs);
+            // Don't check non-defaulted params, dependent defaults or preds with multiple params.
+            if substituted_pred.references_error() || param_count.params.len() > 1 {
+                continue;
+            }
+            // Avoid duplication of predicates that contain no parameters, for example.
+            if !predicates.predicates.contains(&substituted_pred) {
+                substituted_predicates.push(substituted_pred);
+            }
+        }
+
+        predicates.predicates.extend(substituted_predicates);
+        let predicates = predicates.instantiate_identity(fcx.tcx);
+        let predicates = fcx.normalize_associated_types_in(span, &predicates);
+
         let obligations =
             predicates.predicates
                       .iter()
@@ -385,7 +451,6 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
                                       fcx: &FnCtxt<'fcx, 'gcx, 'tcx>,
                                       span: Span,
                                       sig: ty::PolyFnSig<'tcx>,
-                                      predicates: &ty::InstantiatedPredicates<'tcx>,
                                       def_id: DefId,
                                       implied_bounds: &mut Vec<Ty<'tcx>>)
     {
@@ -402,7 +467,7 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
         // FIXME(#25759) return types should not be implied bounds
         implied_bounds.push(sig.output());
 
-        self.check_where_clauses(fcx, span, predicates);
+        self.check_where_clauses(fcx, span, def_id);
     }
 
     fn check_method_receiver<'fcx, 'tcx>(&mut self,
diff --git a/src/test/run-pass/defaults-well-formedness.rs b/src/test/run-pass/defaults-well-formedness.rs
new file mode 100644
index 00000000000..f3594679095
--- /dev/null
+++ b/src/test/run-pass/defaults-well-formedness.rs
@@ -0,0 +1,30 @@
+// 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+trait Trait<T> {}
+struct Foo<U, V=i32>(U, V) where U: Trait<V>;
+
+trait Marker {}
+struct TwoParams<T, U>(T, U);
+impl Marker for TwoParams<i32, i32> {}
+
+// Clauses with more than 1 param are not checked.
+struct IndividuallyBogus<T = i32, U = i32>(TwoParams<T, U>) where TwoParams<T, U>: Marker;
+struct BogusTogether<T = u32, U = i32>(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>: Marker;
+struct DefaultedLhs<U, V=i32>(U, V) where V: Trait<U>;
+// Dependent defaults are not checked.
+struct Dependent<T, U = T>(T, U) where U: Copy;
+trait SelfBound<T: Copy=Self> {}
+// Not even for well-formedness.
+struct WellFormedProjection<A, T=<A as Iterator>::Item>(A, T);
+
+fn main() {}
diff --git a/src/test/run-pass/type-macros-simple.rs b/src/test/run-pass/type-macros-simple.rs
index c61133f795b..67b1d2c06e3 100644
--- a/src/test/run-pass/type-macros-simple.rs
+++ b/src/test/run-pass/type-macros-simple.rs
@@ -34,3 +34,4 @@ fn issue_36540() {
 }
 
 trait Trait<T> {}
+impl Trait<i32> for i32 {}
diff --git a/src/test/ui/type-check-defaults.rs b/src/test/ui/type-check-defaults.rs
new file mode 100644
index 00000000000..f916df5d32d
--- /dev/null
+++ b/src/test/ui/type-check-defaults.rs
@@ -0,0 +1,37 @@
+// 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::iter::FromIterator;
+use std::vec::IntoIter;
+use std::ops::Add;
+
+struct Foo<T, U: FromIterator<T>>(T, U);
+struct WellFormed<Z = Foo<i32, i32>>(Z);
+//~^ error: the trait bound `i32: std::iter::FromIterator<i32>` is not satisfied [E0277]
+struct WellFormedNoBounds<Z:?Sized = Foo<i32, i32>>(Z);
+//~^ error: the trait bound `i32: std::iter::FromIterator<i32>` is not satisfied [E0277]
+
+struct Bounds<T:Copy=String>(T);
+//~^ error: the trait bound `std::string::String: std::marker::Copy` is not satisfied [E0277]
+
+struct WhereClause<T=String>(T) where T: Copy;
+//~^ error: the trait bound `std::string::String: std::marker::Copy` is not satisfied [E0277]
+
+trait TraitBound<T:Copy=String> {}
+//~^ error: the trait bound `std::string::String: std::marker::Copy` is not satisfied [E0277]
+
+trait Super<T: Copy> { }
+trait Base<T = String>: Super<T> { }
+//~^ error: the trait bound `T: std::marker::Copy` is not satisfied [E0277]
+
+trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
+//~^ error:  cannot add `u8` to `i32` [E0277]
+
+fn main() { }
diff --git a/src/test/ui/type-check-defaults.stderr b/src/test/ui/type-check-defaults.stderr
new file mode 100644
index 00000000000..f39b7dcb31f
--- /dev/null
+++ b/src/test/ui/type-check-defaults.stderr
@@ -0,0 +1,75 @@
+error[E0277]: the trait bound `i32: std::iter::FromIterator<i32>` is not satisfied
+  --> $DIR/type-check-defaults.rs:16:19
+   |
+LL | struct WellFormed<Z = Foo<i32, i32>>(Z);
+   |                   ^ a collection of type `i32` cannot be built from an iterator over elements of type `i32`
+   |
+   = help: the trait `std::iter::FromIterator<i32>` is not implemented for `i32`
+note: required by `Foo`
+  --> $DIR/type-check-defaults.rs:15:1
+   |
+LL | struct Foo<T, U: FromIterator<T>>(T, U);
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error[E0277]: the trait bound `i32: std::iter::FromIterator<i32>` is not satisfied
+  --> $DIR/type-check-defaults.rs:18:27
+   |
+LL | struct WellFormedNoBounds<Z:?Sized = Foo<i32, i32>>(Z);
+   |                           ^ a collection of type `i32` cannot be built from an iterator over elements of type `i32`
+   |
+   = help: the trait `std::iter::FromIterator<i32>` is not implemented for `i32`
+note: required by `Foo`
+  --> $DIR/type-check-defaults.rs:15:1
+   |
+LL | struct Foo<T, U: FromIterator<T>>(T, U);
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
+  --> $DIR/type-check-defaults.rs:21:1
+   |
+LL | struct Bounds<T:Copy=String>(T);
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
+   |
+   = note: required by `std::marker::Copy`
+
+error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
+  --> $DIR/type-check-defaults.rs:24:1
+   |
+LL | struct WhereClause<T=String>(T) where T: Copy;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
+   |
+   = note: required by `std::marker::Copy`
+
+error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
+  --> $DIR/type-check-defaults.rs:27:1
+   |
+LL | trait TraitBound<T:Copy=String> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
+   |
+   = note: required by `std::marker::Copy`
+
+error[E0277]: the trait bound `T: std::marker::Copy` is not satisfied
+  --> $DIR/type-check-defaults.rs:31:1
+   |
+LL | trait Base<T = String>: Super<T> { }
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `T`
+   |
+   = help: consider adding a `where T: std::marker::Copy` bound
+note: required by `Super`
+  --> $DIR/type-check-defaults.rs:30:1
+   |
+LL | trait Super<T: Copy> { }
+   | ^^^^^^^^^^^^^^^^^^^^
+
+error[E0277]: cannot add `u8` to `i32`
+  --> $DIR/type-check-defaults.rs:34:1
+   |
+LL | trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `i32 + u8`
+   |
+   = help: the trait `std::ops::Add<u8>` is not implemented for `i32`
+   = note: required by `std::ops::Add`
+
+error: aborting due to 7 previous errors
+
+If you want more information on this error, try using "rustc --explain E0277"