Skip to content

Commit

Permalink
Rollup merge of rust-lang#63934 - Aaron1011:fix/impl-trait-coherence,…
Browse files Browse the repository at this point in the history
… r=nikomatsakis

Fix coherence checking for impl trait in type aliases

**UPDATE**: This PR now treats all opaque types as remote. The original description appears below, but is no longer accurate.

Fixes rust-lang#63677

[RFC 2071](rust-lang/rfcs#2071) (impl-trait-existential-types) does not explicitly state how `type_alias_impl_trait` should interact with coherence. However, there's only one choice which makes sense - coherence should look at the underlying type (i.e. the *"defining"* type of the `impl Trait`) of the type alias, just like we do for non-`impl Trait` type aliases.

Specifically, `impl Trait` type aliases that resolve to a local type should be treated like a local type with respect to coherence (e.g. `impl Trait` type aliases which resolve to a foreign type should be treated as a foreign type, and those that resolve to a local type should be treated as a local type).

Since neither inherent impls nor direct trait impl (i.e. `impl MyType` or `impl MyTrait for MyType`) are allowed for type aliases, this usually does not come up. Before we ever attempt to do coherence checking, we will have errored out if an `impl Trait` type alias was used directly in an `impl` clause.

However, during trait selection, we sometimes need to prove bounds like `T: Sized` for some type `T`. If `T` is an impl trait type alias, this requires to know the coherence behavior for `impl Trait` type aliases when we perform coherence checking.

Note: Since determining the underlying type of an `impl Trait` type alias requires us to perform body type checking, this commit causes us to type check some bodies easier than we otherwise would have. However, since this is done through a query, this shouldn't cause any problems

For completeness, I've added an additional test of the coherence-related behavior of `impl Trait` type aliases.

cc rust-lang#63063
  • Loading branch information
Centril committed Sep 24, 2019
2 parents c623aa4 + 61cfe92 commit 94628af
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 6 deletions.
18 changes: 13 additions & 5 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ fn orphan_check_trait_ref<'tcx>(
}

fn uncovered_tys<'tcx>(tcx: TyCtxt<'_>, ty: Ty<'tcx>, in_crate: InCrate) -> Vec<Ty<'tcx>> {
if ty_is_local_constructor(ty, in_crate) {
if ty_is_local_constructor(tcx, ty, in_crate) {
vec![]
} else if fundamental_ty(ty) {
ty.walk_shallow()
Expand All @@ -451,7 +451,7 @@ fn is_possibly_remote_type(ty: Ty<'_>, _in_crate: InCrate) -> bool {
}

fn ty_is_local(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bool {
ty_is_local_constructor(ty, in_crate) ||
ty_is_local_constructor(tcx, ty, in_crate) ||
fundamental_ty(ty) && ty.walk_shallow().any(|t| ty_is_local(tcx, t, in_crate))
}

Expand All @@ -472,7 +472,7 @@ fn def_id_is_local(def_id: DefId, in_crate: InCrate) -> bool {
}
}

fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool {
fn ty_is_local_constructor(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bool {
debug!("ty_is_local_constructor({:?})", ty);

match ty.sty {
Expand Down Expand Up @@ -504,6 +504,15 @@ fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool {

ty::Adt(def, _) => def_id_is_local(def.did, in_crate),
ty::Foreign(did) => def_id_is_local(did, in_crate),
ty::Opaque(did, _) => {
// Check the underlying type that this opaque
// type resolves to.
// This recursion will eventually terminate,
// since we've already managed to successfully
// resolve all opaque types by this point
let real_ty = tcx.type_of(did);
ty_is_local_constructor(tcx, real_ty, in_crate)
}

ty::Dynamic(ref tt, ..) => {
if let Some(principal) = tt.principal() {
Expand All @@ -518,8 +527,7 @@ fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool {
ty::UnnormalizedProjection(..) |
ty::Closure(..) |
ty::Generator(..) |
ty::GeneratorWitness(..) |
ty::Opaque(..) => {
ty::GeneratorWitness(..) => {
bug!("ty_is_local invoked on unexpected type: {:?}", ty)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/specialize/specialization_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ impl<'tcx> Graph {
/// The parent of a given impl, which is the `DefId` of the trait when the
/// impl is a "specialization root".
pub fn parent(&self, child: DefId) -> DefId {
*self.parent.get(&child).unwrap()
*self.parent.get(&child).unwrap_or_else(|| panic!("Failed to get parent for {:?}", child))
}
}

Expand Down
23 changes: 23 additions & 0 deletions src/test/ui/impl-trait/auto-trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Tests that type alias impls traits do not leak auto-traits for
// the purposes of coherence checking
#![feature(type_alias_impl_trait)]

trait OpaqueTrait { }
impl<T> OpaqueTrait for T { }
type OpaqueType = impl OpaqueTrait;
fn mk_opaque() -> OpaqueType { () }

#[derive(Debug)]
struct D<T>(T);

trait AnotherTrait { }
impl<T: Send> AnotherTrait for T { }

// This is in error, because we cannot assume that `OpaqueType: !Send`.
// (We treat opaque types as "foreign types" that could grow more impls
// in the future.)
impl AnotherTrait for D<OpaqueType> {
//~^ ERROR conflicting implementations of trait `AnotherTrait` for type `D<OpaqueType>`
}

fn main() {}
12 changes: 12 additions & 0 deletions src/test/ui/impl-trait/auto-trait.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0119]: conflicting implementations of trait `AnotherTrait` for type `D<OpaqueType>`:
--> $DIR/auto-trait.rs:19:1
|
LL | impl<T: Send> AnotherTrait for T { }
| -------------------------------- first implementation here
...
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `D<OpaqueType>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0119`.
22 changes: 22 additions & 0 deletions src/test/ui/impl-trait/negative-reasoning.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Tests that we cannot assume that an opaque type does *not* implement some
// other trait
#![feature(type_alias_impl_trait)]

trait OpaqueTrait { }
impl<T> OpaqueTrait for T { }
type OpaqueType = impl OpaqueTrait;
fn mk_opaque() -> OpaqueType { () }

#[derive(Debug)]
struct D<T>(T);

trait AnotherTrait { }
impl<T: std::fmt::Debug> AnotherTrait for T { }


// This is in error, because we cannot assume that `OpaqueType: !Debug`
impl AnotherTrait for D<OpaqueType> {
//~^ ERROR conflicting implementations of trait `AnotherTrait` for type `D<OpaqueType>`
}

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/impl-trait/negative-reasoning.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0119]: conflicting implementations of trait `AnotherTrait` for type `D<OpaqueType>`:
--> $DIR/negative-reasoning.rs:18:1
|
LL | impl<T: std::fmt::Debug> AnotherTrait for T { }
| ------------------------------------------- first implementation here
...
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `D<OpaqueType>`
|
= note: upstream crates may add a new impl of trait `std::fmt::Debug` for type `OpaqueType` in future versions

error: aborting due to previous error

For more information about this error, try `rustc --explain E0119`.
2 changes: 2 additions & 0 deletions src/test/ui/type-alias-impl-trait/auxiliary/foreign-crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub trait ForeignTrait {}
pub struct ForeignType<T>(pub T);
17 changes: 17 additions & 0 deletions src/test/ui/type-alias-impl-trait/coherence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// aux-build:foreign-crate.rs
#![feature(type_alias_impl_trait)]

extern crate foreign_crate;

trait LocalTrait {}
impl<T> LocalTrait for foreign_crate::ForeignType<T> {}

type AliasOfForeignType<T> = impl LocalTrait;
fn use_alias<T>(val: T) -> AliasOfForeignType<T> {
foreign_crate::ForeignType(val)
}

impl<T> foreign_crate::ForeignTrait for AliasOfForeignType<T> {}
//~^ ERROR the type parameter `T` is not constrained by the impl trait, self type, or predicates

fn main() {}
9 changes: 9 additions & 0 deletions src/test/ui/type-alias-impl-trait/coherence.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0207]: the type parameter `T` is not constrained by the impl trait, self type, or predicates
--> $DIR/coherence.rs:14:6
|
LL | impl<T> foreign_crate::ForeignTrait for AliasOfForeignType<T> {}
| ^ unconstrained type parameter

error: aborting due to previous error

For more information about this error, try `rustc --explain E0207`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// check-pass
// Regression test for issue #63677 - ensure that
// coherence checking can properly handle 'impl trait'
// in type aliases
#![feature(type_alias_impl_trait)]

pub trait Trait {}
pub struct S1<T>(T);
pub struct S2<T>(T);

pub type T1 = impl Trait;
pub type T2 = S1<T1>;
pub type T3 = S2<T2>;

impl<T> Trait for S1<T> {}
impl<T: Trait> S2<T> {}
impl T3 {}

pub fn use_t1() -> T1 { S1(()) }

fn main() {}

0 comments on commit 94628af

Please sign in to comment.