Auto merge of #66590 - Aaron1011:fix/real-impl-trait-coherence, r=varkor

Apply proper commit from PR #63934

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

src/librustc/traits/coherence.rs

@@ -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]
@@ -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(());
@@ -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);
@@ -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() {
@@ -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>> {
@@ -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, ..) => {

src/test/ui/type-alias-impl-trait/issue-66580-closure-coherence.rs

@@ -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() {}