Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 99 additions & 5 deletions compiler/rustc_hir_analysis/src/collect/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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 {
BoxyUwU marked this conversation as resolved.
Show resolved Hide resolved
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(
Expand All @@ -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
Expand Down
18 changes: 0 additions & 18 deletions src/test/ui/issues/issue-47511.stderr

This file was deleted.

5 changes: 5 additions & 0 deletions src/test/ui/late-bound-lifetimes/auxiliary/upstream_alias.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub trait Trait<'a> {
type Assoc;
}

pub type Alias<'a, T> = <T as Trait<'a>>::Assoc;
10 changes: 10 additions & 0 deletions src/test/ui/late-bound-lifetimes/cross_crate_alias.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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!()
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/late-bound-lifetimes/late_bound_through_alias.rs
Original file line number Diff line number Diff line change
@@ -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;
}
12 changes: 12 additions & 0 deletions src/test/ui/late-bound-lifetimes/mismatched_arg_count.rs
Original file line number Diff line number Diff line change
@@ -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() {}
17 changes: 17 additions & 0 deletions src/test/ui/late-bound-lifetimes/mismatched_arg_count.stderr
Original file line number Diff line number Diff line change
@@ -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`.