From ba35e8053499b694498184905f1be11b727cdf72 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 8 Jun 2018 17:00:03 +0100 Subject: [PATCH] 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 | 5 -- src/librustc/traits/select.rs | 72 ++++++++++++++++--- src/test/ui/feature-gate-trivial_bounds.rs | 4 +- .../ui/feature-gate-trivial_bounds.stderr | 29 +------- ...ounds-inconsistent-associated-functions.rs | 1 - ...ivial-bounds-inconsistent-copy-reborrow.rs | 1 - .../ui/trivial-bounds-inconsistent-copy.rs | 1 - .../ui/trivial-bounds-inconsistent-sized.rs | 1 - ...trivial-bounds-inconsistent-well-formed.rs | 1 - src/test/ui/trivial-bounds-inconsistent.rs | 1 - src/test/ui/trivial-bounds-leak-copy.rs | 1 - src/test/ui/trivial-bounds-leak.rs | 1 - src/test/ui/trivial-bounds-lint.rs | 1 - 13 files changed, 65 insertions(+), 54 deletions(-) diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 3bd4e11b0e8..e284b3fc75a 100644 --- a/src/librustc/traits/mod.rs +++ b/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); diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 7a52a5cbf5a..08af80543df 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2011,9 +2011,6 @@ fn assemble_candidates_for_unsizing(&mut self, // 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 @@ fn candidate_should_be_dropped_in_favor_of<'o>( 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 @@ fn candidate_should_be_dropped_in_favor_of<'o>( BuiltinObjectCandidate | BuiltinUnsizeCandidate | BuiltinCandidate { .. } => { - // We have a where-clause so don't go around looking - // for impls. true } ObjectCandidate | @@ -2054,22 +2082,44 @@ fn candidate_should_be_dropped_in_favor_of<'o>( // 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 } } diff --git a/src/test/ui/feature-gate-trivial_bounds.rs b/src/test/ui/feature-gate-trivial_bounds.rs index e72b8782e50..dba66e0b69b 100644 --- a/src/test/ui/feature-gate-trivial_bounds.rs +++ b/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 { } struct TwoStrs(str, str) where str: Sized; //~ ERROR -//~^ ERROR + fn unsized_local() where Dst: Sized { //~ ERROR let x: Dst = *(Box::new(Dst { x: 1 }) as Box>); diff --git a/src/test/ui/feature-gate-trivial_bounds.stderr b/src/test/ui/feature-gate-trivial_bounds.stderr index 32b263119e5..9c2c80600b8 100644 --- a/src/test/ui/feature-gate-trivial_bounds.stderr +++ b/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` --> $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`. diff --git a/src/test/ui/trivial-bounds-inconsistent-associated-functions.rs b/src/test/ui/trivial-bounds-inconsistent-associated-functions.rs index 4cacbc2c914..49c9df95bc7 100644 --- a/src/test/ui/trivial-bounds-inconsistent-associated-functions.rs +++ b/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 diff --git a/src/test/ui/trivial-bounds-inconsistent-copy-reborrow.rs b/src/test/ui/trivial-bounds-inconsistent-copy-reborrow.rs index a743b429698..2c4d9d81385 100644 --- a/src/test/ui/trivial-bounds-inconsistent-copy-reborrow.rs +++ b/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)] diff --git a/src/test/ui/trivial-bounds-inconsistent-copy.rs b/src/test/ui/trivial-bounds-inconsistent-copy.rs index f73c96a002d..375885a02c7 100644 --- a/src/test/ui/trivial-bounds-inconsistent-copy.rs +++ b/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)] diff --git a/src/test/ui/trivial-bounds-inconsistent-sized.rs b/src/test/ui/trivial-bounds-inconsistent-sized.rs index 11f0080fbab..14ba11c44de 100644 --- a/src/test/ui/trivial-bounds-inconsistent-sized.rs +++ b/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)] diff --git a/src/test/ui/trivial-bounds-inconsistent-well-formed.rs b/src/test/ui/trivial-bounds-inconsistent-well-formed.rs index a78ecdc8ff3..5fcdbfc437a 100644 --- a/src/test/ui/trivial-bounds-inconsistent-well-formed.rs +++ b/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)] diff --git a/src/test/ui/trivial-bounds-inconsistent.rs b/src/test/ui/trivial-bounds-inconsistent.rs index c8e8c320bc5..2c8b873b8c9 100644 --- a/src/test/ui/trivial-bounds-inconsistent.rs +++ b/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 diff --git a/src/test/ui/trivial-bounds-leak-copy.rs b/src/test/ui/trivial-bounds-leak-copy.rs index 6f000006ca9..9850ec2bd1f 100644 --- a/src/test/ui/trivial-bounds-leak-copy.rs +++ b/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)] diff --git a/src/test/ui/trivial-bounds-leak.rs b/src/test/ui/trivial-bounds-leak.rs index 15dee64f70e..98cb5b2b503 100644 --- a/src/test/ui/trivial-bounds-leak.rs +++ b/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)] diff --git a/src/test/ui/trivial-bounds-lint.rs b/src/test/ui/trivial-bounds-lint.rs index e37600a653a..e6988cb9f8b 100644 --- a/src/test/ui/trivial-bounds-lint.rs +++ b/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)] -- 2.44.0