Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-enable trivial bounds #51042

Merged
merged 5 commits into from
Jun 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
72 changes: 61 additions & 11 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh. But makes sense. Chalk can't come soon enough!

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 \
Expand All @@ -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 |
Expand All @@ -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
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/feature-gate-trivial_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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>>);
Expand Down
29 changes: 2 additions & 27 deletions src/test/ui/feature-gate-trivial_bounds.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
|
Expand All @@ -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`.
2 changes: 1 addition & 1 deletion src/test/ui/issue-50825-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issue-50825.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: &[()])
Expand Down
40 changes: 40 additions & 0 deletions src/test/ui/issue-51044.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion src/test/ui/trivial-bounds-inconsistent-copy-reborrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/trivial-bounds-inconsistent-copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/trivial-bounds-inconsistent-projection-error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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.

#![feature(trivial_bounds)]
#![allow(unused)]

struct B;

trait A {
type X;
fn get_x() -> Self::X;
}

impl A for B {
type X = u8;
fn get_x() -> u8 { 0 }
}

fn global_bound_is_hidden() -> u8
where
B: A<X = i32>
{
B::get_x() //~ ERROR
}

fn main () {}
12 changes: 12 additions & 0 deletions src/test/ui/trivial-bounds-inconsistent-projection-error.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0308]: mismatched types
--> $DIR/trivial-bounds-inconsistent-projection-error.rs:30:5
|
LL | fn global_bound_is_hidden() -> u8
| -- expected `u8` because of return type
...
LL | B::get_x() //~ ERROR
| ^^^^^^^^^^ expected u8, found i32

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
Loading