Auto merge of #66178 - Aaron1011:fix/opaque-normalize, r=varkor

Fix opaque types resulting from projections in function signature

When we normalize the types in a function signature, we may end up
resolving a projection to an opaque type (e.g. `Self::MyType` when
we have `type MyType = impl SomeTrait`). When the projection is
resolved, we will instantiate the generic parameters into fresh
inference variables.

While we do want to normalize projections to opaque types, we don't want
to replace the explicit generic parameters (e.g. `T` in `impl
MyTrait<T>`) with inference variables. We want the opaque type in the
function signature to be eligible to be a defining use of that opaque
type - adding inference variables prevents this, since the opaque type
substs now appears to refer to some specific type, rather than a generic
type.

To resolve this issue, we inspect the opaque types in the function
signature after normalization. Any inference variables in the substs are
replaced with the corresponding generic parameter in the identity substs
(e.g. `T` in `impl MyTrait<T>`). Note that normalization is the only way
that we can end up with inference variables in opaque substs in a
function signature - users have no way of getting inference variables
into a function signature.

Note that all of this refers to the opaque type (ty::Opaque) and its
subst - *not* to the underlying type.

Fixes #59342

Author: bors

src/librustc_typeck/check/mod.rs

@@ -114,7 +114,7 @@ use rustc::ty::{
 use rustc::ty::adjustment::{
     Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCast
 };
-use rustc::ty::fold::TypeFoldable;
+use rustc::ty::fold::{TypeFoldable, TypeFolder};
 use rustc::ty::query::Providers;
 use rustc::ty::subst::{
     GenericArgKind, Subst, InternalSubsts, SubstsRef, UserSelfTy, UserSubsts,
@@ -872,6 +872,111 @@ fn used_trait_imports(tcx: TyCtxt<'_>, def_id: DefId) -> &DefIdSet {
     &*tcx.typeck_tables_of(def_id).used_trait_imports
 }
 
+/// Inspects the substs of opaque types, replacing any inference variables
+/// with proper generic parameter from the identity substs.
+///
+/// This is run after we normalize the function signature, to fix any inference
+/// variables introduced by the projection of associated types. This ensures that
+/// any opaque types used in the signature continue to refer to generic parameters,
+/// allowing them to be considered for defining uses in the function body
+///
+/// For example, consider this code.
+///
+/// ```rust
+/// trait MyTrait {
+///     type MyItem;
+///     fn use_it(self) -> Self::MyItem
+/// }
+/// impl<T, I> MyTrait for T where T: Iterator<Item = I> {
+///     type MyItem = impl Iterator<Item = I>;
+///     fn use_it(self) -> Self::MyItem {
+///         self
+///     }
+/// }
+/// ```
+///
+/// When we normalize the signature of `use_it` from the impl block,
+/// we will normalize `Self::MyItem` to the opaque type `impl Iterator<Item = I>`
+/// However, this projection result may contain inference variables, due
+/// to the way that projection works. We didn't have any inference variables
+/// in the signature to begin with - leaving them in will cause us to incorrectly
+/// conclude that we don't have a defining use of `MyItem`. By mapping inference
+/// variables back to the actual generic parameters, we will correctly see that
+/// we have a defining use of `MyItem`
+fn fixup_opaque_types<'tcx, T>(tcx: TyCtxt<'tcx>, val: &T) -> T where T: TypeFoldable<'tcx> {
+    struct FixupFolder<'tcx> {
+        tcx: TyCtxt<'tcx>
+    }
+
+    impl<'tcx> TypeFolder<'tcx> for FixupFolder<'tcx> {
+        fn tcx<'a>(&'a self) -> TyCtxt<'tcx> {
+            self.tcx
+        }
+
+        fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
+            match ty.kind {
+                ty::Opaque(def_id, substs) => {
+                    debug!("fixup_opaque_types: found type {:?}", ty);
+                    // Here, we replace any inference variables that occur within
+                    // the substs of an opaque type. By definition, any type occuring
+                    // in the substs has a corresponding generic parameter, which is what
+                    // we replace it with.
+                    // This replacement is only run on the function signature, so any
+                    // inference variables that we come across must be the rust of projection
+                    // (there's no other way for a user to get inference variables into
+                    // a function signature).
+                    if ty.needs_infer() {
+                        let new_substs = InternalSubsts::for_item(self.tcx, def_id, |param, _| {
+                            let old_param = substs[param.index as usize];
+                            match old_param.unpack() {
+                                GenericArgKind::Type(old_ty) => {
+                                    if let ty::Infer(_) = old_ty.kind {
+                                        // Replace inference type with a generic parameter
+                                        self.tcx.mk_param_from_def(param)
+                                    } else {
+                                        old_param.fold_with(self)
+                                    }
+                                },
+                                GenericArgKind::Const(old_const) => {
+                                    if let ty::ConstKind::Infer(_) = old_const.val {
+                        // This should never happen - we currently do not support
+                        // 'const projections', e.g.:
+                        // `impl<T: SomeTrait> MyTrait for T where <T as SomeTrait>::MyConst == 25`
+                        // which should be the only way for us to end up with a const inference
+                        // variable after projection. If Rust ever gains support for this kind
+                        // of projection, this should *probably* be changed to
+                        // `self.tcx.mk_param_from_def(param)`
+                                        bug!("Found infer const: `{:?}` in opaque type: {:?}",
+                                             old_const, ty);
+                                    } else {
+                                        old_param.fold_with(self)
+                                    }
+                                }
+                                GenericArgKind::Lifetime(old_region) => {
+                                    if let RegionKind::ReVar(_) = old_region {
+                                        self.tcx.mk_param_from_def(param)
+                                    } else {
+                                        old_param.fold_with(self)
+                                    }
+                                }
+                            }
+                        });
+                        let new_ty = self.tcx.mk_opaque(def_id, new_substs);
+                        debug!("fixup_opaque_types: new type: {:?}", new_ty);
+                        new_ty
+                    } else {
+                        ty
+                    }
+                },
+                _ => ty.super_fold_with(self)
+            }
+        }
+    }
+
+    debug!("fixup_opaque_types({:?})", val);
+    val.fold_with(&mut FixupFolder { tcx })
+}
+
 fn typeck_tables_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::TypeckTables<'_> {
     // Closures' tables come from their outermost function,
     // as they are part of the same "inference environment".
@@ -911,6 +1016,8 @@ fn typeck_tables_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::TypeckTables<'_> {
                                                   param_env,
                                                   &fn_sig);
 
+            let fn_sig = fixup_opaque_types(tcx, &fn_sig);
+
             let fcx = check_fn(&inh, param_env, fn_sig, decl, id, body, None).0;
             fcx
         } else {

src/librustc_typeck/collect.rs

@@ -1617,11 +1617,18 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
                     ty::Param(_) => true,
                     _ => false,
                 };
-                if !substs.types().all(is_param) {
-                    self.tcx.sess.span_err(
-                        span,
-                        "defining opaque type use does not fully define opaque type",
-                    );
+                let bad_substs: Vec<_> = substs.types().enumerate()
+                    .filter(|(_, ty)| !is_param(ty)).collect();
+                if !bad_substs.is_empty() {
+                    let identity_substs = InternalSubsts::identity_for_item(self.tcx, self.def_id);
+                    for (i, bad_subst) in bad_substs {
+                        self.tcx.sess.span_err(
+                            span,
+                            &format!("defining opaque type use does not fully define opaque type: \
+                            generic parameter `{}` is specified as concrete type `{}`",
+                            identity_substs.type_at(i), bad_subst)
+                        );
+                    }
                 } else if let Some((prev_span, prev_ty, ref prev_indices)) = self.found {
                     let mut ty = concrete_type.walk().fuse();
                     let mut p_ty = prev_ty.walk().fuse();
@@ -2059,6 +2066,9 @@ fn explicit_predicates_of(
                 ty::print::with_no_queries(|| {
                     let substs = InternalSubsts::identity_for_item(tcx, def_id);
                     let opaque_ty = tcx.mk_opaque(def_id, substs);
+                    debug!("explicit_predicates_of({:?}): created opaque type {:?}",
+                        def_id, opaque_ty);
+
 
                     // Collect the bounds, i.e., the `A + B + 'c` in `impl A + B + 'c`.
                     let bounds = AstConv::compute_bounds(

src/test/ui/impl-trait/type-alias-generic-param.rs

@@ -0,0 +1,22 @@
+// Regression test for issue #59342
+// Checks that we properly detect defining uses of opaque
+// types in 'item' position when generic parameters are involved
+//
+// run-pass
+#![feature(type_alias_impl_trait)]
+
+trait Meow {
+    type MeowType;
+    fn meow(self) -> Self::MeowType;
+}
+
+impl<T, I> Meow for I
+    where I: Iterator<Item = T>
+{
+    type MeowType = impl Iterator<Item = T>;
+    fn meow(self) -> Self::MeowType {
+        self
+    }
+}
+
+fn main() {}

src/test/ui/type-alias-impl-trait/assoc-type-const.rs

@@ -0,0 +1,36 @@
+// Tests that we properly detect defining usages when using
+// const generics in an associated opaque type
+// check-pass
+
+#![feature(type_alias_impl_trait)]
+#![feature(const_generics)]
+//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash
+
+trait UnwrapItemsExt<const C: usize> {
+    type Iter;
+    fn unwrap_items(self) -> Self::Iter;
+}
+
+struct MyStruct<const C: usize> {}
+
+trait MyTrait<'a, const C: usize> {
+    type MyItem;
+    const MY_CONST: usize;
+}
+
+impl<'a, const C: usize> MyTrait<'a, {C}> for MyStruct<{C}> {
+    type MyItem = u8;
+    const MY_CONST: usize = C;
+}
+
+impl<'a, I, const C: usize> UnwrapItemsExt<{C}> for I
+where
+{
+    type Iter = impl MyTrait<'a, {C}>;
+
+    fn unwrap_items(self) -> Self::Iter {
+        MyStruct::<{C}> {}
+    }
+}
+
+fn main() {}

src/test/ui/type-alias-impl-trait/assoc-type-const.stderr

@@ -0,0 +1,8 @@
+warning: the feature `const_generics` is incomplete and may cause the compiler to crash
+  --> $DIR/assoc-type-const.rs:6:12
+   |
+LL | #![feature(const_generics)]
+   |            ^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(incomplete_features)]` on by default
+

src/test/ui/type-alias-impl-trait/assoc-type-lifetime.rs

@@ -0,0 +1,28 @@
+// Tests that we still detect defining usages when
+// lifetimes are used in an associated opaque type
+// check-pass
+
+#![feature(type_alias_impl_trait)]
+
+trait UnwrapItemsExt {
+    type Iter;
+    fn unwrap_items(self) -> Self::Iter;
+}
+
+struct MyStruct {}
+
+trait MyTrait<'a> {}
+
+impl<'a> MyTrait<'a> for MyStruct {}
+
+impl<'a, I> UnwrapItemsExt for I
+where
+{
+    type Iter = impl MyTrait<'a>;
+
+    fn unwrap_items(self) -> Self::Iter {
+        MyStruct {}
+    }
+}
+
+fn main() {}

src/test/ui/type-alias-impl-trait/bound_reduction2.stderr

@@ -1,4 +1,4 @@
-error: defining opaque type use does not fully define opaque type
+error: defining opaque type use does not fully define opaque type: generic parameter `V` is specified as concrete type `<T as TraitWithAssoc>::Assoc`
   --> $DIR/bound_reduction2.rs:17:1
    |
 LL | / fn foo_desugared<T: TraitWithAssoc>(_: T) -> Foo<T::Assoc> {

src/test/ui/type-alias-impl-trait/generic_nondefining_use.stderr

@@ -4,7 +4,7 @@ error: at least one trait must be specified
 LL | type Cmp<T> = impl 'static;
    |               ^^^^^^^^^^^^
 
-error: defining opaque type use does not fully define opaque type
+error: defining opaque type use does not fully define opaque type: generic parameter `T` is specified as concrete type `u32`
   --> $DIR/generic_nondefining_use.rs:11:1
    |
 LL | / fn cmp() -> Cmp<u32> {

src/test/ui/type-alias-impl-trait/issue-58887.rs

@@ -1,3 +1,5 @@
+// run-pass
+
 #![feature(type_alias_impl_trait)]
 
 trait UnwrapItemsExt {
@@ -11,11 +13,8 @@ where
     E: std::fmt::Debug,
 {
     type Iter = impl Iterator<Item = T>;
-    //~^ ERROR: could not find defining uses
 
     fn unwrap_items(self) -> Self::Iter {
-    //~^ ERROR: type parameter `T` is part of concrete type
-    //~| ERROR: type parameter `E` is part of concrete type
         self.map(|x| x.unwrap())
     }
 }

src/test/ui/type-alias-impl-trait/issue-58887.stderr

@@ -18,7 +18,7 @@ where
 {
     type BitsIter = IterBitsIter<T, E, u8>;
     fn iter_bits(self, n: u8) -> Self::BitsIter {
-    //~^ ERROR type parameter `E` is part of concrete type but not used
+    //~^ ERROR defining opaque type use does not fully define opaque type
         (0u8..n)
             .rev()
             .map(move |shift| ((self >> T::from(shift)) & T::from(1)).try_into().unwrap())

src/test/ui/type-alias-impl-trait/issue-60564.rs

@@ -1,8 +1,7 @@
-error: type parameter `E` is part of concrete type but not used in parameter list for the `impl Trait` type alias
-  --> $DIR/issue-60564.rs:20:49
+error: defining opaque type use does not fully define opaque type: generic parameter `I` is specified as concrete type `u8`
+  --> $DIR/issue-60564.rs:20:5
    |
-LL |       fn iter_bits(self, n: u8) -> Self::BitsIter {
-   |  _________________________________________________^
+LL | /     fn iter_bits(self, n: u8) -> Self::BitsIter {
 LL | |
 LL | |         (0u8..n)
 LL | |             .rev()

src/test/ui/type-alias-impl-trait/issue-60564.stderr

@@ -1,4 +1,4 @@
-error: defining opaque type use does not fully define opaque type
+error: defining opaque type use does not fully define opaque type: generic parameter `U` is specified as concrete type `u32`
   --> $DIR/not_a_defining_use.rs:9:1
    |
 LL | / fn two<T: Debug>(t: T) -> Two<T, u32> {