From ba3d22ed6685f3ddbfda29edc20b93810a641db8 Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Mon, 12 Apr 2021 09:12:10 -0400 Subject: [PATCH 1/7] Precompute inverse binder depth --- compiler/rustc_resolve/src/late/lifetimes.rs | 177 ++++++++----------- 1 file changed, 73 insertions(+), 104 deletions(-) diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index fc1ea4ec84629..88ceb23b72f5b 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -249,6 +249,8 @@ enum Scope<'a> { /// requires binders of nested trait refs to be merged. from_poly_trait_ref: bool, + binder_depth: u32, + /// The late bound vars for a given item are stored by `HirId` to be /// queried later. However, if we enter an elision scope, we have to /// later append the elided bound vars to the list and need to know what @@ -345,6 +347,7 @@ impl<'a> fmt::Debug for TruncatedScopeDebug<'a> { track_lifetime_uses, opaque_type_parent, from_poly_trait_ref, + binder_depth, hir_id, s: _, } => f @@ -354,6 +357,7 @@ impl<'a> fmt::Debug for TruncatedScopeDebug<'a> { .field("track_lifetime_uses", track_lifetime_uses) .field("opaque_type_parent", opaque_type_parent) .field("from_poly_trait_ref", from_poly_trait_ref) + .field("binder_depth", binder_depth) .field("hir_id", hir_id) .field("s", &"..") .finish(), @@ -618,6 +622,45 @@ fn late_region_as_bound_region<'tcx>(tcx: TyCtxt<'tcx>, region: &Region) -> ty:: } } +impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { + fn depth(&self, concanetate: bool) -> u32 { + let mut passed_boundary = false; + let mut scope = self.scope; + loop { + match *scope { + Scope::Root => { + break 0; + } + + Scope::TraitRefBoundary { s, .. } => { + passed_boundary = true; + scope = s; + } + + Scope::Binder { binder_depth, from_poly_trait_ref, .. } => { + break if concanetate { + if passed_boundary || !from_poly_trait_ref { + binder_depth + 1 + } else { + binder_depth + } + } else { + binder_depth + 1 + }; + } + + Scope::Elision { s, .. } + | Scope::ObjectLifetimeDefault { s, .. } + | Scope::TraitRefHackInner { s, .. } + | Scope::Supertrait { s, .. } + | Scope::Body { s, .. } => { + scope = s; + } + } + } + } +} + impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { type Map = Map<'tcx>; @@ -676,6 +719,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: false, from_poly_trait_ref: false, + binder_depth: self.depth(false), }; self.with(scope, move |_old_scope, this| { intravisit::walk_fn(this, fk, fd, b, s, hir_id) @@ -801,6 +845,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { opaque_type_parent: true, track_lifetime_uses, from_poly_trait_ref: false, + binder_depth: self.depth(false), s: ROOT_SCOPE, }; self.with(scope, |old_scope, this| { @@ -870,6 +915,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: false, from_poly_trait_ref: false, + binder_depth: self.depth(false), }; self.with(scope, |old_scope, this| { // a bare fn has no bounds, so everything @@ -1063,6 +1109,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: false, from_poly_trait_ref: false, + binder_depth: this.depth(false), }; this.with(scope, |_old_scope, this| { this.visit_generics(generics); @@ -1083,6 +1130,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: false, from_poly_trait_ref: false, + binder_depth: self.depth(false), }; self.with(scope, |_old_scope, this| { let scope = Scope::TraitRefBoundary { s: this.scope }; @@ -1142,6 +1190,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: true, from_poly_trait_ref: false, + binder_depth: self.depth(false), }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -1211,6 +1260,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: true, from_poly_trait_ref: false, + binder_depth: self.depth(false), }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -1324,6 +1374,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: false, from_poly_trait_ref: true, + binder_depth: this.depth(false), }; this.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &bound_generic_params); @@ -1370,6 +1421,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: false, from_poly_trait_ref: false, + binder_depth: self.depth(false), }; self.with(scope, |_, this| { intravisit::walk_param_bound(this, bound); @@ -1516,6 +1568,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { track_lifetime_uses: true, opaque_type_parent: false, from_poly_trait_ref: true, + binder_depth: self.depth(true), }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &trait_ref.bound_generic_params); @@ -2266,6 +2319,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { opaque_type_parent: true, track_lifetime_uses: false, from_poly_trait_ref: false, + binder_depth: self.depth(false), }; self.with(scope, move |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -2323,7 +2377,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { // given name or we run out of scopes. // search. let mut late_depth = 0; - let mut in_poly_trait_ref = false; + let mut first_binder_depth = None; let mut scope = self.scope; let mut outermost_body = None; let result = loop { @@ -2341,25 +2395,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { break None; } - Scope::TraitRefBoundary { s, .. } => { - // We've exited nested poly trait refs; mark that we are no longer in nested trait refs. - // We don't increase the late depth because this isn't a `Binder` scope. - // - // This came up in #83737, which boiled down to a case like this: - // - // ``` - // F: for<> Fn(&()) -> Box Future + Unpin>, - // // ^^^^^ - - // ``` - // - // Here, as we traverse upwards from the `dyn for<>` binder, we want to reset `in_poly_trait_ref` - // to false, so that we avoid excess contaenation when we encounter the outer `for<>` binder. - in_poly_trait_ref = false; - scope = s; - } - - Scope::Binder { ref lifetimes, from_poly_trait_ref, s, .. } => { + Scope::Binder { ref lifetimes, s, binder_depth, .. } => { match lifetime_ref.name { LifetimeName::Param(param_name) => { if let Some(&def) = lifetimes.get(¶m_name.normalize_to_macros_2_0()) @@ -2369,47 +2405,16 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } _ => bug!("expected LifetimeName::Param"), } - - match (from_poly_trait_ref, in_poly_trait_ref) { - // This is the first binder we see that is a poly trait ref; add one to the - // late depth and mark that we're potentially in nested trait refs. - (true, false) => { - in_poly_trait_ref = true; - late_depth += 1; - } - // We've already seen a binder that is a poly trait ref and this one is too, - // that means that they are nested and we are concatenating the bound vars; - // don't increase the late depth. - // - // This happens specifically with associated trait bounds like the following: - // - // ``` - // for<'a> T: Iterator Foo<'a, 'b>> - // ``` - // - // In this case, as we traverse `for<'b>`, we would increment `late_depth` but - // set `in_poly_trait_ref` to true. Then when we traverse `for<'a>`, we would - // not increment `late_depth` again. (NB: Niko thinks this logic is actually - // wrong.) - (true, true) => {} - // We've exited nested poly trait refs; add one to the late depth and mark - // that we are no longer in nested trait refs - (false, true) => { - in_poly_trait_ref = false; - late_depth += 1; - } - // Any other kind of nested binders: just increase late depth. - (false, false) => { - late_depth += 1; - } - } + first_binder_depth = first_binder_depth.or(Some(binder_depth)); + late_depth = first_binder_depth.unwrap_or(binder_depth) - binder_depth + 1; scope = s; } Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } | Scope::TraitRefHackInner { s, .. } - | Scope::Supertrait { s, .. } => { + | Scope::Supertrait { s, .. } + | Scope::TraitRefBoundary { s, .. } => { scope = s; } } @@ -3112,7 +3117,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { let span = lifetime_refs[0].span; let mut late_depth = 0; - let mut in_poly_trait_ref = false; + let mut first_binder_depth = None; let mut scope = self.scope; let mut lifetime_names = FxHashSet::default(); let mut lifetime_spans = vec![]; @@ -3123,14 +3128,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::Root => break None, - Scope::TraitRefBoundary { s, .. } => { - // We've exited nested poly trait refs; mark that we are no longer in nested trait refs. - // We don't increase the late depth because this isn't a `Binder` scope - in_poly_trait_ref = false; - scope = s; - } - - Scope::Binder { s, ref lifetimes, from_poly_trait_ref, .. } => { + Scope::Binder { s, ref lifetimes, binder_depth, .. } => { // collect named lifetimes for suggestions for name in lifetimes.keys() { if let hir::ParamName::Plain(name) = name { @@ -3138,21 +3136,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { lifetime_spans.push(name.span); } } - // See comments in `resolve_lifetime_ref` - match (from_poly_trait_ref, in_poly_trait_ref) { - (true, false) => { - in_poly_trait_ref = true; - late_depth += 1; - } - (true, true) => {} - (false, true) => { - in_poly_trait_ref = false; - late_depth += 1; - } - (false, false) => { - late_depth += 1; - } - } + first_binder_depth = first_binder_depth.or(Some(binder_depth)); + late_depth = first_binder_depth.unwrap_or(binder_depth) - binder_depth + 1; scope = s; } @@ -3202,7 +3187,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::ObjectLifetimeDefault { s, .. } | Scope::TraitRefHackInner { s, .. } - | Scope::Supertrait { s, .. } => { + | Scope::Supertrait { s, .. } + | Scope::TraitRefBoundary { s, .. } => { scope = s; } } @@ -3308,32 +3294,13 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { fn resolve_object_lifetime_default(&mut self, lifetime_ref: &'tcx hir::Lifetime) { debug!("resolve_object_lifetime_default(lifetime_ref={:?})", lifetime_ref); let mut late_depth = 0; - let mut in_poly_trait_ref = false; + let mut first_binder_depth = None; let mut scope = self.scope; let lifetime = loop { match *scope { - Scope::TraitRefBoundary { s, .. } => { - // We've exited nested poly trait refs; mark that we are no longer in nested trait refs. - // We don't increase the late depth because this isn't a `Binder` scope - in_poly_trait_ref = false; - scope = s; - } - - Scope::Binder { s, from_poly_trait_ref, .. } => { - match (from_poly_trait_ref, in_poly_trait_ref) { - (true, false) => { - in_poly_trait_ref = true; - late_depth += 1; - } - (true, true) => {} - (false, true) => { - in_poly_trait_ref = false; - late_depth += 1; - } - (false, false) => { - late_depth += 1; - } - } + Scope::Binder { s, binder_depth, .. } => { + first_binder_depth = first_binder_depth.or(Some(binder_depth)); + late_depth = first_binder_depth.unwrap_or(binder_depth) - binder_depth + 1; scope = s; } @@ -3343,7 +3310,9 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::ObjectLifetimeDefault { lifetime: Some(l), .. } => break l, - Scope::TraitRefHackInner { s, .. } | Scope::Supertrait { s, .. } => { + Scope::TraitRefHackInner { s, .. } + | Scope::Supertrait { s, .. } + | Scope::TraitRefBoundary { s, .. } => { scope = s; } } From 32942ab8078dd9c266aa5886f98ab32eda310749 Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Mon, 12 Apr 2021 09:26:39 -0400 Subject: [PATCH 2/7] A non-minimal set of TraitRefBoundarys to work on removing from_poly_trait_ref --- compiler/rustc_resolve/src/late/lifetimes.rs | 177 ++++++++++--------- 1 file changed, 93 insertions(+), 84 deletions(-) diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index 88ceb23b72f5b..4e3e4f2c2bcb5 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -638,6 +638,9 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } Scope::Binder { binder_depth, from_poly_trait_ref, .. } => { + if concanetate && !passed_boundary && !from_poly_trait_ref { + bug!("{:?}", self.scope); + } break if concanetate { if passed_boundary || !from_poly_trait_ref { binder_depth + 1 @@ -850,7 +853,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); - intravisit::walk_item(this, item); + let scope = Scope::TraitRefBoundary { s: this.scope }; + this.with(scope, |_, this| { + intravisit::walk_item(this, item); + }); }); self.missing_named_lifetime_spots.pop(); } @@ -985,9 +991,12 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { // Elided lifetimes are not allowed in non-return // position impl Trait - let scope = Scope::Elision { elide: Elide::Forbid, s: self.scope }; + let scope = Scope::TraitRefBoundary { s: self.scope }; self.with(scope, |_, this| { - intravisit::walk_item(this, opaque_ty); + let scope = Scope::Elision { elide: Elide::Forbid, s: this.scope }; + this.with(scope, |_, this| { + intravisit::walk_item(this, opaque_ty); + }) }); return; @@ -1320,93 +1329,93 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { if !self.trait_definition_only { check_mixed_explicit_and_in_band_defs(self.tcx, &generics.params); } - for param in generics.params { - match param.kind { - GenericParamKind::Lifetime { .. } => {} - GenericParamKind::Type { ref default, .. } => { - walk_list!(self, visit_param_bound, param.bounds); - if let Some(ref ty) = default { - self.visit_ty(&ty); + let scope = Scope::TraitRefBoundary { s: self.scope }; + self.with(scope, |_, this| { + for param in generics.params { + match param.kind { + GenericParamKind::Lifetime { .. } => {} + GenericParamKind::Type { ref default, .. } => { + walk_list!(this, visit_param_bound, param.bounds); + if let Some(ref ty) = default { + this.visit_ty(&ty); + } + } + GenericParamKind::Const { ref ty, .. } => { + let was_in_const_generic = this.is_in_const_generic; + this.is_in_const_generic = true; + walk_list!(this, visit_param_bound, param.bounds); + this.visit_ty(&ty); + this.is_in_const_generic = was_in_const_generic; } } - GenericParamKind::Const { ref ty, .. } => { - let was_in_const_generic = self.is_in_const_generic; - self.is_in_const_generic = true; - walk_list!(self, visit_param_bound, param.bounds); - self.visit_ty(&ty); - self.is_in_const_generic = was_in_const_generic; - } - } - } - for predicate in generics.where_clause.predicates { - match predicate { - &hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate { - ref bounded_ty, - bounds, - ref bound_generic_params, - .. - }) => { - let (lifetimes, binders): (FxHashMap, Vec<_>) = - bound_generic_params - .iter() - .filter_map(|param| match param.kind { - GenericParamKind::Lifetime { .. } => Some(param), - _ => None, - }) - .enumerate() - .map(|(late_bound_idx, param)| { - let pair = - Region::late(late_bound_idx as u32, &self.tcx.hir(), param); - let r = late_region_as_bound_region(self.tcx, &pair.1); - (pair, r) - }) - .unzip(); - self.map.late_bound_vars.insert(bounded_ty.hir_id, binders.clone()); - let scope = Scope::TraitRefBoundary { s: self.scope }; - self.with(scope, |_, this| { - if !lifetimes.is_empty() { - let next_early_index = this.next_early_index(); - let scope = Scope::Binder { - hir_id: bounded_ty.hir_id, - lifetimes, - s: this.scope, - next_early_index, - track_lifetime_uses: true, - opaque_type_parent: false, - from_poly_trait_ref: true, - binder_depth: this.depth(false), - }; - this.with(scope, |old_scope, this| { - this.check_lifetime_params(old_scope, &bound_generic_params); + } + for predicate in generics.where_clause.predicates { + match predicate { + &hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate { + ref bounded_ty, + bounds, + ref bound_generic_params, + .. + }) => { + let (lifetimes, binders): (FxHashMap, Vec<_>) = + bound_generic_params + .iter() + .filter_map(|param| match param.kind { + GenericParamKind::Lifetime { .. } => Some(param), + _ => None, + }) + .enumerate() + .map(|(late_bound_idx, param)| { + let pair = + Region::late(late_bound_idx as u32, &this.tcx.hir(), param); + let r = late_region_as_bound_region(this.tcx, &pair.1); + (pair, r) + }) + .unzip(); + this.map.late_bound_vars.insert(bounded_ty.hir_id, binders.clone()); + if !lifetimes.is_empty() { + let next_early_index = this.next_early_index(); + let scope = Scope::Binder { + hir_id: bounded_ty.hir_id, + lifetimes, + s: this.scope, + next_early_index, + track_lifetime_uses: true, + opaque_type_parent: false, + from_poly_trait_ref: true, + binder_depth: this.depth(false), + }; + this.with(scope, |old_scope, this| { + this.check_lifetime_params(old_scope, &bound_generic_params); + this.visit_ty(&bounded_ty); + this.trait_ref_hack = Some(bounded_ty.hir_id); + walk_list!(this, visit_param_bound, bounds); + this.trait_ref_hack = None; + }) + } else { this.visit_ty(&bounded_ty); - this.trait_ref_hack = Some(bounded_ty.hir_id); walk_list!(this, visit_param_bound, bounds); - this.trait_ref_hack = None; - }) - } else { - this.visit_ty(&bounded_ty); - walk_list!(this, visit_param_bound, bounds); - } - }) - } - &hir::WherePredicate::RegionPredicate(hir::WhereRegionPredicate { - ref lifetime, - bounds, - .. - }) => { - self.visit_lifetime(lifetime); - walk_list!(self, visit_param_bound, bounds); - } - &hir::WherePredicate::EqPredicate(hir::WhereEqPredicate { - ref lhs_ty, - ref rhs_ty, - .. - }) => { - self.visit_ty(lhs_ty); - self.visit_ty(rhs_ty); + } + } + &hir::WherePredicate::RegionPredicate(hir::WhereRegionPredicate { + ref lifetime, + bounds, + .. + }) => { + this.visit_lifetime(lifetime); + walk_list!(this, visit_param_bound, bounds); + } + &hir::WherePredicate::EqPredicate(hir::WhereEqPredicate { + ref lhs_ty, + ref rhs_ty, + .. + }) => { + this.visit_ty(lhs_ty); + this.visit_ty(rhs_ty); + } } } - } + }) } fn visit_param_bound(&mut self, bound: &'tcx hir::GenericBound<'tcx>) { From 457c4c133a4bc83da668b5bceac75cd229ff112e Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Tue, 13 Apr 2021 16:58:00 -0400 Subject: [PATCH 3/7] Add BinderScopeType to replace binder_depth and from_poly_trait_ref --- compiler/rustc_resolve/src/late/lifetimes.rs | 197 ++++++++----------- 1 file changed, 86 insertions(+), 111 deletions(-) diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index 4e3e4f2c2bcb5..58d053aca1ab1 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -244,12 +244,7 @@ enum Scope<'a> { /// of the resulting opaque type. opaque_type_parent: bool, - /// True only if this `Binder` scope is from the quantifiers on a - /// `PolyTraitRef`. This is necessary for `associated_type_bounds`, which - /// requires binders of nested trait refs to be merged. - from_poly_trait_ref: bool, - - binder_depth: u32, + scope_type: BinderScopeType, /// The late bound vars for a given item are stored by `HirId` to be /// queried later. However, if we enter an elision scope, we have to @@ -335,6 +330,13 @@ enum Scope<'a> { Root, } +#[derive(Copy, Clone, Debug)] +enum BinderScopeType { + Other, + PolyTraitRef, + Concatenating, +} + // A helper struct for debugging scopes without printing parent scopes struct TruncatedScopeDebug<'a>(&'a Scope<'a>); @@ -346,8 +348,7 @@ impl<'a> fmt::Debug for TruncatedScopeDebug<'a> { next_early_index, track_lifetime_uses, opaque_type_parent, - from_poly_trait_ref, - binder_depth, + scope_type, hir_id, s: _, } => f @@ -356,8 +357,7 @@ impl<'a> fmt::Debug for TruncatedScopeDebug<'a> { .field("next_early_index", next_early_index) .field("track_lifetime_uses", track_lifetime_uses) .field("opaque_type_parent", opaque_type_parent) - .field("from_poly_trait_ref", from_poly_trait_ref) - .field("binder_depth", binder_depth) + .field("scope_type", scope_type) .field("hir_id", hir_id) .field("s", &"..") .finish(), @@ -622,48 +622,6 @@ fn late_region_as_bound_region<'tcx>(tcx: TyCtxt<'tcx>, region: &Region) -> ty:: } } -impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { - fn depth(&self, concanetate: bool) -> u32 { - let mut passed_boundary = false; - let mut scope = self.scope; - loop { - match *scope { - Scope::Root => { - break 0; - } - - Scope::TraitRefBoundary { s, .. } => { - passed_boundary = true; - scope = s; - } - - Scope::Binder { binder_depth, from_poly_trait_ref, .. } => { - if concanetate && !passed_boundary && !from_poly_trait_ref { - bug!("{:?}", self.scope); - } - break if concanetate { - if passed_boundary || !from_poly_trait_ref { - binder_depth + 1 - } else { - binder_depth - } - } else { - binder_depth + 1 - }; - } - - Scope::Elision { s, .. } - | Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } - | Scope::Supertrait { s, .. } - | Scope::Body { s, .. } => { - scope = s; - } - } - } - } -} - impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { type Map = Map<'tcx>; @@ -721,8 +679,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: self.scope, track_lifetime_uses: true, opaque_type_parent: false, - from_poly_trait_ref: false, - binder_depth: self.depth(false), + scope_type: BinderScopeType::Other, }; self.with(scope, move |_old_scope, this| { intravisit::walk_fn(this, fk, fd, b, s, hir_id) @@ -847,8 +804,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index: index + non_lifetime_count, opaque_type_parent: true, track_lifetime_uses, - from_poly_trait_ref: false, - binder_depth: self.depth(false), + scope_type: BinderScopeType::Other, s: ROOT_SCOPE, }; self.with(scope, |old_scope, this| { @@ -920,8 +876,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index, track_lifetime_uses: true, opaque_type_parent: false, - from_poly_trait_ref: false, - binder_depth: self.depth(false), + scope_type: BinderScopeType::Other, }; self.with(scope, |old_scope, this| { // a bare fn has no bounds, so everything @@ -1117,8 +1072,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: this.scope, track_lifetime_uses: true, opaque_type_parent: false, - from_poly_trait_ref: false, - binder_depth: this.depth(false), + scope_type: BinderScopeType::Other, }; this.with(scope, |_old_scope, this| { this.visit_generics(generics); @@ -1138,8 +1092,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: self.scope, track_lifetime_uses: true, opaque_type_parent: false, - from_poly_trait_ref: false, - binder_depth: self.depth(false), + scope_type: BinderScopeType::Other, }; self.with(scope, |_old_scope, this| { let scope = Scope::TraitRefBoundary { s: this.scope }; @@ -1198,8 +1151,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: self.scope, track_lifetime_uses: true, opaque_type_parent: true, - from_poly_trait_ref: false, - binder_depth: self.depth(false), + scope_type: BinderScopeType::Other, }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -1268,8 +1220,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: self.scope, track_lifetime_uses: true, opaque_type_parent: true, - from_poly_trait_ref: false, - binder_depth: self.depth(false), + scope_type: BinderScopeType::Other, }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -1373,29 +1324,28 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { }) .unzip(); this.map.late_bound_vars.insert(bounded_ty.hir_id, binders.clone()); - if !lifetimes.is_empty() { - let next_early_index = this.next_early_index(); - let scope = Scope::Binder { - hir_id: bounded_ty.hir_id, - lifetimes, - s: this.scope, - next_early_index, - track_lifetime_uses: true, - opaque_type_parent: false, - from_poly_trait_ref: true, - binder_depth: this.depth(false), - }; - this.with(scope, |old_scope, this| { - this.check_lifetime_params(old_scope, &bound_generic_params); - this.visit_ty(&bounded_ty); - this.trait_ref_hack = Some(bounded_ty.hir_id); - walk_list!(this, visit_param_bound, bounds); - this.trait_ref_hack = None; - }) - } else { + if !lifetimes.is_empty() { + let next_early_index = this.next_early_index(); + let scope = Scope::Binder { + hir_id: bounded_ty.hir_id, + lifetimes, + s: this.scope, + next_early_index, + track_lifetime_uses: true, + opaque_type_parent: false, + scope_type: BinderScopeType::PolyTraitRef, + }; + this.with(scope, |old_scope, this| { + this.check_lifetime_params(old_scope, &bound_generic_params); this.visit_ty(&bounded_ty); + this.trait_ref_hack = Some(bounded_ty.hir_id); walk_list!(this, visit_param_bound, bounds); - } + this.trait_ref_hack = None; + }) + } else { + this.visit_ty(&bounded_ty); + walk_list!(this, visit_param_bound, bounds); + } } &hir::WherePredicate::RegionPredicate(hir::WhereRegionPredicate { ref lifetime, @@ -1429,8 +1379,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index: self.next_early_index(), track_lifetime_uses: true, opaque_type_parent: false, - from_poly_trait_ref: false, - binder_depth: self.depth(false), + scope_type: BinderScopeType::Other, }; self.with(scope, |_, this| { intravisit::walk_param_bound(this, bound); @@ -1527,12 +1476,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { break vec![]; } - Scope::Binder { hir_id, from_poly_trait_ref, .. } => { - if !from_poly_trait_ref { - // We should only see super trait lifetimes if there is a `Binder` above - assert!(supertrait_lifetimes.is_empty()); - break vec![]; - } + Scope::Binder { hir_id, .. } => { // Nested poly trait refs have the binders concatenated let mut full_binders = self.map.late_bound_vars.entry(*hir_id).or_default().clone(); @@ -1569,6 +1513,33 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { self.map.late_bound_vars.insert(trait_ref.trait_ref.hir_ref_id, binders); if trait_ref_hack.is_none() || has_lifetimes { + let scope_type = { + let mut scope = self.scope; + loop { + match *scope { + Scope::Root | Scope::TraitRefBoundary { .. } => { + break BinderScopeType::PolyTraitRef; + } + + Scope::Binder { scope_type, .. } => { + if let BinderScopeType::Other = scope_type { + bug!( + "Expected all syntacic poly trait refs to be surrounded by a `TraitRefBoundary`" + ) + } + break BinderScopeType::Concatenating; + } + + Scope::Elision { s, .. } + | Scope::ObjectLifetimeDefault { s, .. } + | Scope::TraitRefHackInner { s, .. } + | Scope::Supertrait { s, .. } + | Scope::Body { s, .. } => { + scope = s; + } + } + } + }; let scope = Scope::Binder { hir_id: trait_ref.trait_ref.hir_ref_id, lifetimes, @@ -1576,8 +1547,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index, track_lifetime_uses: true, opaque_type_parent: false, - from_poly_trait_ref: true, - binder_depth: self.depth(true), + scope_type, }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &trait_ref.bound_generic_params); @@ -2327,8 +2297,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { s: self.scope, opaque_type_parent: true, track_lifetime_uses: false, - from_poly_trait_ref: false, - binder_depth: self.depth(false), + scope_type: BinderScopeType::Other, }; self.with(scope, move |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -2386,7 +2355,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { // given name or we run out of scopes. // search. let mut late_depth = 0; - let mut first_binder_depth = None; let mut scope = self.scope; let mut outermost_body = None; let result = loop { @@ -2404,7 +2372,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { break None; } - Scope::Binder { ref lifetimes, s, binder_depth, .. } => { + Scope::Binder { ref lifetimes, scope_type, s, .. } => { match lifetime_ref.name { LifetimeName::Param(param_name) => { if let Some(&def) = lifetimes.get(¶m_name.normalize_to_macros_2_0()) @@ -2414,8 +2382,11 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } _ => bug!("expected LifetimeName::Param"), } - first_binder_depth = first_binder_depth.or(Some(binder_depth)); - late_depth = first_binder_depth.unwrap_or(binder_depth) - binder_depth + 1; + match scope_type { + BinderScopeType::Other => late_depth += 1, + BinderScopeType::PolyTraitRef => late_depth += 1, + BinderScopeType::Concatenating => {} + } scope = s; } @@ -3126,7 +3097,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { let span = lifetime_refs[0].span; let mut late_depth = 0; - let mut first_binder_depth = None; let mut scope = self.scope; let mut lifetime_names = FxHashSet::default(); let mut lifetime_spans = vec![]; @@ -3137,7 +3107,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::Root => break None, - Scope::Binder { s, ref lifetimes, binder_depth, .. } => { + Scope::Binder { s, ref lifetimes, scope_type, .. } => { // collect named lifetimes for suggestions for name in lifetimes.keys() { if let hir::ParamName::Plain(name) = name { @@ -3145,8 +3115,11 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { lifetime_spans.push(name.span); } } - first_binder_depth = first_binder_depth.or(Some(binder_depth)); - late_depth = first_binder_depth.unwrap_or(binder_depth) - binder_depth + 1; + match scope_type { + BinderScopeType::Other => late_depth += 1, + BinderScopeType::PolyTraitRef => late_depth += 1, + BinderScopeType::Concatenating => {} + } scope = s; } @@ -3303,13 +3276,15 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { fn resolve_object_lifetime_default(&mut self, lifetime_ref: &'tcx hir::Lifetime) { debug!("resolve_object_lifetime_default(lifetime_ref={:?})", lifetime_ref); let mut late_depth = 0; - let mut first_binder_depth = None; let mut scope = self.scope; let lifetime = loop { match *scope { - Scope::Binder { s, binder_depth, .. } => { - first_binder_depth = first_binder_depth.or(Some(binder_depth)); - late_depth = first_binder_depth.unwrap_or(binder_depth) - binder_depth + 1; + Scope::Binder { s, scope_type, .. } => { + match scope_type { + BinderScopeType::Other => late_depth += 1, + BinderScopeType::PolyTraitRef => late_depth += 1, + BinderScopeType::Concatenating => {} + } scope = s; } From 9891582897ae5f588022cec6a2f798d7a2883629 Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Tue, 20 Apr 2021 16:39:41 -0400 Subject: [PATCH 4/7] Remove TraitRefHackInner and use the concatenating functionality instead of trait_ref_hack --- compiler/rustc_resolve/src/late/lifetimes.rs | 421 ++++++++----------- 1 file changed, 176 insertions(+), 245 deletions(-) diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index 58d053aca1ab1..62cbbd8e8f705 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -165,29 +165,6 @@ crate struct LifetimeContext<'a, 'tcx> { map: &'a mut NamedRegionMap, scope: ScopeRef<'a>, - /// This is slightly complicated. Our representation for poly-trait-refs contains a single - /// binder and thus we only allow a single level of quantification. However, - /// the syntax of Rust permits quantification in two places, e.g., `T: for <'a> Foo<'a>` - /// and `for <'a, 'b> &'b T: Foo<'a>`. In order to get the De Bruijn indices - /// correct when representing these constraints, we should only introduce one - /// scope. However, we want to support both locations for the quantifier and - /// during lifetime resolution we want precise information (so we can't - /// desugar in an earlier phase). Moreso, an error here doesn't cause a bail - /// from type checking, so we need to be extra careful that we don't lose - /// any bound var information. - /// - /// So, if we encounter a quantifier at the outer scope, we set - /// `trait_ref_hack` to the hir id of the bounded type (and introduce a scope). - /// Then, if we encounter a quantifier at the inner scope, then we know to - /// emit an error. Importantly though, we do have to track the lifetimes - /// defined on the outer scope (i.e. the bounded ty), since we continue - /// to type check after emitting an error; we therefore assume that the bound - /// vars on the inner trait refs come from both quantifiers. - /// - /// If we encounter a quantifier in the inner scope `trait_ref_hack` being - /// `None`, then we just introduce the scope at the inner quantifier as normal. - trait_ref_hack: Option, - /// Used to disallow the use of in-band lifetimes in `fn` or `Fn` syntax. is_in_fn_syntax: bool, @@ -279,41 +256,6 @@ enum Scope<'a> { s: ScopeRef<'a>, }, - /// This is a particularly interesting consequence of how we handle poly - /// trait refs. See `trait_ref_hack` for additional info. This bit is - /// important w.r.t. querying late-bound vars. - /// - /// To completely understand why this is necessary, first it's important to - /// realize that `T: for<'a> U + for<'a, 'b> V` is actually two separate - /// trait refs: `T: for<'a> U` and `T: for<'b> V` and as such, the late - /// bound vars on each needs to be tracked separately. Also, in this case, - /// are *three* relevant `HirId`s: one for the entire bound and one - /// for each separate one. - /// - /// Next, imagine three different poly trait refs: - /// 1) `for<'a, 'b> T: U<'a, 'b>` - /// 2) `T: for<'a, 'b> U<'a, 'b>` - /// 3) `for<'a> T: for<'b> U<'a, 'b>` - /// - /// First, note that the third example is semantically invalid and an error, - /// but we *must* handle it as valid, since type checking isn't bailed out - /// of. Other than that, if ask for bound vars for each, we expect - /// `['a, 'b]`. If we *didn't* allow binders before `T`, then we would - /// always introduce a binder scope at the inner trait ref. This is great, - /// because later on during type-checking, we will ask "what are the late - /// bound vars on this trait ref". However, because we allow bound vars on - /// the bound itself, we have to have some way of keeping track of the fact - /// that we actually want to store the late bound vars as being associated - /// with the trait ref; this is that. - /// - /// One alternative way to handle this would be to just introduce a new - /// `Binder` scope, but that's semantically a bit different, since bound - /// vars from both `for<...>`s *do* share the same binder level. - TraitRefHackInner { - hir_id: hir::HirId, - s: ScopeRef<'a>, - }, - /// When we have nested trait refs, we concanetate late bound vars for inner /// trait refs from outer ones. But we also need to include any HRTB /// lifetimes encountered when identifying the trait that an associated type @@ -332,9 +274,48 @@ enum Scope<'a> { #[derive(Copy, Clone, Debug)] enum BinderScopeType { - Other, + /// In a syntactic trait ref, this represents the outermost binder. So, if + /// you had `T: for<'a> Foo Baz<'a, 'b>>`, then the `for<'a>` + /// scope uses `PolyTraitRef`. PolyTraitRef, + /// This is slightly complicated. Our representation for poly-trait-refs contains a single + /// binder and thus we only allow a single level of quantification. However, + /// the syntax of Rust permits quantification in two places in where clauses, + /// e.g., `T: for <'a> Foo<'a>` and `for <'a, 'b> &'b T: Foo<'a>`. In order + /// to get the De Bruijn indices correct when representing these constraints, + /// we should only introduce one scope. However, we want to support both + /// locations for the quantifier and during lifetime resolution we want + /// precise information (so we can't desugar in an earlier phase). Moreso, + /// an error here doesn't cause a bail from type checking, so we need to be + /// extra careful that we don't lose any bound var information for *either* + /// syntactic binder and that we track all lifetimes defined in both binders. + /// + /// This mechanism is similar to the concatenation done in nested poly trait + /// refs, i.e. the inner syntactic binder extends upon the lifetimes on the + /// outer syntactic binder. However, we require a separate variant here to + /// distinguish `for<'a> T: for<'b> Foo<'a, 'b>` from + /// `T: for<'a> Bar Foo<'a, 'b>>`. In this case, the innermost + /// `: for<'b> Foo<'a, 'b>` both have a `for<'a>` scope above it. However, + /// in the former case, we must emit an error because this is invalid syntax. + /// Put another way: `PolyTraitRef` and `BoundedTy` behave identically except + /// that `BoundedTy` is used to signal that an error should be emitted if + /// another syntactic binder is found. + BoundedTy, + /// Within a syntactic trait ref, there may be multiple poly trait refs that + /// are nested (under the `associcated_type_bounds` feature). The binders of + /// the innner poly trait refs are extended from the outer poly trait refs + /// and don't increase the late bound depth. If you had + /// `T: for<'a> Foo Baz<'a, 'b>>`, then the `for<'b>` scope + /// would be `Concatenating`. This also used in trait refs in where clauses + /// where we have two binders `for<> T: for<> Foo` (I've intentionally left + /// out any lifetimes because they aren't needed to show the two scopes). + /// See `BoundedTy` for a bit more details, but the inner `for<>` has a scope + /// of `Concatenating`. Concatenating, + /// Any other binder scopes. These are "normal" in that they increase the binder + /// depth, are fully syntactic, don't concatenate, and don't have special syntactical + /// considerations. + Other, } // A helper struct for debugging scopes without printing parent scopes @@ -372,11 +353,6 @@ impl<'a> fmt::Debug for TruncatedScopeDebug<'a> { .field("lifetime", lifetime) .field("s", &"..") .finish(), - Scope::TraitRefHackInner { hir_id, s: _ } => f - .debug_struct("TraitRefHackInner") - .field("hir_id", hir_id) - .field("s", &"..") - .finish(), Scope::Supertrait { lifetimes, s: _ } => f .debug_struct("Supertrait") .field("lifetimes", lifetimes) @@ -499,7 +475,6 @@ fn do_resolve( tcx, map: &mut named_region_map, scope: ROOT_SCOPE, - trait_ref_hack: None, is_in_fn_syntax: false, is_in_const_generic: false, trait_definition_only, @@ -1324,28 +1299,25 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { }) .unzip(); this.map.late_bound_vars.insert(bounded_ty.hir_id, binders.clone()); - if !lifetimes.is_empty() { - let next_early_index = this.next_early_index(); - let scope = Scope::Binder { - hir_id: bounded_ty.hir_id, - lifetimes, - s: this.scope, - next_early_index, - track_lifetime_uses: true, - opaque_type_parent: false, - scope_type: BinderScopeType::PolyTraitRef, - }; - this.with(scope, |old_scope, this| { - this.check_lifetime_params(old_scope, &bound_generic_params); - this.visit_ty(&bounded_ty); - this.trait_ref_hack = Some(bounded_ty.hir_id); - walk_list!(this, visit_param_bound, bounds); - this.trait_ref_hack = None; - }) - } else { + let next_early_index = this.next_early_index(); + // Even if there are no lifetimes defined here, we still wrap it in a binder + // scope. If there happens to be a nested poly trait ref (an error), that + // will be `Concatenating` anyways, so we don't have to worry about the depth + // being wrong. + let scope = Scope::Binder { + hir_id: bounded_ty.hir_id, + lifetimes, + s: this.scope, + next_early_index, + track_lifetime_uses: true, + opaque_type_parent: false, + scope_type: BinderScopeType::BoundedTy, + }; + this.with(scope, |old_scope, this| { + this.check_lifetime_params(old_scope, &bound_generic_params); this.visit_ty(&bounded_ty); walk_list!(this, visit_param_bound, bounds); - } + }) } &hir::WherePredicate::RegionPredicate(hir::WhereRegionPredicate { ref lifetime, @@ -1369,8 +1341,37 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { } fn visit_param_bound(&mut self, bound: &'tcx hir::GenericBound<'tcx>) { + // FIXME(jackh726): This is pretty weird. `LangItemTrait` doesn't go + // through the regular poly trait ref code, so we don't get another + // chance to introduce a binder. For now, I'm keeping the existing logic + // of "if there isn't a `BoundedTy` scope above us, add one", but I + // imagine there's a better way to go about this. + let mut scope = self.scope; + let trait_ref_hack = loop { + match scope { + Scope::Body { .. } | Scope::Root => { + break false; + } + + Scope::Elision { s, .. } + | Scope::ObjectLifetimeDefault { s, .. } + | Scope::Supertrait { s, .. } => { + scope = s; + } + + Scope::TraitRefBoundary { .. } => { + break false; + } + + Scope::Binder { scope_type, lifetimes, .. } => { + let trait_ref_hack = + matches!(scope_type, BinderScopeType::BoundedTy) && !lifetimes.is_empty(); + break trait_ref_hack; + } + } + }; match bound { - hir::GenericBound::LangItemTrait(_, _, hir_id, _) if self.trait_ref_hack.is_none() => { + hir::GenericBound::LangItemTrait(_, _, hir_id, _) if !trait_ref_hack => { self.map.late_bound_vars.insert(*hir_id, vec![]); let scope = Scope::Binder { hir_id: *hir_id, @@ -1398,15 +1399,56 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { let should_pop_missing_lt = self.is_trait_ref_fn_scope(trait_ref); - let trait_ref_hack = self.trait_ref_hack.take(); let next_early_index = self.next_early_index(); - // See note on `trait_ref_hack`. If `for<..>` has been defined in both - // the outer and inner part of the trait ref, emit an error. + let mut scope = self.scope; + let mut supertrait_lifetimes = vec![]; + let (mut binders, trait_ref_hack, scope_type) = loop { + match scope { + Scope::Body { .. } | Scope::Root => { + break (vec![], false, BinderScopeType::PolyTraitRef); + } + + Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } => { + scope = s; + } + + Scope::Supertrait { s, lifetimes } => { + supertrait_lifetimes = lifetimes.clone(); + scope = s; + } + + Scope::TraitRefBoundary { .. } => { + // We should only see super trait lifetimes if there is a `Binder` above + assert!(supertrait_lifetimes.is_empty()); + break (vec![], false, BinderScopeType::PolyTraitRef); + } + + Scope::Binder { hir_id, scope_type, lifetimes, .. } => { + if let BinderScopeType::Other = scope_type { + bug!( + "Expected all syntacic poly trait refs to be surrounded by a `TraitRefBoundary`" + ) + } + + // Nested poly trait refs have the binders concatenated + let mut full_binders = + self.map.late_bound_vars.entry(*hir_id).or_default().clone(); + full_binders.extend(supertrait_lifetimes.into_iter()); + let trait_ref_hack = + matches!(scope_type, BinderScopeType::BoundedTy) && !lifetimes.is_empty(); + break (full_binders, trait_ref_hack, BinderScopeType::Concatenating); + } + } + }; + + // See note on `BinderScopeType::BoundedTy`. If `for<..>` + // has been defined in both the outer and inner part of the + // trait ref, emit an error. let has_lifetimes = trait_ref.bound_generic_params.iter().any(|param| match param.kind { GenericParamKind::Lifetime { .. } => true, _ => false, }); - if trait_ref_hack.is_some() && has_lifetimes { + if trait_ref_hack && has_lifetimes { struct_span_err!( self.tcx.sess, trait_ref.span, @@ -1416,152 +1458,50 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { .emit(); } - let (binders, lifetimes) = if let Some(hir_id) = trait_ref_hack { - let mut binders = self.map.late_bound_vars.entry(hir_id).or_default().clone(); - let initial_bound_vars = binders.len() as u32; - let mut lifetimes: FxHashMap = FxHashMap::default(); - let binders_iter = trait_ref - .bound_generic_params - .iter() - .filter_map(|param| match param.kind { - GenericParamKind::Lifetime { .. } => Some(param), - _ => None, - }) - .enumerate() - .map(|(late_bound_idx, param)| { - let pair = Region::late( - initial_bound_vars + late_bound_idx as u32, - &self.tcx.hir(), - param, - ); - let r = late_region_as_bound_region(self.tcx, &pair.1); - lifetimes.insert(pair.0, pair.1); - r - }); - binders.extend(binders_iter); - - (binders, lifetimes) - } else { - let mut supertrait_lifetimes = vec![]; - let mut scope = self.scope; - let mut outer_binders = loop { - match scope { - Scope::Body { .. } | Scope::Root => { - break vec![]; - } - - Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } => { - scope = s; - } - - Scope::TraitRefHackInner { hir_id, .. } => { - // Nested poly trait refs have the binders concatenated - // If we reach `TraitRefHackInner`, then there is only one more `Binder` above us, - // over all the bounds. We don't want this, since all the lifetimes we care about - // are here anyways. - let mut full_binders = - self.map.late_bound_vars.entry(*hir_id).or_default().clone(); - full_binders.extend(supertrait_lifetimes.into_iter()); - break full_binders; - } - - Scope::Supertrait { s, lifetimes } => { - supertrait_lifetimes = lifetimes.clone(); - scope = s; - } - - Scope::TraitRefBoundary { .. } => { - // We should only see super trait lifetimes if there is a `Binder` above - assert!(supertrait_lifetimes.is_empty()); - break vec![]; - } - - Scope::Binder { hir_id, .. } => { - // Nested poly trait refs have the binders concatenated - let mut full_binders = - self.map.late_bound_vars.entry(*hir_id).or_default().clone(); - full_binders.extend(supertrait_lifetimes.into_iter()); - break full_binders; - } - } - }; - let (lifetimes, local_binders): (FxHashMap, Vec<_>) = trait_ref - .bound_generic_params - .iter() - .filter_map(|param| match param.kind { - GenericParamKind::Lifetime { .. } => Some(param), - _ => None, - }) - .enumerate() - .map(|(late_bound_idx, param)| { - let pair = Region::late( - outer_binders.len() as u32 + late_bound_idx as u32, - &self.tcx.hir(), - param, - ); - let r = late_region_as_bound_region(self.tcx, &pair.1); - (pair, r) - }) - .unzip(); - - outer_binders.extend(local_binders.into_iter()); - - (outer_binders, lifetimes) - }; + let initial_bound_vars = binders.len() as u32; + let mut lifetimes: FxHashMap = FxHashMap::default(); + let binders_iter = trait_ref + .bound_generic_params + .iter() + .filter_map(|param| match param.kind { + GenericParamKind::Lifetime { .. } => Some(param), + _ => None, + }) + .enumerate() + .map(|(late_bound_idx, param)| { + let pair = Region::late( + initial_bound_vars + late_bound_idx as u32, + &self.tcx.hir(), + param, + ); + let r = late_region_as_bound_region(self.tcx, &pair.1); + lifetimes.insert(pair.0, pair.1); + r + }); + binders.extend(binders_iter); debug!(?binders); self.map.late_bound_vars.insert(trait_ref.trait_ref.hir_ref_id, binders); - if trait_ref_hack.is_none() || has_lifetimes { - let scope_type = { - let mut scope = self.scope; - loop { - match *scope { - Scope::Root | Scope::TraitRefBoundary { .. } => { - break BinderScopeType::PolyTraitRef; - } - - Scope::Binder { scope_type, .. } => { - if let BinderScopeType::Other = scope_type { - bug!( - "Expected all syntacic poly trait refs to be surrounded by a `TraitRefBoundary`" - ) - } - break BinderScopeType::Concatenating; - } + // Always introduce a scope here, even if this is in a where clause and + // we introduced the binders around the bounded Ty. In that case, we + // just reuse the concatenation functionality also present in nested trait + // refs. See `BinderScopeType::BoundedTy` for more details on that case. + let scope = Scope::Binder { + hir_id: trait_ref.trait_ref.hir_ref_id, + lifetimes, + s: self.scope, + next_early_index, + track_lifetime_uses: true, + opaque_type_parent: false, + scope_type, + }; + self.with(scope, |old_scope, this| { + this.check_lifetime_params(old_scope, &trait_ref.bound_generic_params); + walk_list!(this, visit_generic_param, trait_ref.bound_generic_params); + this.visit_trait_ref(&trait_ref.trait_ref); + }); - Scope::Elision { s, .. } - | Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } - | Scope::Supertrait { s, .. } - | Scope::Body { s, .. } => { - scope = s; - } - } - } - }; - let scope = Scope::Binder { - hir_id: trait_ref.trait_ref.hir_ref_id, - lifetimes, - s: self.scope, - next_early_index, - track_lifetime_uses: true, - opaque_type_parent: false, - scope_type, - }; - self.with(scope, |old_scope, this| { - this.check_lifetime_params(old_scope, &trait_ref.bound_generic_params); - walk_list!(this, visit_generic_param, trait_ref.bound_generic_params); - this.visit_trait_ref(&trait_ref.trait_ref); - }); - } else { - let scope = - Scope::TraitRefHackInner { hir_id: trait_ref.trait_ref.hir_ref_id, s: self.scope }; - self.with(scope, |_old_scope, this| { - this.visit_trait_ref(&trait_ref.trait_ref); - }); - } - self.trait_ref_hack = trait_ref_hack; if should_pop_missing_lt { self.missing_named_lifetime_spots.pop(); } @@ -1712,7 +1652,6 @@ fn extract_labels(ctxt: &mut LifetimeContext<'_, '_>, body: &hir::Body<'_>) { Scope::Body { s, .. } | Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } | Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => { scope = s; @@ -1903,12 +1842,10 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { let labels_in_fn = take(&mut self.labels_in_fn); let xcrate_object_lifetime_defaults = take(&mut self.xcrate_object_lifetime_defaults); let missing_named_lifetime_spots = take(&mut self.missing_named_lifetime_spots); - let trait_ref_hack = take(&mut self.trait_ref_hack); let mut this = LifetimeContext { tcx: *tcx, map, scope: &wrap_scope, - trait_ref_hack, is_in_fn_syntax: self.is_in_fn_syntax, is_in_const_generic: self.is_in_const_generic, trait_definition_only: self.trait_definition_only, @@ -1928,7 +1865,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { self.labels_in_fn = this.labels_in_fn; self.xcrate_object_lifetime_defaults = this.xcrate_object_lifetime_defaults; self.missing_named_lifetime_spots = this.missing_named_lifetime_spots; - self.trait_ref_hack = this.trait_ref_hack; } /// helper method to determine the span to remove when suggesting the @@ -2321,7 +2257,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { | Scope::Body { s, .. } | Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } | Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => scope = s, } @@ -2384,6 +2319,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } match scope_type { BinderScopeType::Other => late_depth += 1, + BinderScopeType::BoundedTy => late_depth += 1, BinderScopeType::PolyTraitRef => late_depth += 1, BinderScopeType::Concatenating => {} } @@ -2392,7 +2328,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } | Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => { scope = s; @@ -2547,7 +2482,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::Binder { s, .. } | Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } | Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => { scope = s; @@ -2746,7 +2680,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { let mut scope = &*self.scope; let hir_id = loop { match scope { - Scope::Binder { hir_id, .. } | Scope::TraitRefHackInner { hir_id, .. } => { + Scope::Binder { hir_id, .. } => { break *hir_id; } Scope::Body { id, .. } => break id.hir_id, @@ -3117,6 +3051,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } match scope_type { BinderScopeType::Other => late_depth += 1, + BinderScopeType::BoundedTy => late_depth += 1, BinderScopeType::PolyTraitRef => late_depth += 1, BinderScopeType::Concatenating => {} } @@ -3168,7 +3103,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } | Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => { scope = s; @@ -3282,6 +3216,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::Binder { s, scope_type, .. } => { match scope_type { BinderScopeType::Other => late_depth += 1, + BinderScopeType::BoundedTy => late_depth += 1, BinderScopeType::PolyTraitRef => late_depth += 1, BinderScopeType::Concatenating => {} } @@ -3294,9 +3229,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::ObjectLifetimeDefault { lifetime: Some(l), .. } => break l, - Scope::TraitRefHackInner { s, .. } - | Scope::Supertrait { s, .. } - | Scope::TraitRefBoundary { s, .. } => { + Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => { scope = s; } } @@ -3423,7 +3356,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::Body { s, .. } | Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } | Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => { old_scope = s; @@ -3482,7 +3414,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } => break false, Scope::ObjectLifetimeDefault { s, .. } - | Scope::TraitRefHackInner { s, .. } | Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => scope = s, } From 4568e7d62edc52dca67e28ea771978513cf6f393 Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Wed, 21 Apr 2021 03:12:04 -0400 Subject: [PATCH 5/7] Move nested quantification check to ast_validation --- .../rustc_ast_passes/src/ast_validation.rs | 47 ++++++++--- compiler/rustc_resolve/src/late/lifetimes.rs | 80 ++++--------------- 2 files changed, 52 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index bb09f701531cf..809660379f326 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -81,6 +81,13 @@ struct AstValidator<'a> { is_assoc_ty_bound_banned: bool, lint_buffer: &'a mut LintBuffer, + + /// This is slightly complicated. Our representation for poly-trait-refs contains a single + /// binder and thus we only allow a single level of quantification. However, + /// the syntax of Rust permits quantification in two places in where clauses, + /// e.g., `T: for <'a> Foo<'a>` and `for <'a, 'b> &'b T: Foo<'a>`. If both are + /// defined, then error. + trait_ref_hack: bool, } impl<'a> AstValidator<'a> { @@ -1213,8 +1220,25 @@ impl<'a> Visitor<'a> for AstValidator<'a> { deny_equality_constraints(self, predicate, generics); } } + walk_list!(self, visit_generic_param, &generics.params); + for predicate in &generics.where_clause.predicates { + match predicate { + WherePredicate::BoundPredicate(bound_pred) => { + // A type binding, eg `for<'c> Foo: Send+Clone+'c` + self.check_late_bound_lifetime_defs(&bound_pred.bound_generic_params); - visit::walk_generics(self, generics) + self.visit_ty(&bound_pred.bounded_ty); + + self.trait_ref_hack = !bound_pred.bound_generic_params.is_empty(); + walk_list!(self, visit_param_bound, &bound_pred.bounds); + walk_list!(self, visit_generic_param, &bound_pred.bound_generic_params); + self.trait_ref_hack = false; + } + _ => { + self.visit_where_predicate(predicate); + } + } + } } fn visit_generic_param(&mut self, param: &'a GenericParam) { @@ -1263,17 +1287,21 @@ impl<'a> Visitor<'a> for AstValidator<'a> { visit::walk_pat(self, pat) } - fn visit_where_predicate(&mut self, p: &'a WherePredicate) { - if let &WherePredicate::BoundPredicate(ref bound_predicate) = p { - // A type binding, eg `for<'c> Foo: Send+Clone+'c` - self.check_late_bound_lifetime_defs(&bound_predicate.bound_generic_params); - } - visit::walk_where_predicate(self, p); - } - fn visit_poly_trait_ref(&mut self, t: &'a PolyTraitRef, m: &'a TraitBoundModifier) { self.check_late_bound_lifetime_defs(&t.bound_generic_params); + if self.trait_ref_hack && !t.bound_generic_params.is_empty() { + struct_span_err!( + self.err_handler(), + t.span, + E0316, + "nested quantification of lifetimes" + ) + .emit(); + } + let trait_ref_hack = self.trait_ref_hack; + self.trait_ref_hack = false; visit::walk_poly_trait_ref(self, t, m); + self.trait_ref_hack = trait_ref_hack; } fn visit_variant_data(&mut self, s: &'a VariantData) { @@ -1492,6 +1520,7 @@ pub fn check_crate(session: &Session, krate: &Crate, lints: &mut LintBuffer) -> is_impl_trait_banned: false, is_assoc_ty_bound_banned: false, lint_buffer: lints, + trait_ref_hack: false, }; visit::walk_crate(&mut validator, krate); diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index 62cbbd8e8f705..3cccbb06bc426 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -278,29 +278,6 @@ enum BinderScopeType { /// you had `T: for<'a> Foo Baz<'a, 'b>>`, then the `for<'a>` /// scope uses `PolyTraitRef`. PolyTraitRef, - /// This is slightly complicated. Our representation for poly-trait-refs contains a single - /// binder and thus we only allow a single level of quantification. However, - /// the syntax of Rust permits quantification in two places in where clauses, - /// e.g., `T: for <'a> Foo<'a>` and `for <'a, 'b> &'b T: Foo<'a>`. In order - /// to get the De Bruijn indices correct when representing these constraints, - /// we should only introduce one scope. However, we want to support both - /// locations for the quantifier and during lifetime resolution we want - /// precise information (so we can't desugar in an earlier phase). Moreso, - /// an error here doesn't cause a bail from type checking, so we need to be - /// extra careful that we don't lose any bound var information for *either* - /// syntactic binder and that we track all lifetimes defined in both binders. - /// - /// This mechanism is similar to the concatenation done in nested poly trait - /// refs, i.e. the inner syntactic binder extends upon the lifetimes on the - /// outer syntactic binder. However, we require a separate variant here to - /// distinguish `for<'a> T: for<'b> Foo<'a, 'b>` from - /// `T: for<'a> Bar Foo<'a, 'b>>`. In this case, the innermost - /// `: for<'b> Foo<'a, 'b>` both have a `for<'a>` scope above it. However, - /// in the former case, we must emit an error because this is invalid syntax. - /// Put another way: `PolyTraitRef` and `BoundedTy` behave identically except - /// that `BoundedTy` is used to signal that an error should be emitted if - /// another syntactic binder is found. - BoundedTy, /// Within a syntactic trait ref, there may be multiple poly trait refs that /// are nested (under the `associcated_type_bounds` feature). The binders of /// the innner poly trait refs are extended from the outer poly trait refs @@ -309,8 +286,7 @@ enum BinderScopeType { /// would be `Concatenating`. This also used in trait refs in where clauses /// where we have two binders `for<> T: for<> Foo` (I've intentionally left /// out any lifetimes because they aren't needed to show the two scopes). - /// See `BoundedTy` for a bit more details, but the inner `for<>` has a scope - /// of `Concatenating`. + /// The inner `for<>` has a scope of `Concatenating`. Concatenating, /// Any other binder scopes. These are "normal" in that they increase the binder /// depth, are fully syntactic, don't concatenate, and don't have special syntactical @@ -1311,7 +1287,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index, track_lifetime_uses: true, opaque_type_parent: false, - scope_type: BinderScopeType::BoundedTy, + scope_type: BinderScopeType::PolyTraitRef, }; this.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &bound_generic_params); @@ -1344,30 +1320,24 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { // FIXME(jackh726): This is pretty weird. `LangItemTrait` doesn't go // through the regular poly trait ref code, so we don't get another // chance to introduce a binder. For now, I'm keeping the existing logic - // of "if there isn't a `BoundedTy` scope above us, add one", but I + // of "if there isn't a Binder scope above us, add one", but I // imagine there's a better way to go about this. let mut scope = self.scope; let trait_ref_hack = loop { match scope { - Scope::Body { .. } | Scope::Root => { + Scope::TraitRefBoundary { .. } | Scope::Body { .. } | Scope::Root => { break false; } + Scope::Binder { .. } => { + break true; + } + Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } | Scope::Supertrait { s, .. } => { scope = s; } - - Scope::TraitRefBoundary { .. } => { - break false; - } - - Scope::Binder { scope_type, lifetimes, .. } => { - let trait_ref_hack = - matches!(scope_type, BinderScopeType::BoundedTy) && !lifetimes.is_empty(); - break trait_ref_hack; - } } }; match bound { @@ -1402,10 +1372,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { let next_early_index = self.next_early_index(); let mut scope = self.scope; let mut supertrait_lifetimes = vec![]; - let (mut binders, trait_ref_hack, scope_type) = loop { + let (mut binders, scope_type) = loop { match scope { Scope::Body { .. } | Scope::Root => { - break (vec![], false, BinderScopeType::PolyTraitRef); + break (vec![], BinderScopeType::PolyTraitRef); } Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } => { @@ -1420,10 +1390,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { Scope::TraitRefBoundary { .. } => { // We should only see super trait lifetimes if there is a `Binder` above assert!(supertrait_lifetimes.is_empty()); - break (vec![], false, BinderScopeType::PolyTraitRef); + break (vec![], BinderScopeType::PolyTraitRef); } - Scope::Binder { hir_id, scope_type, lifetimes, .. } => { + Scope::Binder { hir_id, scope_type, .. } => { if let BinderScopeType::Other = scope_type { bug!( "Expected all syntacic poly trait refs to be surrounded by a `TraitRefBoundary`" @@ -1434,30 +1404,11 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { let mut full_binders = self.map.late_bound_vars.entry(*hir_id).or_default().clone(); full_binders.extend(supertrait_lifetimes.into_iter()); - let trait_ref_hack = - matches!(scope_type, BinderScopeType::BoundedTy) && !lifetimes.is_empty(); - break (full_binders, trait_ref_hack, BinderScopeType::Concatenating); + break (full_binders, BinderScopeType::Concatenating); } } }; - // See note on `BinderScopeType::BoundedTy`. If `for<..>` - // has been defined in both the outer and inner part of the - // trait ref, emit an error. - let has_lifetimes = trait_ref.bound_generic_params.iter().any(|param| match param.kind { - GenericParamKind::Lifetime { .. } => true, - _ => false, - }); - if trait_ref_hack && has_lifetimes { - struct_span_err!( - self.tcx.sess, - trait_ref.span, - E0316, - "nested quantification of lifetimes" - ) - .emit(); - } - let initial_bound_vars = binders.len() as u32; let mut lifetimes: FxHashMap = FxHashMap::default(); let binders_iter = trait_ref @@ -1486,7 +1437,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { // Always introduce a scope here, even if this is in a where clause and // we introduced the binders around the bounded Ty. In that case, we // just reuse the concatenation functionality also present in nested trait - // refs. See `BinderScopeType::BoundedTy` for more details on that case. + // refs. let scope = Scope::Binder { hir_id: trait_ref.trait_ref.hir_ref_id, lifetimes, @@ -2319,7 +2270,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } match scope_type { BinderScopeType::Other => late_depth += 1, - BinderScopeType::BoundedTy => late_depth += 1, BinderScopeType::PolyTraitRef => late_depth += 1, BinderScopeType::Concatenating => {} } @@ -3051,7 +3001,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } match scope_type { BinderScopeType::Other => late_depth += 1, - BinderScopeType::BoundedTy => late_depth += 1, BinderScopeType::PolyTraitRef => late_depth += 1, BinderScopeType::Concatenating => {} } @@ -3216,7 +3165,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { Scope::Binder { s, scope_type, .. } => { match scope_type { BinderScopeType::Other => late_depth += 1, - BinderScopeType::BoundedTy => late_depth += 1, BinderScopeType::PolyTraitRef => late_depth += 1, BinderScopeType::Concatenating => {} } From b78c0d8a4d5af91a4a55d029293e3ecb879ec142 Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Wed, 21 Apr 2021 11:49:59 -0400 Subject: [PATCH 6/7] Review comments --- .../rustc_ast_passes/src/ast_validation.rs | 54 +++++++++---------- compiler/rustc_resolve/src/late/lifetimes.rs | 15 +++--- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 809660379f326..8a3f76415968e 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -81,13 +81,6 @@ struct AstValidator<'a> { is_assoc_ty_bound_banned: bool, lint_buffer: &'a mut LintBuffer, - - /// This is slightly complicated. Our representation for poly-trait-refs contains a single - /// binder and thus we only allow a single level of quantification. However, - /// the syntax of Rust permits quantification in two places in where clauses, - /// e.g., `T: for <'a> Foo<'a>` and `for <'a, 'b> &'b T: Foo<'a>`. If both are - /// defined, then error. - trait_ref_hack: bool, } impl<'a> AstValidator<'a> { @@ -1227,17 +1220,33 @@ impl<'a> Visitor<'a> for AstValidator<'a> { // A type binding, eg `for<'c> Foo: Send+Clone+'c` self.check_late_bound_lifetime_defs(&bound_pred.bound_generic_params); - self.visit_ty(&bound_pred.bounded_ty); - - self.trait_ref_hack = !bound_pred.bound_generic_params.is_empty(); - walk_list!(self, visit_param_bound, &bound_pred.bounds); - walk_list!(self, visit_generic_param, &bound_pred.bound_generic_params); - self.trait_ref_hack = false; - } - _ => { - self.visit_where_predicate(predicate); + // This is slightly complicated. Our representation for poly-trait-refs contains a single + // binder and thus we only allow a single level of quantification. However, + // the syntax of Rust permits quantification in two places in where clauses, + // e.g., `T: for <'a> Foo<'a>` and `for <'a, 'b> &'b T: Foo<'a>`. If both are + // defined, then error. + if !bound_pred.bound_generic_params.is_empty() { + for bound in &bound_pred.bounds { + match bound { + GenericBound::Trait(t, _) => { + if !t.bound_generic_params.is_empty() { + struct_span_err!( + self.err_handler(), + t.span, + E0316, + "nested quantification of lifetimes" + ) + .emit(); + } + } + GenericBound::Outlives(_) => {} + } + } + } } + _ => {} } + self.visit_where_predicate(predicate); } } @@ -1289,19 +1298,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { fn visit_poly_trait_ref(&mut self, t: &'a PolyTraitRef, m: &'a TraitBoundModifier) { self.check_late_bound_lifetime_defs(&t.bound_generic_params); - if self.trait_ref_hack && !t.bound_generic_params.is_empty() { - struct_span_err!( - self.err_handler(), - t.span, - E0316, - "nested quantification of lifetimes" - ) - .emit(); - } - let trait_ref_hack = self.trait_ref_hack; - self.trait_ref_hack = false; visit::walk_poly_trait_ref(self, t, m); - self.trait_ref_hack = trait_ref_hack; } fn visit_variant_data(&mut self, s: &'a VariantData) { @@ -1520,7 +1517,6 @@ pub fn check_crate(session: &Session, krate: &Crate, lints: &mut LintBuffer) -> is_impl_trait_banned: false, is_assoc_ty_bound_banned: false, lint_buffer: lints, - trait_ref_hack: false, }; visit::walk_crate(&mut validator, krate); diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index 3cccbb06bc426..dfb6167c20cac 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -1323,14 +1323,15 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { // of "if there isn't a Binder scope above us, add one", but I // imagine there's a better way to go about this. let mut scope = self.scope; - let trait_ref_hack = loop { + let (binders, scope_type) = loop { match scope { Scope::TraitRefBoundary { .. } | Scope::Body { .. } | Scope::Root => { - break false; + break (vec![], BinderScopeType::PolyTraitRef); } - Scope::Binder { .. } => { - break true; + Scope::Binder { hir_id, .. } => { + let binders = self.map.late_bound_vars.entry(*hir_id).or_default().clone(); + break (binders, BinderScopeType::Concatenating); } Scope::Elision { s, .. } @@ -1341,8 +1342,8 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { } }; match bound { - hir::GenericBound::LangItemTrait(_, _, hir_id, _) if !trait_ref_hack => { - self.map.late_bound_vars.insert(*hir_id, vec![]); + hir::GenericBound::LangItemTrait(_, _, hir_id, _) => { + self.map.late_bound_vars.insert(*hir_id, binders); let scope = Scope::Binder { hir_id: *hir_id, lifetimes: FxHashMap::default(), @@ -1350,7 +1351,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index: self.next_early_index(), track_lifetime_uses: true, opaque_type_parent: false, - scope_type: BinderScopeType::Other, + scope_type, }; self.with(scope, |_, this| { intravisit::walk_param_bound(this, bound); From c78724f869add98e0f2af8950f760a37ec35243b Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Wed, 21 Apr 2021 12:26:19 -0400 Subject: [PATCH 7/7] More review changes --- compiler/rustc_resolve/src/late/lifetimes.rs | 144 ++++++++----------- 1 file changed, 59 insertions(+), 85 deletions(-) diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index dfb6167c20cac..174df09cbdbb2 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -274,10 +274,8 @@ enum Scope<'a> { #[derive(Copy, Clone, Debug)] enum BinderScopeType { - /// In a syntactic trait ref, this represents the outermost binder. So, if - /// you had `T: for<'a> Foo Baz<'a, 'b>>`, then the `for<'a>` - /// scope uses `PolyTraitRef`. - PolyTraitRef, + /// Any non-concatenating binder scopes. + Normal, /// Within a syntactic trait ref, there may be multiple poly trait refs that /// are nested (under the `associcated_type_bounds` feature). The binders of /// the innner poly trait refs are extended from the outer poly trait refs @@ -288,10 +286,6 @@ enum BinderScopeType { /// out any lifetimes because they aren't needed to show the two scopes). /// The inner `for<>` has a scope of `Concatenating`. Concatenating, - /// Any other binder scopes. These are "normal" in that they increase the binder - /// depth, are fully syntactic, don't concatenate, and don't have special syntactical - /// considerations. - Other, } // A helper struct for debugging scopes without printing parent scopes @@ -573,6 +567,43 @@ fn late_region_as_bound_region<'tcx>(tcx: TyCtxt<'tcx>, region: &Region) -> ty:: } } +impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { + /// Returns the binders in scope and the type of `Binder` that should be created for a poly trait ref. + fn poly_trait_ref_binder_info(&mut self) -> (Vec, BinderScopeType) { + let mut scope = self.scope; + let mut supertrait_lifetimes = vec![]; + loop { + match scope { + Scope::Body { .. } | Scope::Root => { + break (vec![], BinderScopeType::Normal); + } + + Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } => { + scope = s; + } + + Scope::Supertrait { s, lifetimes } => { + supertrait_lifetimes = lifetimes.clone(); + scope = s; + } + + Scope::TraitRefBoundary { .. } => { + // We should only see super trait lifetimes if there is a `Binder` above + assert!(supertrait_lifetimes.is_empty()); + break (vec![], BinderScopeType::Normal); + } + + Scope::Binder { hir_id, .. } => { + // Nested poly trait refs have the binders concatenated + let mut full_binders = + self.map.late_bound_vars.entry(*hir_id).or_default().clone(); + full_binders.extend(supertrait_lifetimes.into_iter()); + break (full_binders, BinderScopeType::Concatenating); + } + } + } + } +} impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { type Map = Map<'tcx>; @@ -630,7 +661,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: self.scope, track_lifetime_uses: true, opaque_type_parent: false, - scope_type: BinderScopeType::Other, + scope_type: BinderScopeType::Normal, }; self.with(scope, move |_old_scope, this| { intravisit::walk_fn(this, fk, fd, b, s, hir_id) @@ -755,7 +786,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index: index + non_lifetime_count, opaque_type_parent: true, track_lifetime_uses, - scope_type: BinderScopeType::Other, + scope_type: BinderScopeType::Normal, s: ROOT_SCOPE, }; self.with(scope, |old_scope, this| { @@ -827,7 +858,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index, track_lifetime_uses: true, opaque_type_parent: false, - scope_type: BinderScopeType::Other, + scope_type: BinderScopeType::Normal, }; self.with(scope, |old_scope, this| { // a bare fn has no bounds, so everything @@ -1023,7 +1054,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: this.scope, track_lifetime_uses: true, opaque_type_parent: false, - scope_type: BinderScopeType::Other, + scope_type: BinderScopeType::Normal, }; this.with(scope, |_old_scope, this| { this.visit_generics(generics); @@ -1043,7 +1074,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: self.scope, track_lifetime_uses: true, opaque_type_parent: false, - scope_type: BinderScopeType::Other, + scope_type: BinderScopeType::Normal, }; self.with(scope, |_old_scope, this| { let scope = Scope::TraitRefBoundary { s: this.scope }; @@ -1102,7 +1133,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: self.scope, track_lifetime_uses: true, opaque_type_parent: true, - scope_type: BinderScopeType::Other, + scope_type: BinderScopeType::Normal, }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -1171,7 +1202,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { s: self.scope, track_lifetime_uses: true, opaque_type_parent: true, - scope_type: BinderScopeType::Other, + scope_type: BinderScopeType::Normal, }; self.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -1287,7 +1318,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { next_early_index, track_lifetime_uses: true, opaque_type_parent: false, - scope_type: BinderScopeType::PolyTraitRef, + scope_type: BinderScopeType::Normal, }; this.with(scope, |old_scope, this| { this.check_lifetime_params(old_scope, &bound_generic_params); @@ -1317,32 +1348,15 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { } fn visit_param_bound(&mut self, bound: &'tcx hir::GenericBound<'tcx>) { - // FIXME(jackh726): This is pretty weird. `LangItemTrait` doesn't go - // through the regular poly trait ref code, so we don't get another - // chance to introduce a binder. For now, I'm keeping the existing logic - // of "if there isn't a Binder scope above us, add one", but I - // imagine there's a better way to go about this. - let mut scope = self.scope; - let (binders, scope_type) = loop { - match scope { - Scope::TraitRefBoundary { .. } | Scope::Body { .. } | Scope::Root => { - break (vec![], BinderScopeType::PolyTraitRef); - } - - Scope::Binder { hir_id, .. } => { - let binders = self.map.late_bound_vars.entry(*hir_id).or_default().clone(); - break (binders, BinderScopeType::Concatenating); - } - - Scope::Elision { s, .. } - | Scope::ObjectLifetimeDefault { s, .. } - | Scope::Supertrait { s, .. } => { - scope = s; - } - } - }; match bound { hir::GenericBound::LangItemTrait(_, _, hir_id, _) => { + // FIXME(jackh726): This is pretty weird. `LangItemTrait` doesn't go + // through the regular poly trait ref code, so we don't get another + // chance to introduce a binder. For now, I'm keeping the existing logic + // of "if there isn't a Binder scope above us, add one", but I + // imagine there's a better way to go about this. + let (binders, scope_type) = self.poly_trait_ref_binder_info(); + self.map.late_bound_vars.insert(*hir_id, binders); let scope = Scope::Binder { hir_id: *hir_id, @@ -1371,44 +1385,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { let should_pop_missing_lt = self.is_trait_ref_fn_scope(trait_ref); let next_early_index = self.next_early_index(); - let mut scope = self.scope; - let mut supertrait_lifetimes = vec![]; - let (mut binders, scope_type) = loop { - match scope { - Scope::Body { .. } | Scope::Root => { - break (vec![], BinderScopeType::PolyTraitRef); - } - - Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } => { - scope = s; - } - - Scope::Supertrait { s, lifetimes } => { - supertrait_lifetimes = lifetimes.clone(); - scope = s; - } - - Scope::TraitRefBoundary { .. } => { - // We should only see super trait lifetimes if there is a `Binder` above - assert!(supertrait_lifetimes.is_empty()); - break (vec![], BinderScopeType::PolyTraitRef); - } - - Scope::Binder { hir_id, scope_type, .. } => { - if let BinderScopeType::Other = scope_type { - bug!( - "Expected all syntacic poly trait refs to be surrounded by a `TraitRefBoundary`" - ) - } - - // Nested poly trait refs have the binders concatenated - let mut full_binders = - self.map.late_bound_vars.entry(*hir_id).or_default().clone(); - full_binders.extend(supertrait_lifetimes.into_iter()); - break (full_binders, BinderScopeType::Concatenating); - } - } - }; + let (mut binders, scope_type) = self.poly_trait_ref_binder_info(); let initial_bound_vars = binders.len() as u32; let mut lifetimes: FxHashMap = FxHashMap::default(); @@ -2185,7 +2162,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { s: self.scope, opaque_type_parent: true, track_lifetime_uses: false, - scope_type: BinderScopeType::Other, + scope_type: BinderScopeType::Normal, }; self.with(scope, move |old_scope, this| { this.check_lifetime_params(old_scope, &generics.params); @@ -2270,8 +2247,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { _ => bug!("expected LifetimeName::Param"), } match scope_type { - BinderScopeType::Other => late_depth += 1, - BinderScopeType::PolyTraitRef => late_depth += 1, + BinderScopeType::Normal => late_depth += 1, BinderScopeType::Concatenating => {} } scope = s; @@ -3001,8 +2977,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } } match scope_type { - BinderScopeType::Other => late_depth += 1, - BinderScopeType::PolyTraitRef => late_depth += 1, + BinderScopeType::Normal => late_depth += 1, BinderScopeType::Concatenating => {} } scope = s; @@ -3165,8 +3140,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { match *scope { Scope::Binder { s, scope_type, .. } => { match scope_type { - BinderScopeType::Other => late_depth += 1, - BinderScopeType::PolyTraitRef => late_depth += 1, + BinderScopeType::Normal => late_depth += 1, BinderScopeType::Concatenating => {} } scope = s;