Skip to content

Commit

Permalink
Apply proper commit from PR #63934
Browse files Browse the repository at this point in the history
While working on PR #63934, I accidentally reverted to an older version
of the PR while working on a rebase. The PR was then merged, not with
the later, approved changes, but with earlier, unapproved changes.

This PR applies the changes that were *suppoesd* to be mereged in
PR #63934. All of the proper tests appear to have been merged
in PR #63934, so this PR adds no new tests

Fixes #66580
  • Loading branch information
Aaron1011 committed Nov 28, 2019
1 parent b9cf541 commit 79fcaf8
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 15 deletions.
48 changes: 33 additions & 15 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ fn orphan_check_trait_ref<'tcx>(
ty: Ty<'tcx>,
in_crate: InCrate,
) -> Vec<Ty<'tcx>> {
if fundamental_ty(ty) && ty_is_non_local(tcx, ty, in_crate).is_some() {
if fundamental_ty(ty) && ty_is_non_local(ty, in_crate).is_some() {
ty.walk_shallow().flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)).collect()
} else {
vec![ty]
Expand All @@ -396,7 +396,7 @@ fn orphan_check_trait_ref<'tcx>(
.enumerate()
{
debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty);
let non_local_tys = ty_is_non_local(tcx, input_ty, in_crate);
let non_local_tys = ty_is_non_local(input_ty, in_crate);
if non_local_tys.is_none() {
debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty);
return Ok(());
Expand All @@ -405,7 +405,7 @@ fn orphan_check_trait_ref<'tcx>(
let local_type = trait_ref
.input_types()
.flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate))
.filter(|ty| ty_is_non_local_constructor(tcx, ty, in_crate).is_none())
.filter(|ty| ty_is_non_local_constructor(ty, in_crate).is_none())
.next();

debug!("orphan_check_trait_ref: uncovered ty local_type: `{:?}`", local_type);
Expand All @@ -423,13 +423,13 @@ fn orphan_check_trait_ref<'tcx>(
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>>> {
match ty_is_non_local_constructor(tcx, ty, in_crate) {
fn ty_is_non_local<'t>(ty: Ty<'t>, in_crate: InCrate) -> Option<Vec<Ty<'t>>> {
match ty_is_non_local_constructor(ty, in_crate) {
Some(ty) => if !fundamental_ty(ty) {
Some(vec![ty])
} else {
let tys: Vec<_> = ty.walk_shallow()
.filter_map(|t| ty_is_non_local(tcx, t, in_crate))
.filter_map(|t| ty_is_non_local(t, in_crate))
.flat_map(|i| i)
.collect();
if tys.is_empty() {
Expand Down Expand Up @@ -460,7 +460,6 @@ fn def_id_is_local(def_id: DefId, in_crate: InCrate) -> bool {
}

fn ty_is_non_local_constructor<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
in_crate: InCrate,
) -> Option<Ty<'tcx>> {
Expand Down Expand Up @@ -503,14 +502,33 @@ fn ty_is_non_local_constructor<'tcx>(
} else {
Some(ty)
},
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_non_local_constructor(tcx, real_ty, in_crate)
ty::Opaque(..) => {
// This merits some explanation.
// Normally, opaque types are not involed when performing
// coherence checking, since it is illegal to directly
// implement a trait on an opaque type. However, we might
// end up looking at an opaque type during coherence checking
// if an opaque type gets used within another type (e.g. as
// a type parameter). This requires us to decide whether or
// not an opaque type should be considered 'local' or not.
//
// We choose to treat all opaque types as non-local, even
// those that appear within the same crate. This seems
// somewhat suprising at first, but makes sense when
// you consider that opaque types are supposed to hide
// the underlying type *within the same crate*. When an
// opaque type is used from outside the module
// where it is declared, it should be impossible to observe
// anyything about it other than the traits that it implements.
//
// The alternative would be to look at the underlying type
// to determine whether or not the opaque type itself should
// be considered local. However, this could make it a breaking change
// to switch the underlying ('defining') type from a local type
// to a remote type. This would violate the rule that opaque
// types should be completely opaque apart from the traits
// that they implement, so we don't use this behavior.
Some(ty)
}

ty::Dynamic(ref tt, ..) => {
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/type-alias-impl-trait/issue-66580-closure-coherence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression test for issue #66580
// Ensures that we don't try to determine whether a closure
// is foreign when it's the underlying type of an opaque type
// check-pass
#![feature(type_alias_impl_trait)]

type Closure = impl FnOnce();

fn closure() -> Closure {
|| {}
}

struct Wrap<T> { f: T }

impl Wrap<Closure> {}

impl<T> Wrap<T> {}

fn main() {}

0 comments on commit 79fcaf8

Please sign in to comment.