From c0889a6005ecac3d304009e734baa97b37b534d2 Mon Sep 17 00:00:00 2001 From: Boxy Date: Sun, 6 Nov 2022 13:39:13 +0000 Subject: [PATCH 1/3] fixyfixfix --- .../src/collect/lifetimes.rs | 92 ++++++++++++++++++- src/test/ui/issues/issue-47511.stderr | 18 ---- .../downgraded_to_early_through_alias.rs | 24 +++++ .../issue-47511.rs | 7 +- .../mismatched_arg_count.rs | 12 +++ .../mismatched_arg_count.stderr | 17 ++++ 6 files changed, 141 insertions(+), 29 deletions(-) delete mode 100644 src/test/ui/issues/issue-47511.stderr create mode 100644 src/test/ui/late-bound-lifetimes/downgraded_to_early_through_alias.rs rename src/test/ui/{issues => late-bound-lifetimes}/issue-47511.rs (52%) create mode 100644 src/test/ui/late-bound-lifetimes/mismatched_arg_count.rs create mode 100644 src/test/ui/late-bound-lifetimes/mismatched_arg_count.stderr diff --git a/compiler/rustc_hir_analysis/src/collect/lifetimes.rs b/compiler/rustc_hir_analysis/src/collect/lifetimes.rs index 3d07f3fbc674d..b9583bd2244b9 100644 --- a/compiler/rustc_hir_analysis/src/collect/lifetimes.rs +++ b/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,44 @@ 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 { + 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, } - 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 +1882,56 @@ 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 }, + )) => { + // If this is a top level type alias attempt to "look through" it to see if the args + // are constrained, instead of assuming they are and inserting all the lifetimes. + // This is necessary for the following case: + // ``` + // type Alias<'a, T> = >::Assoc; + // fn foo<'a>(_: Alias<'a, ()>) -> Alias<'a, ()> { ... } + // ``` + // 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`. + + 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 diff --git a/src/test/ui/issues/issue-47511.stderr b/src/test/ui/issues/issue-47511.stderr deleted file mode 100644 index 9998ee0e8d0c6..0000000000000 --- a/src/test/ui/issues/issue-47511.stderr +++ /dev/null @@ -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`. diff --git a/src/test/ui/late-bound-lifetimes/downgraded_to_early_through_alias.rs b/src/test/ui/late-bound-lifetimes/downgraded_to_early_through_alias.rs new file mode 100644 index 0000000000000..e56a34218e231 --- /dev/null +++ b/src/test/ui/late-bound-lifetimes/downgraded_to_early_through_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 >::Assoc) -> >::Assoc2; +} + +impl<'a> Gats<'a> for () { + type Assoc = &'a u32; + type Assoc2 = (); +} + +type GatsAssoc<'a, T> = >::Assoc; +type GatsAssoc2<'a, T> = >::Assoc2; + +impl Trait for () { + fn foo<'a>(_: &mut GatsAssoc<'a, Self>) -> GatsAssoc2<'a, Self> {} +} + +fn main() {} diff --git a/src/test/ui/issues/issue-47511.rs b/src/test/ui/late-bound-lifetimes/issue-47511.rs similarity index 52% rename from src/test/ui/issues/issue-47511.rs rename to src/test/ui/late-bound-lifetimes/issue-47511.rs index eb4860e75d758..7894435154082 100644 --- a/src/test/ui/issues/issue-47511.rs +++ b/src/test/ui/late-bound-lifetimes/issue-47511.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!() diff --git a/src/test/ui/late-bound-lifetimes/mismatched_arg_count.rs b/src/test/ui/late-bound-lifetimes/mismatched_arg_count.rs new file mode 100644 index 0000000000000..0b331e2039f25 --- /dev/null +++ b/src/test/ui/late-bound-lifetimes/mismatched_arg_count.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> = >::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() {} diff --git a/src/test/ui/late-bound-lifetimes/mismatched_arg_count.stderr b/src/test/ui/late-bound-lifetimes/mismatched_arg_count.stderr new file mode 100644 index 0000000000000..3704d9bb957ed --- /dev/null +++ b/src/test/ui/late-bound-lifetimes/mismatched_arg_count.stderr @@ -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> = >::Assoc; + | ^^^^^ -- + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0107`. From 49be827dcae4b226a2cb6f56e49083d49908fa6f Mon Sep 17 00:00:00 2001 From: Boxy Date: Tue, 8 Nov 2022 21:59:58 +0000 Subject: [PATCH 2/3] comment --- .../src/collect/lifetimes.rs | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect/lifetimes.rs b/compiler/rustc_hir_analysis/src/collect/lifetimes.rs index b9583bd2244b9..c64177eea3f83 100644 --- a/compiler/rustc_hir_analysis/src/collect/lifetimes.rs +++ b/compiler/rustc_hir_analysis/src/collect/lifetimes.rs @@ -1834,6 +1834,27 @@ fn is_late_bound_map(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<&FxIndexSet< debug!(?late_bound); return Some(tcx.arena.alloc(late_bound)); + /// 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> = >::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]>, } @@ -1886,17 +1907,8 @@ fn is_late_bound_map(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<&FxIndexSet< None, hir::Path { res: Res::Def(DefKind::TyAlias, alias_def), segments, span }, )) => { - // If this is a top level type alias attempt to "look through" it to see if the args - // are constrained, instead of assuming they are and inserting all the lifetimes. - // This is necessary for the following case: - // ``` - // type Alias<'a, T> = >::Assoc; - // fn foo<'a>(_: Alias<'a, ()>) -> Alias<'a, ()> { ... } - // ``` - // 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`. - + // 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(), From 983a90d7160b5e6054827f42c0ecf9aa52cebcd4 Mon Sep 17 00:00:00 2001 From: Boxy Date: Tue, 8 Nov 2022 22:15:40 +0000 Subject: [PATCH 3/3] tests --- .../auxiliary/upstream_alias.rs | 5 +++++ .../ui/late-bound-lifetimes/cross_crate_alias.rs | 10 ++++++++++ .../late_bound_through_alias.rs | 16 ++++++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 src/test/ui/late-bound-lifetimes/auxiliary/upstream_alias.rs create mode 100644 src/test/ui/late-bound-lifetimes/cross_crate_alias.rs create mode 100644 src/test/ui/late-bound-lifetimes/late_bound_through_alias.rs diff --git a/src/test/ui/late-bound-lifetimes/auxiliary/upstream_alias.rs b/src/test/ui/late-bound-lifetimes/auxiliary/upstream_alias.rs new file mode 100644 index 0000000000000..5b9dc0e43087c --- /dev/null +++ b/src/test/ui/late-bound-lifetimes/auxiliary/upstream_alias.rs @@ -0,0 +1,5 @@ +pub trait Trait<'a> { + type Assoc; +} + +pub type Alias<'a, T> = >::Assoc; diff --git a/src/test/ui/late-bound-lifetimes/cross_crate_alias.rs b/src/test/ui/late-bound-lifetimes/cross_crate_alias.rs new file mode 100644 index 0000000000000..4154c27924323 --- /dev/null +++ b/src/test/ui/late-bound-lifetimes/cross_crate_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() {} diff --git a/src/test/ui/late-bound-lifetimes/late_bound_through_alias.rs b/src/test/ui/late-bound-lifetimes/late_bound_through_alias.rs new file mode 100644 index 0000000000000..91839673c1f7a --- /dev/null +++ b/src/test/ui/late-bound-lifetimes/late_bound_through_alias.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; +}