Auto merge of #51042 - matthewjasper:reenable-trivial-bounds, r=nikomatsakis

Re-enable trivial bounds

cc #50825

Remove implementations from global bounds in winnowing when there is ambiguity.

This results in the reverse of #24066 happening sometimes. I'm not sure if anything can be done about that though.

cc #48214

r? @nikomatsakis

Author: bors

src/librustc/traits/mod.rs

@@ -648,13 +648,8 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
     let predicates: Vec<_> =
         util::elaborate_predicates(tcx, unnormalized_env.caller_bounds.to_vec())
-        .filter(|p| !p.is_global() || p.has_late_bound_regions()) // (*)
         .collect();
 
-    // (*) FIXME(#50825) This shouldn't be needed.
-    // Removing the bounds here stopped them from being prefered in selection.
-    // See the issue-50825 ui tests for examples
-
     debug!("normalize_param_env_or_error: elaborated-predicates={:?}",
            predicates);
 

src/librustc/traits/select.rs

@@ -2011,9 +2011,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
     // attempt to evaluate recursive bounds to see if they are
     // satisfied.
 
-    /// Returns true if `candidate_i` should be dropped in favor of
-    /// `candidate_j`.  Generally speaking we will drop duplicate
-    /// candidates and prefer where-clause candidates.
     /// Returns true if `victim` should be dropped in favor of
     /// `other`.  Generally speaking we will drop duplicate
     /// candidates and prefer where-clause candidates.
@@ -2025,13 +2022,46 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         other: &EvaluatedCandidate<'tcx>)
         -> bool
     {
+        // Check if a bound would previously have been removed when normalizing
+        // the param_env so that it can be given the lowest priority. See
+        // #50825 for the motivation for this.
+        let is_global = |cand: &ty::PolyTraitRef<'_>| {
+            cand.is_global() && !cand.has_late_bound_regions()
+        };
+
         if victim.candidate == other.candidate {
             return true;
         }
 
         match other.candidate {
+            ParamCandidate(ref cand) => match victim.candidate {
+                AutoImplCandidate(..) => {
+                    bug!(
+                        "default implementations shouldn't be recorded \
+                         when there are other valid candidates");
+                }
+                ImplCandidate(..) |
+                ClosureCandidate |
+                GeneratorCandidate |
+                FnPointerCandidate |
+                BuiltinObjectCandidate |
+                BuiltinUnsizeCandidate |
+                BuiltinCandidate { .. } => {
+                    // Global bounds from the where clause should be ignored
+                    // here (see issue #50825). Otherwise, we have a where
+                    // clause so don't go around looking for impls.
+                    !is_global(cand)
+                }
+                ObjectCandidate |
+                ProjectionCandidate => {
+                    // Arbitrarily give param candidates priority
+                    // over projection and object candidates.
+                    !is_global(cand)
+                },
+                ParamCandidate(..) => false,
+            },
             ObjectCandidate |
-            ParamCandidate(_) | ProjectionCandidate => match victim.candidate {
+            ProjectionCandidate => match victim.candidate {
                 AutoImplCandidate(..) => {
                     bug!(
                         "default implementations shouldn't be recorded \
@@ -2044,8 +2074,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                 BuiltinObjectCandidate |
                 BuiltinUnsizeCandidate |
                 BuiltinCandidate { .. } => {
-                    // We have a where-clause so don't go around looking
-                    // for impls.
                     true
                 }
                 ObjectCandidate |
@@ -2054,22 +2082,44 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                     // over projection and object candidates.
                     true
                 },
-                ParamCandidate(..) => false,
+                ParamCandidate(ref cand) => is_global(cand),
             },
             ImplCandidate(other_def) => {
                 // See if we can toss out `victim` based on specialization.
                 // This requires us to know *for sure* that the `other` impl applies
                 // i.e. EvaluatedToOk:
                 if other.evaluation == EvaluatedToOk {
-                    if let ImplCandidate(victim_def) = victim.candidate {
-                        let tcx = self.tcx().global_tcx();
-                        return tcx.specializes((other_def, victim_def)) ||
-                            tcx.impls_are_allowed_to_overlap(other_def, victim_def);
+                    match victim.candidate {
+                        ImplCandidate(victim_def) => {
+                            let tcx = self.tcx().global_tcx();
+                            return tcx.specializes((other_def, victim_def)) ||
+                                tcx.impls_are_allowed_to_overlap(other_def, victim_def);
+                        }
+                        ParamCandidate(ref cand) => {
+                            // Prefer the impl to a global where clause candidate.
+                            return is_global(cand);
+                        }
+                        _ => ()
                     }
                 }
 
                 false
             },
+            ClosureCandidate |
+            GeneratorCandidate |
+            FnPointerCandidate |
+            BuiltinObjectCandidate |
+            BuiltinUnsizeCandidate |
+            BuiltinCandidate { .. } => {
+                match victim.candidate {
+                    ParamCandidate(ref cand) => {
+                        // Prefer these to a global where-clause bound
+                        // (see issue #50825)
+                        is_global(cand) && other.evaluation == EvaluatedToOk
+                    }
+                    _ => false,
+                }
+            }
             _ => false
         }
     }

src/librustc_typeck/check/wfcheck.rs

@@ -711,6 +711,7 @@ fn check_false_global_bounds<'a, 'gcx, 'tcx>(
     for pred in implied_obligations {
         // Match the existing behavior.
         if pred.is_global() && !pred.has_late_bound_regions() {
+            let pred = fcx.normalize_associated_types_in(span, &pred);
             let obligation = traits::Obligation::new(
                 traits::ObligationCause::new(
                     span,

src/test/ui/feature-gate-trivial_bounds.rs

@@ -28,7 +28,7 @@ union U where i32: Foo { f: i32 } //~ ERROR
 type Y where i32: Foo = (); // OK - bound is ignored
 
 impl Foo for () where i32: Foo { //~ ERROR
-    fn test(&self) { //~ ERROR
+    fn test(&self) {
         3i32.test();
         Foo::test(&4i32);
         generic_function(5i32);
@@ -60,7 +60,7 @@ struct Dst<X: ?Sized> {
 }
 
 struct TwoStrs(str, str) where str: Sized; //~ ERROR
-//~^ ERROR
+
 
 fn unsized_local() where Dst<A>: Sized { //~ ERROR
     let x: Dst<A> = *(Box::new(Dst { x: 1 }) as Box<Dst<A>>);

src/test/ui/feature-gate-trivial_bounds.stderr

@@ -38,7 +38,7 @@ error[E0277]: the trait bound `i32: Foo` is not satisfied
   --> $DIR/feature-gate-trivial_bounds.rs:30:1
    |
 LL | / impl Foo for () where i32: Foo { //~ ERROR
-LL | |     fn test(&self) { //~ ERROR
+LL | |     fn test(&self) {
 LL | |         3i32.test();
 LL | |         Foo::test(&4i32);
 LL | |         generic_function(5i32);
@@ -97,15 +97,6 @@ LL | struct TwoStrs(str, str) where str: Sized; //~ ERROR
    = help: see issue #48214
    = help: add #![feature(trivial_bounds)] to the crate attributes to enable
 
-error[E0277]: the trait bound `str: std::marker::Sized` is not satisfied
-  --> $DIR/feature-gate-trivial_bounds.rs:62:16
-   |
-LL | struct TwoStrs(str, str) where str: Sized; //~ ERROR
-   |                ^^^ `str` does not have a constant size known at compile-time
-   |
-   = help: the trait `std::marker::Sized` is not implemented for `str`
-   = note: only the last field of a struct may have a dynamically sized type
-
 error[E0277]: the trait bound `A + 'static: std::marker::Sized` is not satisfied in `Dst<A + 'static>`
   --> $DIR/feature-gate-trivial_bounds.rs:65:1
    |
@@ -131,22 +122,6 @@ LL | | }
    = help: see issue #48214
    = help: add #![feature(trivial_bounds)] to the crate attributes to enable
 
-error[E0277]: the trait bound `i32: Foo` is not satisfied
-  --> $DIR/feature-gate-trivial_bounds.rs:31:5
-   |
-LL | /     fn test(&self) { //~ ERROR
-LL | |         3i32.test();
-LL | |         Foo::test(&4i32);
-LL | |         generic_function(5i32);
-LL | |     }
-   | |_____^ the trait `Foo` is not implemented for `i32`
-   |
-note: required by `Foo`
-  --> $DIR/feature-gate-trivial_bounds.rs:14:1
-   |
-LL | pub trait Foo {
-   | ^^^^^^^^^^^^^
-
-error: aborting due to 13 previous errors
+error: aborting due to 11 previous errors
 
 For more information about this error, try `rustc --explain E0277`.

src/test/ui/issue-50825-1.rs

@@ -10,7 +10,7 @@
 
 // run-pass
 // regression test for issue #50825
-// Make sure that the `impl` bound (): X<T = ()> is prefered over
+// Make sure that the `impl` bound (): X<T = ()> is preferred over
 // the (): X bound in the where clause.
 
 trait X {

src/test/ui/issue-50825.rs

@@ -10,7 +10,7 @@
 
 // run-pass
 // regression test for issue #50825
-// Make sure that the built-in bound {integer}: Sized is prefered over
+// Make sure that the built-in bound {integer}: Sized is preferred over
 // the u64: Sized bound in the where clause.
 
 fn foo(y: &[()])

src/test/ui/issue-51044.rs

@@ -0,0 +1,40 @@
+// Copyright 2018 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.
+
+// run-pass
+// regression test for issue #50825
+// Check that the feature gate normalizes associated types.
+
+#![allow(dead_code)]
+struct Foo<T>(T);
+struct Duck;
+struct Quack;
+
+trait Hello<A> where A: Animal {
+}
+
+trait Animal {
+    type Noise;
+}
+
+trait Loud<R>  {
+}
+
+impl Loud<Quack> for f32 {
+}
+
+impl Animal for Duck {
+    type Noise = Quack;
+}
+
+impl Hello<Duck> for Foo<f32> where f32: Loud<<Duck as Animal>::Noise> {
+}
+
+fn main() {}

src/test/ui/trivial-bounds-inconsistent-associated-functions.rs

@@ -8,7 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// ignore-test FIXME(#50825)
 // run-pass
 // Inconsistent bounds with trait implementations
 

src/test/ui/trivial-bounds-inconsistent-copy-reborrow.rs

@@ -8,7 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// ignore-test FIXME(#50825)
 // Check that reborrows are still illegal with Copy mutable references
 #![feature(trivial_bounds)]
 #![allow(unused)]