1
Fork 0

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

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

fixes #47511
fixes #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, [#84385](https://github.com/rust-lang/rust/pull/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? [#103448](https://github.com/rust-lang/rust/pull/103448) places all lifetimes in `Generics` regardless of late boundness so that may be a good step towards making this possible.
This commit is contained in:
Manish Goregaokar 2022-11-08 21:03:51 -05:00 committed by GitHub
commit f162e3a1b1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 184 additions and 29 deletions

View file

@ -18,7 +18,7 @@ use rustc_middle::bug;
use rustc_middle::hir::map::Map; use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter; use rustc_middle::hir::nested_filter;
use rustc_middle::middle::resolve_lifetime::*; 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::def_id::DefId;
use rustc_span::symbol::{sym, Ident}; use rustc_span::symbol::{sym, Ident};
use rustc_span::Span; 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 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 { for arg_ty in decl.inputs {
constrained_by_input.visit_ty(arg_ty); 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); debug!(?late_bound);
return Some(tcx.arena.alloc(late_bound)); return Some(tcx.arena.alloc(late_bound));
#[derive(Default)] /// Visits a `ty::Ty` collecting information about what generic parameters are constrained.
struct ConstrainedCollector { ///
/// 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>, 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>) { fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
match ty.kind { match ty.kind {
hir::TyKind::Path( hir::TyKind::Path(
@ -1850,6 +1903,47 @@ fn is_late_bound_map(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<&FxIndexSet<
// (defined above) // (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)) => { hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) => {
// consider only the lifetimes on the final // consider only the lifetimes on the final
// segment; I am not sure it's even currently // segment; I am not sure it's even currently

View file

@ -1,18 +0,0 @@
error[E0581]: return type references an anonymous lifetime, which is not constrained by the fn input types
--> $DIR/issue-47511.rs:8:15
|
LL | fn f(_: X) -> X {
| ^
|
= note: lifetimes appearing in an associated or opaque type are not considered constrained
= note: consider introducing a named lifetime parameter
error[E0581]: return type references lifetime `'a`, which is not constrained by the fn input types
--> $DIR/issue-47511.rs:12:23
|
LL | fn g<'a>(_: X<'a>) -> X<'a> {
| ^^^^^
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0581`.

View file

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

View file

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

View file

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

View file

@ -1,9 +1,4 @@
// check-fail // check-pass
// 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.
fn f(_: X) -> X { fn f(_: X) -> X {
unimplemented!() unimplemented!()

View file

@ -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;
}

View file

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

View file

@ -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`.