Skip to content

Commit 1c5626f

Browse files
borsbors
committed
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
2 parents cf91e9b + a1bddcf commit 1c5626f

22 files changed

+302
-56
lines changed

‎src/librustc/traits/mod.rs

-5
Original file line numberDiff line numberDiff line change
@@ -648,13 +648,8 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
648648

649649
let predicates: Vec<_> =
650650
util::elaborate_predicates(tcx, unnormalized_env.caller_bounds.to_vec())
651-
.filter(|p| !p.is_global() || p.has_late_bound_regions()) // (*)
652651
.collect();
653652

654-
// (*) FIXME(#50825) This shouldn't be needed.
655-
// Removing the bounds here stopped them from being prefered in selection.
656-
// See the issue-50825 ui tests for examples
657-
658653
debug!("normalize_param_env_or_error: elaborated-predicates={:?}",
659654
predicates);
660655

‎src/librustc/traits/select.rs

+61-11
Original file line numberDiff line numberDiff line change
@@ -2011,9 +2011,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
20112011
// attempt to evaluate recursive bounds to see if they are
20122012
// satisfied.
20132013

2014-
/// Returns true if `candidate_i` should be dropped in favor of
2015-
/// `candidate_j`. Generally speaking we will drop duplicate
2016-
/// candidates and prefer where-clause candidates.
20172014
/// Returns true if `victim` should be dropped in favor of
20182015
/// `other`. Generally speaking we will drop duplicate
20192016
/// candidates and prefer where-clause candidates.
@@ -2025,13 +2022,46 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
20252022
other: &EvaluatedCandidate<'tcx>)
20262023
-> bool
20272024
{
2025+
// Check if a bound would previously have been removed when normalizing
2026+
// the param_env so that it can be given the lowest priority. See
2027+
// #50825 for the motivation for this.
2028+
let is_global = |cand: &ty::PolyTraitRef<'_>| {
2029+
cand.is_global() && !cand.has_late_bound_regions()
2030+
};
2031+
20282032
if victim.candidate == other.candidate {
20292033
return true;
20302034
}
20312035

20322036
match other.candidate {
2037+
ParamCandidate(ref cand) => match victim.candidate {
2038+
AutoImplCandidate(..) => {
2039+
bug!(
2040+
"default implementations shouldn't be recorded \
2041+
when there are other valid candidates");
2042+
}
2043+
ImplCandidate(..) |
2044+
ClosureCandidate |
2045+
GeneratorCandidate |
2046+
FnPointerCandidate |
2047+
BuiltinObjectCandidate |
2048+
BuiltinUnsizeCandidate |
2049+
BuiltinCandidate { .. } => {
2050+
// Global bounds from the where clause should be ignored
2051+
// here (see issue #50825). Otherwise, we have a where
2052+
// clause so don't go around looking for impls.
2053+
!is_global(cand)
2054+
}
2055+
ObjectCandidate |
2056+
ProjectionCandidate => {
2057+
// Arbitrarily give param candidates priority
2058+
// over projection and object candidates.
2059+
!is_global(cand)
2060+
},
2061+
ParamCandidate(..) => false,
2062+
},
20332063
ObjectCandidate |
2034-
ParamCandidate(_) | ProjectionCandidate => match victim.candidate {
2064+
ProjectionCandidate => match victim.candidate {
20352065
AutoImplCandidate(..) => {
20362066
bug!(
20372067
"default implementations shouldn't be recorded \
@@ -2044,8 +2074,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
20442074
BuiltinObjectCandidate |
20452075
BuiltinUnsizeCandidate |
20462076
BuiltinCandidate { .. } => {
2047-
// We have a where-clause so don't go around looking
2048-
// for impls.
20492077
true
20502078
}
20512079
ObjectCandidate |
@@ -2054,22 +2082,44 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
20542082
// over projection and object candidates.
20552083
true
20562084
},
2057-
ParamCandidate(..) => false,
2085+
ParamCandidate(ref cand) => is_global(cand),
20582086
},
20592087
ImplCandidate(other_def) => {
20602088
// See if we can toss out `victim` based on specialization.
20612089
// This requires us to know *for sure* that the `other` impl applies
20622090
// i.e. EvaluatedToOk:
20632091
if other.evaluation == EvaluatedToOk {
2064-
if let ImplCandidate(victim_def) = victim.candidate {
2065-
let tcx = self.tcx().global_tcx();
2066-
return tcx.specializes((other_def, victim_def)) ||
2067-
tcx.impls_are_allowed_to_overlap(other_def, victim_def);
2092+
match victim.candidate {
2093+
ImplCandidate(victim_def) => {
2094+
let tcx = self.tcx().global_tcx();
2095+
return tcx.specializes((other_def, victim_def)) ||
2096+
tcx.impls_are_allowed_to_overlap(other_def, victim_def);
2097+
}
2098+
ParamCandidate(ref cand) => {
2099+
// Prefer the impl to a global where clause candidate.
2100+
return is_global(cand);
2101+
}
2102+
_ => ()
20682103
}
20692104
}
20702105

20712106
false
20722107
},
2108+
ClosureCandidate |
2109+
GeneratorCandidate |
2110+
FnPointerCandidate |
2111+
BuiltinObjectCandidate |
2112+
BuiltinUnsizeCandidate |
2113+
BuiltinCandidate { .. } => {
2114+
match victim.candidate {
2115+
ParamCandidate(ref cand) => {
2116+
// Prefer these to a global where-clause bound
2117+
// (see issue #50825)
2118+
is_global(cand) && other.evaluation == EvaluatedToOk
2119+
}
2120+
_ => false,
2121+
}
2122+
}
20732123
_ => false
20742124
}
20752125
}

‎src/librustc_typeck/check/wfcheck.rs

+1
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,7 @@ fn check_false_global_bounds<'a, 'gcx, 'tcx>(
711711
for pred in implied_obligations {
712712
// Match the existing behavior.
713713
if pred.is_global() && !pred.has_late_bound_regions() {
714+
let pred = fcx.normalize_associated_types_in(span, &pred);
714715
let obligation = traits::Obligation::new(
715716
traits::ObligationCause::new(
716717
span,

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ union U where i32: Foo { f: i32 } //~ ERROR
2828
type Y where i32: Foo = (); // OK - bound is ignored
2929

3030
impl Foo for () where i32: Foo { //~ ERROR
31-
fn test(&self) { //~ ERROR
31+
fn test(&self) {
3232
3i32.test();
3333
Foo::test(&4i32);
3434
generic_function(5i32);
@@ -60,7 +60,7 @@ struct Dst<X: ?Sized> {
6060
}
6161

6262
struct TwoStrs(str, str) where str: Sized; //~ ERROR
63-
//~^ ERROR
63+
6464

6565
fn unsized_local() where Dst<A>: Sized { //~ ERROR
6666
let x: Dst<A> = *(Box::new(Dst { x: 1 }) as Box<Dst<A>>);

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

+2-27
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ error[E0277]: the trait bound `i32: Foo` is not satisfied
3838
--> $DIR/feature-gate-trivial_bounds.rs:30:1
3939
|
4040
LL | / impl Foo for () where i32: Foo { //~ ERROR
41-
LL | | fn test(&self) { //~ ERROR
41+
LL | | fn test(&self) {
4242
LL | | 3i32.test();
4343
LL | | Foo::test(&4i32);
4444
LL | | generic_function(5i32);
@@ -97,15 +97,6 @@ LL | struct TwoStrs(str, str) where str: Sized; //~ ERROR
9797
= help: see issue #48214
9898
= help: add #![feature(trivial_bounds)] to the crate attributes to enable
9999

100-
error[E0277]: the trait bound `str: std::marker::Sized` is not satisfied
101-
--> $DIR/feature-gate-trivial_bounds.rs:62:16
102-
|
103-
LL | struct TwoStrs(str, str) where str: Sized; //~ ERROR
104-
| ^^^ `str` does not have a constant size known at compile-time
105-
|
106-
= help: the trait `std::marker::Sized` is not implemented for `str`
107-
= note: only the last field of a struct may have a dynamically sized type
108-
109100
error[E0277]: the trait bound `A + 'static: std::marker::Sized` is not satisfied in `Dst<A + 'static>`
110101
--> $DIR/feature-gate-trivial_bounds.rs:65:1
111102
|
@@ -131,22 +122,6 @@ LL | | }
131122
= help: see issue #48214
132123
= help: add #![feature(trivial_bounds)] to the crate attributes to enable
133124

134-
error[E0277]: the trait bound `i32: Foo` is not satisfied
135-
--> $DIR/feature-gate-trivial_bounds.rs:31:5
136-
|
137-
LL | / fn test(&self) { //~ ERROR
138-
LL | | 3i32.test();
139-
LL | | Foo::test(&4i32);
140-
LL | | generic_function(5i32);
141-
LL | | }
142-
| |_____^ the trait `Foo` is not implemented for `i32`
143-
|
144-
note: required by `Foo`
145-
--> $DIR/feature-gate-trivial_bounds.rs:14:1
146-
|
147-
LL | pub trait Foo {
148-
| ^^^^^^^^^^^^^
149-
150-
error: aborting due to 13 previous errors
125+
error: aborting due to 11 previous errors
151126

152127
For more information about this error, try `rustc --explain E0277`.

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
// run-pass
1212
// regression test for issue #50825
13-
// Make sure that the `impl` bound (): X<T = ()> is prefered over
13+
// Make sure that the `impl` bound (): X<T = ()> is preferred over
1414
// the (): X bound in the where clause.
1515

1616
trait X {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
// run-pass
1212
// regression test for issue #50825
13-
// Make sure that the built-in bound {integer}: Sized is prefered over
13+
// Make sure that the built-in bound {integer}: Sized is preferred over
1414
// the u64: Sized bound in the where clause.
1515

1616
fn foo(y: &[()])

‎src/test/ui/issue-51044.rs

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// run-pass
12+
// regression test for issue #50825
13+
// Check that the feature gate normalizes associated types.
14+
15+
#![allow(dead_code)]
16+
struct Foo<T>(T);
17+
struct Duck;
18+
struct Quack;
19+
20+
trait Hello<A> where A: Animal {
21+
}
22+
23+
trait Animal {
24+
type Noise;
25+
}
26+
27+
trait Loud<R> {
28+
}
29+
30+
impl Loud<Quack> for f32 {
31+
}
32+
33+
impl Animal for Duck {
34+
type Noise = Quack;
35+
}
36+
37+
impl Hello<Duck> for Foo<f32> where f32: Loud<<Duck as Animal>::Noise> {
38+
}
39+
40+
fn main() {}

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

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// ignore-test FIXME(#50825)
1211
// run-pass
1312
// Inconsistent bounds with trait implementations
1413

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

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// ignore-test FIXME(#50825)
1211
// Check that reborrows are still illegal with Copy mutable references
1312
#![feature(trivial_bounds)]
1413
#![allow(unused)]

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

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// ignore-test FIXME(#50825)
1211
// run-pass
1312
// Check tautalogically false `Copy` bounds
1413
#![feature(trivial_bounds)]

‎src/test/ui/trivial-bounds-inconsistent-projection-error.rs

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(trivial_bounds)]
12+
#![allow(unused)]
13+
14+
struct B;
15+
16+
trait A {
17+
type X;
18+
fn get_x() -> Self::X;
19+
}
20+
21+
impl A for B {
22+
type X = u8;
23+
fn get_x() -> u8 { 0 }
24+
}
25+
26+
fn global_bound_is_hidden() -> u8
27+
where
28+
B: A<X = i32>
29+
{
30+
B::get_x() //~ ERROR
31+
}
32+
33+
fn main () {}

‎src/test/ui/trivial-bounds-inconsistent-projection-error.stderr

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/trivial-bounds-inconsistent-projection-error.rs:30:5
3+
|
4+
LL | fn global_bound_is_hidden() -> u8
5+
| -- expected `u8` because of return type
6+
...
7+
LL | B::get_x() //~ ERROR
8+
| ^^^^^^^^^^ expected u8, found i32
9+
10+
error: aborting due to previous error
11+
12+
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)