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

Implement the Re-rebalance coherence RFC #56145

Merged
merged 13 commits into from
Jan 5, 2019
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# `re_rebalance_coherence`

The tracking issue for this feature is: [#55437]

[#55437]: https://github.com/rust-lang/rust/issues/55437

------------------------

The `re_rebalance_coherence` feature tweaks the rules regarding which trait
impls are allowed in crates.
The following rule is used:

Given `impl<P1..=Pn> Trait<T1..=Tn> for T0`, an impl is valid only if at
Copy link
Member

Choose a reason for hiding this comment

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

Is this "if and only if"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's directly copied from the RFC. I think it should be "if and only if" but's not written there. (cc @sgrif who has written that thing)

least one of the following is true:
- `Trait` is a local trait
- 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`)


See the [RFC](https://github.com/rust-lang/rfcs/blob/master/text/2451-re-rebalancing-coherence.md) for details.
89 changes: 57 additions & 32 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,43 +371,68 @@ fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt<'_, '_, '_>,
trait_ref);
}

// 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 input_ty in trait_ref.input_types() {
if ty_is_local(tcx, input_ty, in_crate) {
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));
}
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)
//
for input_ty in trait_ref.input_types() {
debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty);
if ty_is_local(tcx, input_ty, in_crate) {
debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty);
return Ok(());
} else if let ty::Param(_) = input_ty.sty {
debug!("orphan_check_trait_ref: uncovered ty: `{:?}`", input_ty);
return Err(OrphanCheckErr::UncoveredTy(input_ty))
}

// OK, found local type, all prior types upheld invariant.
return Ok(());
}
// If we exit above loop, never found a local type.
debug!("orphan_check_trait_ref: no local type");
Err(OrphanCheckErr::NoLocalInputType)
} else {
// 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 input_ty in trait_ref.input_types() {
if ty_is_local(tcx, input_ty, in_crate) {
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));
}
}

// 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));
// 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 we exit above loop, never found a local type.
debug!("orphan_check_trait_ref: no local type");
Err(OrphanCheckErr::NoLocalInputType)
}

// If we exit above loop, never found a local type.
debug!("orphan_check_trait_ref: no local type");
return Err(OrphanCheckErr::NoLocalInputType);
}

fn uncovered_tys<'tcx>(tcx: TyCtxt<'_, '_, '_>, ty: Ty<'tcx>, in_crate: InCrate)
Expand Down
3 changes: 3 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ declare_features! (

// Allows paths to enum variants on type aliases.
(active, type_alias_enum_variants, "1.31.0", Some(49683), None),

// Re-Rebalance coherence
(active, re_rebalance_coherence, "1.32.0", Some(55437), None),
);

declare_features! (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@

pub trait Backend{}
pub trait SupportsDefaultKeyword {}

impl SupportsDefaultKeyword for Postgres {}

pub struct Postgres;

impl Backend for Postgres {}

pub struct AstPass<DB>(::std::marker::PhantomData<DB>);

pub trait QueryFragment<DB: Backend> {}


#[derive(Debug, Clone, Copy)]
pub struct BatchInsert<'a, T: 'a, Tab> {
_marker: ::std::marker::PhantomData<(&'a T, Tab)>,
}

impl<'a, T:'a, Tab, DB> QueryFragment<DB> for BatchInsert<'a, T, Tab>
where DB: SupportsDefaultKeyword + Backend,
{}
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence-bigint-int.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// run-pass
// aux-build:coherence_lib.rs
// revisions: old re

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

// pretty-expanded FIXME #23616

Expand Down
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence-bigint-vecint.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// run-pass
// aux-build:coherence_lib.rs
// revisions: old re

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

// pretty-expanded FIXME #23616

Expand Down
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence-blanket.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// run-pass
#![allow(unused_imports)]
// aux-build:coherence_lib.rs
// revisions: old re

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

// pretty-expanded FIXME #23616

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// run-pass
#![allow(dead_code)]
// aux-build:coherence_lib.rs
// revisions: old re

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

// pretty-expanded FIXME #23616

Expand Down
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence-impl-in-fn.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// run-pass
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]
#![allow(dead_code)]
#![allow(non_camel_case_types)]

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// run-pass
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]
#![allow(dead_code)]
// aux-build:coherence_lib.rs

Expand Down
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence-iterator-vec.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// run-pass
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]
#![allow(dead_code)]
// aux-build:coherence_lib.rs

Expand Down
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence-multidispatch-tuple.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// run-pass
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]
#![allow(unused_imports)]
// pretty-expanded FIXME #23616

Expand Down
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence-negative-impls-safe.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// run-pass
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]
#![allow(dead_code)]
// pretty-expanded FIXME #23616

Expand Down
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence-rfc447-constrained.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// run-pass
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]
// check that trait matching can handle impls whose types are only
// constrained by a projection.

Expand Down
4 changes: 4 additions & 0 deletions src/test/run-pass/coherence/coherence-where-clause.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
// run-pass
// revisions: old re

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

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

Expand Down
3 changes: 3 additions & 0 deletions src/test/run-pass/coherence/coherence_copy_like.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// run-pass
// revisions: old re

#![cfg_attr(re, feature(re_rebalance_coherence))]
#![allow(dead_code)]
// Test that we are able to introduce a negative constraint that
// `MyType: !MyTrait` along with other "fundamental" wrappers.
Expand Down
14 changes: 14 additions & 0 deletions src/test/run-pass/coherence/re-rebalance-coherence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![allow(dead_code)]
#![feature(re_rebalance_coherence)]

// run-pass
// aux-build:re_rebalance_coherence_lib.rs

extern crate re_rebalance_coherence_lib as lib;
use lib::*;

struct Oracle;
impl Backend for Oracle {}
impl<'a, T:'a, Tab> QueryFragment<Oracle> for BatchInsert<'a, T, Tab> {}

fn main() {}
23 changes: 23 additions & 0 deletions src/test/ui/coherence/auxiliary/re_rebalance_coherence_lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@

pub trait Backend{}
pub trait SupportsDefaultKeyword {}

impl SupportsDefaultKeyword for Postgres {}

pub struct Postgres;

impl Backend for Postgres {}

pub struct AstPass<DB>(::std::marker::PhantomData<DB>);

pub trait QueryFragment<DB: Backend> {}


#[derive(Debug, Clone, Copy)]
pub struct BatchInsert<'a, T: 'a, Tab> {
_marker: ::std::marker::PhantomData<(&'a T, Tab)>,
}

impl<'a, T:'a, Tab, DB> QueryFragment<DB> for BatchInsert<'a, T, Tab>
where DB: SupportsDefaultKeyword + Backend,
{}
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:6:1
--> $DIR/coherence-all-remote.rs:9:1
|
LL | impl<T> Remote1<T> for isize { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/coherence/coherence-all-remote.re.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
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:1
|
LL | impl<T> Remote1<T> for isize { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
|
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to previous error

For more information about this error, try `rustc --explain E0210`.
6 changes: 5 additions & 1 deletion src/test/ui/coherence/coherence-all-remote.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
// 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 { }
//~^ ERROR E0210
//[old]~^ ERROR E0210
//[re]~^^ 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:8:1
--> $DIR/coherence-bigint-param.rs:11:1
|
LL | impl<T> Remote1<BigInt> for T { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/coherence/coherence-bigint-param.re.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
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:1
|
LL | impl<T> Remote1<BigInt> for T { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
|
= note: only traits defined in the current crate can be implemented for a type parameter

error: aborting due to previous error

For more information about this error, try `rustc --explain E0210`.
6 changes: 5 additions & 1 deletion src/test/ui/coherence/coherence-bigint-param.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
// 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 { }
//~^ ERROR type parameter `T` must be used as the type parameter for some local type
//[old]~^ ERROR type parameter `T` must be used as the type parameter for some local type
//[re]~^^ ERROR E0210

fn main() { }
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error[E0119]: conflicting implementations of trait `MyTrait`:
--> $DIR/coherence-blanket-conflicts-with-blanket-implemented.rs:24:1
--> $DIR/coherence-blanket-conflicts-with-blanket-implemented.rs:28:1
|
LL | impl<T:Even> MyTrait for T {
| -------------------------- first implementation here
...
LL | impl<T:Odd> MyTrait for T { //~ ERROR E0119
LL | impl<T:Odd> MyTrait for T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation

error: aborting due to previous error
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0119]: conflicting implementations of trait `MyTrait`:
--> $DIR/coherence-blanket-conflicts-with-blanket-implemented.rs:28:1
|
LL | impl<T:Even> MyTrait for T {
| -------------------------- first implementation here
...
LL | impl<T:Odd> MyTrait for T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation

error: aborting due to previous error

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