Reenable trivial bounds

Removes extra global bounds at the winnowing stage rather than when
normalizing the param_env. This avoids breaking inference when there is
a global bound.

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 {
-            ObjectCandidate |
+            ParamCandidate(ref cand) => match victim.candidate {
-            ParamCandidate(_) | ProjectionCandidate => 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 |
+            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 {
+                    match victim.candidate {
-                        let tcx = self.tcx().global_tcx();
+                        ImplCandidate(victim_def) => {
-                        return tcx.specializes((other_def, victim_def)) ||
+                            let tcx = self.tcx().global_tcx();
-                            tcx.impls_are_allowed_to_overlap(other_def, victim_def);
+                            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/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
+error: aborting due to 11 previous errors
-  --> $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
 
 For more information about this error, try `rustc --explain E0277`.

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)]

src/test/ui/trivial-bounds-inconsistent-copy.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
 // Check tautalogically false `Copy` bounds
 #![feature(trivial_bounds)]

src/test/ui/trivial-bounds-inconsistent-sized.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
 // Check tautalogically false `Sized` bounds
 #![feature(trivial_bounds)]

src/test/ui/trivial-bounds-inconsistent-well-formed.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
 // Test that inconsistent bounds are used in well-formedness checks
 #![feature(trivial_bounds)]

src/test/ui/trivial-bounds-inconsistent.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
 
 // Check that tautalogically false bounds are accepted, and are used

src/test/ui/trivial-bounds-leak-copy.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 false Copy bounds don't leak
 #![feature(trivial_bounds)]
 

src/test/ui/trivial-bounds-leak.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 false bounds don't leak
 #![feature(trivial_bounds)]
 

src/test/ui/trivial-bounds-lint.rs

@@ -8,7 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// ignore-test FIXME(#50825)
 #![feature(trivial_bounds)]
 #![allow(unused)]
 #![deny(trivial_bounds)]