Rollup merge of rust-lang#100508 - BoxyUwU:make_less_things_late_bound, r=nikomatsakis

avoid making substs of type aliases late bound when used as fn args

fixes rust-lang#47511
fixes rust-lang#85533
(although I did not know theses issues existed when i was working on this 🙃)

currently `Alias<...>` is treated the same as `Struct<...>` when deciding if generics should be late bound or early bound but this is not correct as `Alias` might normalize to a projection which does not constrain the generics.

I think this needs more tests before merging
more explanation of PR [here](https://hackmd.io/v44a-QVjTIqqhK9uretyQg?view)

Hackmd inline for future readers:
---

This assumes reader is familiar with the concept of early/late bound lifetimes. There's a section on rustc-dev-guide if not (although i think some details are a bit out of date)

## problem & background

Not all lifetimes on a fn can be late bound:
```rust
fn foo<'a>() -> &'a ();
impl<'a> Fn<()> for FooFnDef {
    type Output = &'a (); // uh oh unconstrained lifetime
}
```
so we make make them early bound
```rust
fn foo<'a>() -> &'a ();
impl<'a> Fn<()> for FooFnDef<'a> {// wow look at all that lifetimey
     type Output = &'a ();
}
```
(Closures have the same constraint however it is not enforced leading to soundness bugs, [rust-lang#84385](rust-lang#84385) implements this "downgrading late bound to early bound" for closures)

lifetimes on fn items are only late bound when they are "constrained" by the fn args:
```rust
fn foo<'a>(_: &'a ()) -> &'a ();
//               late bound, not present on `FooFnItem`
//               vv
impl<'a> Trait<(&'a (),)> for FooFnItem {
    type Output = &'a ();
}

// projections do not constrain inputs
fn bar<'a, T: Trait>(_: <T as Trait<'a>>::Assoc) -> &'a (); //  early bound
                                                            //  vv
impl<'a, T: Trait> Fn<(<T as Trait<'a>>::Assoc,)> for BarFnItem<'a, T> {
    type Output = &'a ();
}
```

current logic for determining if inputs "constrain" a lifetime works off of HIR so does not normalize aliases. It also assumes that any path with no self type constrains all its substs (i.e. `Foo<'a, u32>` has no self type but `T::Assoc` does). This falls apart for top level type aliases (see linked issues):

```rust
type Alias<'a, T> = <T as Trait<'a>>::Assoc;
//                      wow look its a path with no self type uwu
//                      i bet that constrains `'a` so it should be latebound
//                      vvvvvvvvvvv
fn foo<'a, T: Trait>(_: Alias<'a, T>) -> &'a ();
//                     `Alias` normalized to make things clearer
//                     vvvvvvvvvvvvvvvvvvvvvvv
impl<'a, T: Trait> Fn<(<T as Trait<'a>>::Assoc,)> for FooFnDef<T> {
    type Output = &'a ();
    // oh no `'a` isnt constrained wah wah waaaah *trumbone noises*
    // i think, idk what musical instrument that is
}
```

## solution

The PR solves this by having the hir visitor that checks for lifetimes in constraining uses check if the path is a `DefKind::Alias`. If it is we ""normalize"" it by calling `type_of` and walking the returned type. This is a bit hacky as it requires a mapping between the substs on the path in hir, and the generics of the `type Alias<...>` which is on the ty layer.

Alternative solutions may involve calculating the "late boundness" of lifetimes after/during astconv rather than relying on hir at all. We already have code to determine whether a lifetime SHOULD be late bound or not as this is currently how the error for `fn foo<'a, T: Trait>(_: Alias<'a, T>) -> &'a ();` gets emitted.

It is probably not possible to do this right now, late boundness is used by `generics_of` and `gather_explicit_predicates_of` as we currently do not put late bound lifetimes in `Generics`. Although this seems sus to me as the long term goal is to make all generics late bound which would result in `generics_of(function)` being empty? [rust-lang#103448](rust-lang#103448) places all lifetimes in `Generics` regardless of late boundness so that may be a good step towards making this possible.

Author: Manishearth

compiler/rustc_hir_analysis/src/collect/lifetimes.rs

@@ -18,7 +18,7 @@ use rustc_middle::bug;
 use rustc_middle::hir::map::Map;
 use rustc_middle::hir::nested_filter;
 use rustc_middle::middle::resolve_lifetime::*;
-use rustc_middle::ty::{self, DefIdTree, TyCtxt};
+use rustc_middle::ty::{self, DefIdTree, TyCtxt, TypeSuperVisitable, TypeVisitor};
 use rustc_span::def_id::DefId;
 use rustc_span::symbol::{sym, Ident};
 use rustc_span::Span;
@@ -1781,7 +1781,7 @@ fn is_late_bound_map(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<&FxIndexSet<
 
     let mut late_bound = FxIndexSet::default();
 
-    let mut constrained_by_input = ConstrainedCollector::default();
+    let mut constrained_by_input = ConstrainedCollector { regions: Default::default(), tcx };
     for arg_ty in decl.inputs {
         constrained_by_input.visit_ty(arg_ty);
     }
@@ -1834,12 +1834,65 @@ fn is_late_bound_map(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<&FxIndexSet<
     debug!(?late_bound);
     return Some(tcx.arena.alloc(late_bound));
 
-    #[derive(Default)]
-    struct ConstrainedCollector {
+    /// Visits a `ty::Ty` collecting information about what generic parameters are constrained.
+    ///
+    /// The visitor does not operate on `hir::Ty` so that it can be called on the rhs of a `type Alias<...> = ...;`
+    /// which may live in a separate crate so there would not be any hir available. Instead we use the `type_of`
+    /// query to obtain a `ty::Ty` which will be present even in cross crate scenarios. It also naturally
+    /// handles cycle detection as we go through the query system.
+    ///
+    /// This is necessary in the first place for the following case:
+    /// ```
+    /// type Alias<'a, T> = <T as Trait<'a>>::Assoc;
+    /// fn foo<'a>(_: Alias<'a, ()>) -> Alias<'a, ()> { ... }
+    /// ```
+    ///
+    /// If we conservatively considered `'a` unconstrained then we could break users who had written code before
+    /// we started correctly handling aliases. If we considered `'a` constrained then it would become late bound
+    /// causing an error during astconv as the `'a` is not constrained by the input type `<() as Trait<'a>>::Assoc`
+    /// but appears in the output type `<() as Trait<'a>>::Assoc`.
+    ///
+    /// We must therefore "look into" the `Alias` to see whether we should consider `'a` constrained or not.
+    ///
+    /// See #100508 #85533 #47511 for additional context
+    struct ConstrainedCollectorPostAstConv {
+        arg_is_constrained: Box<[bool]>,
+    }
+
+    use std::ops::ControlFlow;
+    use ty::Ty;
+    impl<'tcx> TypeVisitor<'tcx> for ConstrainedCollectorPostAstConv {
+        fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<!> {
+            match t.kind() {
+                ty::Param(param_ty) => {
+                    self.arg_is_constrained[param_ty.index as usize] = true;
+                }
+                ty::Projection(_) => return ControlFlow::Continue(()),
+                _ => (),
+            }
+            t.super_visit_with(self)
+        }
+
+        fn visit_const(&mut self, _: ty::Const<'tcx>) -> ControlFlow<!> {
+            ControlFlow::Continue(())
+        }
+
+        fn visit_region(&mut self, r: ty::Region<'tcx>) -> ControlFlow<!> {
+            debug!("r={:?}", r.kind());
+            if let ty::RegionKind::ReEarlyBound(region) = r.kind() {
+                self.arg_is_constrained[region.index as usize] = true;
+            }
+
+            ControlFlow::Continue(())
+        }
+    }
+
+    struct ConstrainedCollector<'tcx> {
+        tcx: TyCtxt<'tcx>,
         regions: FxHashSet<LocalDefId>,
     }
 
-    impl<'v> Visitor<'v> for ConstrainedCollector {
+    impl<'v> Visitor<'v> for ConstrainedCollector<'_> {
         fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
             match ty.kind {
                 hir::TyKind::Path(
@@ -1850,6 +1903,47 @@ fn is_late_bound_map(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<&FxIndexSet<
                     // (defined above)
                 }
 
+                hir::TyKind::Path(hir::QPath::Resolved(
+                    None,
+                    hir::Path { res: Res::Def(DefKind::TyAlias, alias_def), segments, span },
+                )) => {
+                    // See comments on `ConstrainedCollectorPostAstConv` for why this arm does not just consider
+                    // substs to be unconstrained.
+                    let generics = self.tcx.generics_of(alias_def);
+                    let mut walker = ConstrainedCollectorPostAstConv {
+                        arg_is_constrained: vec![false; generics.params.len()].into_boxed_slice(),
+                    };
+                    walker.visit_ty(self.tcx.type_of(alias_def));
+
+                    match segments.last() {
+                        Some(hir::PathSegment { args: Some(args), .. }) => {
+                            let tcx = self.tcx;
+                            for constrained_arg in
+                                args.args.iter().enumerate().flat_map(|(n, arg)| {
+                                    match walker.arg_is_constrained.get(n) {
+                                        Some(true) => Some(arg),
+                                        Some(false) => None,
+                                        None => {
+                                            tcx.sess.delay_span_bug(
+                                                *span,
+                                                format!(
+                                                    "Incorrect generic arg count for alias {:?}",
+                                                    alias_def
+                                                ),
+                                            );
+                                            None
+                                        }
+                                    }
+                                })
+                            {
+                                self.visit_generic_arg(constrained_arg);
+                            }
+                        }
+                        Some(_) => (),
+                        None => bug!("Path with no segments or self type"),
+                    }
+                }
+
                 hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) => {
                     // consider only the lifetimes on the final
                     // segment; I am not sure it's even currently

src/test/ui/issues/issue-47511.stderr

@@ -0,0 +1,5 @@
+pub trait Trait<'a> {
+    type Assoc;
+}
+
+pub type Alias<'a, T> = <T as Trait<'a>>::Assoc;

src/test/ui/late-bound-lifetimes/auxiliary/upstream_alias.rs

@@ -0,0 +1,10 @@
+// aux-build:upstream_alias.rs
+// check-pass
+
+extern crate upstream_alias;
+
+fn foo<'a, T: for<'b> upstream_alias::Trait<'b>>(_: upstream_alias::Alias<'a, T>) -> &'a () {
+    todo!()
+}
+
+fn main() {}

src/test/ui/late-bound-lifetimes/cross_crate_alias.rs

@@ -0,0 +1,24 @@
+// check-pass
+
+trait Gats<'a> {
+    type Assoc;
+    type Assoc2;
+}
+
+trait Trait: for<'a> Gats<'a> {
+    fn foo<'a>(_: &mut <Self as Gats<'a>>::Assoc) -> <Self as Gats<'a>>::Assoc2;
+}
+
+impl<'a> Gats<'a> for () {
+    type Assoc = &'a u32;
+    type Assoc2 = ();
+}
+
+type GatsAssoc<'a, T> = <T as Gats<'a>>::Assoc;
+type GatsAssoc2<'a, T> = <T as Gats<'a>>::Assoc2;
+
+impl Trait for () {
+    fn foo<'a>(_: &mut GatsAssoc<'a, Self>) -> GatsAssoc2<'a, Self> {}
+}
+
+fn main() {}

src/test/ui/late-bound-lifetimes/downgraded_to_early_through_alias.rs

@@ -1,9 +1,4 @@
-// check-fail
-// known-bug: #47511
-
-// Regression test for #47511: anonymous lifetimes can appear
-// unconstrained in a return type, but only if they appear just once
-// in the input, as the input to a projection.
+// check-pass
 
 fn f(_: X) -> X {
     unimplemented!()

src/test/ui/issues/issue-47511.rs src/test/ui/late-bound-lifetimes/issue-47511.rs

@@ -0,0 +1,16 @@
+// check-pass
+
+fn f(_: X) -> X {
+    unimplemented!()
+}
+
+fn g<'a>(_: X<'a>) -> X<'a> {
+    unimplemented!()
+}
+
+type X<'a> = &'a ();
+
+fn main() {
+    let _: for<'a> fn(X<'a>) -> X<'a> = g;
+    let _: for<'a> fn(X<'a>) -> X<'a> = f;
+}

src/test/ui/late-bound-lifetimes/late_bound_through_alias.rs

@@ -0,0 +1,12 @@
+// ensures that we don't ICE when there are too many args supplied to the alias.
+
+trait Trait<'a> {
+    type Assoc;
+}
+
+type Alias<'a, T> = <T as Trait<'a>>::Assoc;
+
+fn bar<'a, T: Trait<'a>>(_: Alias<'a, 'a, T>) {}
+//~^ error: this type alias takes 1 lifetime argument but 2 lifetime arguments were supplied
+
+fn main() {}

src/test/ui/late-bound-lifetimes/mismatched_arg_count.rs

@@ -0,0 +1,17 @@
+error[E0107]: this type alias takes 1 lifetime argument but 2 lifetime arguments were supplied
+  --> $DIR/mismatched_arg_count.rs:9:29
+   |
+LL | fn bar<'a, T: Trait<'a>>(_: Alias<'a, 'a, T>) {}
+   |                             ^^^^^     -- help: remove this lifetime argument
+   |                             |
+   |                             expected 1 lifetime argument
+   |
+note: type alias defined here, with 1 lifetime parameter: `'a`
+  --> $DIR/mismatched_arg_count.rs:7:6
+   |
+LL | type Alias<'a, T> = <T as Trait<'a>>::Assoc;
+   |      ^^^^^ --
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0107`.