Skip to content

Commit

Permalink
Auto merge of #65879 - ohadravid:stabilize-re-rebalance-coherence, r=…
Browse files Browse the repository at this point in the history
…nikomatsakis

Stabilize the `re_rebalance_coherence` feature

This PR stabilizes [RFC 2451](https://rust-lang.github.io/rfcs/2451-re-rebalancing-coherence.html), re-rebalance coherence.

Changes include removing the attribute from tests which tested both the old and new behavior, moving the feature to `accepted` and removing the old logic.

I'll also open a [PR](rust-lang/reference#703) against the reference, updating it with the content of the RFC.

Closes #63599

r? @nikomatsakis
  • Loading branch information
bors committed Nov 9, 2019
2 parents 475c713 + 3e0759d commit 98c173a
Show file tree
Hide file tree
Showing 229 changed files with 269 additions and 1,640 deletions.

This file was deleted.

27 changes: 12 additions & 15 deletions src/libcore/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,12 @@ pub trait AsMut<T: ?Sized> {
/// A value-to-value conversion that consumes the input value. The
/// opposite of [`From`].
///
/// One should only implement [`Into`] if a conversion to a type outside the current crate is
/// required. Otherwise one should always prefer implementing [`From`] over [`Into`] because
/// implementing [`From`] automatically provides one with a implementation of [`Into`] thanks to
/// the blanket implementation in the standard library. [`From`] cannot do these type of
/// conversions because of Rust's orphaning rules.
/// One should avoid implementing [`Into`] and implement [`From`] instead.
/// Implementing [`From`] automatically provides one with an implementation of [`Into`]
/// thanks to the blanket implementation in the standard library.
///
/// Prefer using [`Into`] over [`From`] when specifying trait bounds on a generic function
/// to ensure that types that only implement [`Into`] can be used as well.
///
/// **Note: This trait must not fail**. If the conversion can fail, use [`TryInto`].
///
Expand All @@ -218,23 +219,22 @@ pub trait AsMut<T: ?Sized> {
/// - [`From`]`<T> for U` implies `Into<U> for T`
/// - [`Into`] is reflexive, which means that `Into<T> for T` is implemented
///
/// # Implementing [`Into`] for conversions to external types
/// # Implementing [`Into`] for conversions to external types in old versions of Rust
///
/// If the destination type is not part of the current crate
/// then you can't implement [`From`] directly.
/// Prior to Rust 1.40, if the destination type was not part of the current crate
/// then you couldn't implement [`From`] directly.
/// For example, take this code:
///
/// ```compile_fail
/// ```
/// struct Wrapper<T>(Vec<T>);
/// impl<T> From<Wrapper<T>> for Vec<T> {
/// fn from(w: Wrapper<T>) -> Vec<T> {
/// w.0
/// }
/// }
/// ```
/// This will fail to compile because we cannot implement a trait for a type
/// if both the trait and the type are not defined by the current crate.
/// This is due to Rust's orphaning rules. To bypass this, you can implement [`Into`] directly:
/// This will fail to compile in older versions of the language because Rust's orphaning rules
/// used to be a little bit more strict. To bypass this, you could implement [`Into`] directly:
///
/// ```
/// struct Wrapper<T>(Vec<T>);
Expand All @@ -249,9 +249,6 @@ pub trait AsMut<T: ?Sized> {
/// (as [`From`] does with [`Into`]). Therefore, you should always try to implement [`From`]
/// and then fall back to [`Into`] if [`From`] can't be implemented.
///
/// Prefer using [`Into`] over [`From`] when specifying trait bounds on a generic function
/// to ensure that types that only implement [`Into`] can be used as well.
///
/// # Examples
///
/// [`String`] implements [`Into`]`<`[`Vec`]`<`[`u8`]`>>`:
Expand Down
146 changes: 40 additions & 106 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,118 +367,52 @@ fn orphan_check_trait_ref<'tcx>(
trait_ref);
}

if tcx.features().re_rebalance_coherence {
// Given impl<P1..=Pn> Trait<T1..=Tn> for T0, an impl is valid only
// if at least one of the following is true:
//
// - Trait is a local trait
// (already checked in orphan_check prior to calling this function)
// - All of
// - At least one of the types T0..=Tn must be a local type.
// Let Ti be the first such type.
// - No uncovered type parameters P1..=Pn may appear in T0..Ti (excluding Ti)
//
fn uncover_fundamental_ty<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
in_crate: InCrate,
) -> Vec<Ty<'tcx>> {
if fundamental_ty(ty) && ty_is_non_local(tcx, ty, in_crate).is_some() {
ty.walk_shallow().flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)).collect()
} else {
vec![ty]
}
// Given impl<P1..=Pn> Trait<T1..=Tn> for T0, an impl is valid only
// if at least one of the following is true:
//
// - Trait is a local trait
// (already checked in orphan_check prior to calling this function)
// - All of
// - At least one of the types T0..=Tn must be a local type.
// Let Ti be the first such type.
// - No uncovered type parameters P1..=Pn may appear in T0..Ti (excluding Ti)
//
fn uncover_fundamental_ty<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
in_crate: InCrate,
) -> Vec<Ty<'tcx>> {
if fundamental_ty(ty) && ty_is_non_local(tcx, ty, in_crate).is_some() {
ty.walk_shallow().flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)).collect()
} else {
vec![ty]
}
}

let mut non_local_spans = vec![];
for (i, input_ty) in trait_ref
.input_types()
.flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate))
.enumerate()
{
debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty);
let non_local_tys = ty_is_non_local(tcx, input_ty, in_crate);
if non_local_tys.is_none() {
debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty);
return Ok(());
} else if let ty::Param(_) = input_ty.kind {
debug!("orphan_check_trait_ref: uncovered ty: `{:?}`", input_ty);
return Err(OrphanCheckErr::UncoveredTy(input_ty))
}
if let Some(non_local_tys) = non_local_tys {
for input_ty in non_local_tys {
non_local_spans.push((input_ty, i == 0));
}
}
let mut non_local_spans = vec![];
for (i, input_ty) in trait_ref
.input_types()
.flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate))
.enumerate()
{
debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty);
let non_local_tys = ty_is_non_local(tcx, input_ty, in_crate);
if non_local_tys.is_none() {
debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty);
return Ok(());
} else if let ty::Param(_) = input_ty.kind {
debug!("orphan_check_trait_ref: uncovered ty: `{:?}`", input_ty);
return Err(OrphanCheckErr::UncoveredTy(input_ty))
}
// If we exit above loop, never found a local type.
debug!("orphan_check_trait_ref: no local type");
Err(OrphanCheckErr::NonLocalInputType(non_local_spans))
} else {
let mut non_local_spans = vec![];
// First, create an ordered iterator over all the type
// parameters to the trait, with the self type appearing
// first. Find the first input type that either references a
// type parameter OR some local type.
for (i, input_ty) in trait_ref.input_types().enumerate() {
let non_local_tys = ty_is_non_local(tcx, input_ty, in_crate);
if non_local_tys.is_none() {
debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty);

// First local input type. Check that there are no
// uncovered type parameters.
let uncovered_tys = uncovered_tys(tcx, input_ty, in_crate);
for uncovered_ty in uncovered_tys {
if let Some(param) = uncovered_ty.walk()
.find(|t| is_possibly_remote_type(t, in_crate))
{
debug!("orphan_check_trait_ref: uncovered type `{:?}`", param);
return Err(OrphanCheckErr::UncoveredTy(param));
}
}

// OK, found local type, all prior types upheld invariant.
return Ok(());
}

// Otherwise, enforce invariant that there are no type
// parameters reachable.
if let Some(param) = input_ty.walk()
.find(|t| is_possibly_remote_type(t, in_crate))
{
debug!("orphan_check_trait_ref: uncovered type `{:?}`", param);
return Err(OrphanCheckErr::UncoveredTy(param));
}

if let Some(non_local_tys) = non_local_tys {
for input_ty in non_local_tys {
non_local_spans.push((input_ty, i == 0));
}
if let Some(non_local_tys) = non_local_tys {
for input_ty in non_local_tys {
non_local_spans.push((input_ty, i == 0));
}
}
// If we exit above loop, never found a local type.
debug!("orphan_check_trait_ref: no local type");
Err(OrphanCheckErr::NonLocalInputType(non_local_spans))
}
}

fn uncovered_tys<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, in_crate: InCrate) -> Vec<Ty<'tcx>> {
if ty_is_non_local_constructor(tcx, ty, in_crate).is_none() {
vec![]
} else if fundamental_ty(ty) {
ty.walk_shallow()
.flat_map(|t| uncovered_tys(tcx, t, in_crate))
.collect()
} else {
vec![ty]
}
}

fn is_possibly_remote_type(ty: Ty<'_>, _in_crate: InCrate) -> bool {
match ty.kind {
ty::Projection(..) | ty::Param(..) => true,
_ => false,
}
// If we exit above loop, never found a local type.
debug!("orphan_check_trait_ref: no local type");
Err(OrphanCheckErr::NonLocalInputType(non_local_spans))
}

fn ty_is_non_local<'t>(tcx: TyCtxt<'t>, ty: Ty<'t>, in_crate: InCrate) -> Option<Vec<Ty<'t>>> {
Expand Down
3 changes: 3 additions & 0 deletions src/libsyntax/feature_gate/accepted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,9 @@ declare_features! (
(accepted, const_constructor, "1.40.0", Some(61456), None),
/// Allows the use of `#[cfg(doctest)]`, set when rustdoc is collecting doctests.
(accepted, cfg_doctest, "1.40.0", Some(62210), None),
/// Allows relaxing the coherence rules such that
/// `impl<T> ForeignTrait<LocalType> for ForeignType<T>` is permitted.
(accepted, re_rebalance_coherence, "1.41.0", Some(55437), None),

// -------------------------------------------------------------------------
// feature-group-end: accepted features
Expand Down
4 changes: 0 additions & 4 deletions src/libsyntax/feature_gate/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,10 +466,6 @@ declare_features! (
/// Allows exhaustive integer pattern matching on `usize` and `isize`.
(active, precise_pointer_size_matching, "1.32.0", Some(56354), None),

/// Allows relaxing the coherence rules such that
/// `impl<T> ForeignTrait<LocalType> for ForeignType<T>` is permitted.
(active, re_rebalance_coherence, "1.32.0", Some(55437), None),

/// Allows using `#[ffi_returns_twice]` on foreign functions.
(active, ffi_returns_twice, "1.34.0", Some(58314), None),

Expand Down
11 changes: 0 additions & 11 deletions src/test/ui/coherence/coherence-all-remote.re.stderr

This file was deleted.

6 changes: 1 addition & 5 deletions src/test/ui/coherence/coherence-all-remote.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
// aux-build:coherence_lib.rs
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]

extern crate coherence_lib as lib;
use lib::Remote1;

impl<T> Remote1<T> for isize { }
//[old]~^ ERROR E0210
//[re]~^^ ERROR E0210
//~^ ERROR E0210

fn main() { }
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
--> $DIR/coherence-all-remote.rs:9:6
--> $DIR/coherence-all-remote.rs:6:6
|
LL | impl<T> Remote1<T> for isize { }
| ^ type parameter `T` must be used as the type parameter for some local type
Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/coherence/coherence-bigint-int.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
// run-pass
// aux-build:coherence_lib.rs
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]

// pretty-expanded FIXME #23616

Expand Down
11 changes: 0 additions & 11 deletions src/test/ui/coherence/coherence-bigint-param.old.stderr

This file was deleted.

6 changes: 1 addition & 5 deletions src/test/ui/coherence/coherence-bigint-param.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
// aux-build:coherence_lib.rs
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]

extern crate coherence_lib as lib;
use lib::Remote1;

pub struct BigInt;

impl<T> Remote1<BigInt> for T { }
//[old]~^ ERROR type parameter `T` must be used as the type parameter for some local type
//[re]~^^ ERROR E0210
//~^ ERROR E0210

fn main() { }
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
--> $DIR/coherence-bigint-param.rs:11:6
--> $DIR/coherence-bigint-param.rs:8:6
|
LL | impl<T> Remote1<BigInt> for T { }
| ^ type parameter `T` must be used as the type parameter for some local type
Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/coherence/coherence-bigint-vecint.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
// run-pass
// aux-build:coherence_lib.rs
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]

// pretty-expanded FIXME #23616

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]

use std::fmt::Debug;
use std::default::Default;

Expand All @@ -26,8 +22,7 @@ impl<T:Even> MyTrait for T {
}

impl<T:Odd> MyTrait for T {
//[old]~^ ERROR E0119
//[re]~^^ ERROR E0119
//~^ ERROR E0119

fn get(&self) -> usize { 0 }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0119]: conflicting implementations of trait `MyTrait`:
--> $DIR/coherence-blanket-conflicts-with-blanket-implemented.rs:28:1
--> $DIR/coherence-blanket-conflicts-with-blanket-implemented.rs:24:1
|
LL | impl<T:Even> MyTrait for T {
| -------------------------- first implementation here
Expand Down
Loading

0 comments on commit 98c173a

Please sign in to comment.