Skip to content

Commit 5d0631a

Browse files
borsbors
committed
Auto merge of #51215 - eddyb:visit-for-a-lifetime, r=nikomatsakis
 rustc: don't visit lifetime parameters through visit_lifetime. Ideally we'd also not use the `Lifetime` struct for parameters, but I'll leave that to @varkor (#48149). Fixes #51185 (discovered while auditing all the `visit_lifetime` implementations). r? @nikomatsakis
2 parents fddb46e + 5c76b64 commit 5d0631a

File tree

9 files changed

+122
-143
lines changed

9 files changed

+122
-143
lines changed

‎src/librustc/hir/intravisit.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,8 +580,8 @@ pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) {
580580
walk_list!(visitor, visit_ty, tuple_element_types);
581581
}
582582
TyBareFn(ref function_declaration) => {
583-
visitor.visit_fn_decl(&function_declaration.decl);
584583
walk_list!(visitor, visit_generic_param, &function_declaration.generic_params);
584+
visitor.visit_fn_decl(&function_declaration.decl);
585585
}
586586
TyPath(ref qpath) => {
587587
visitor.visit_qpath(qpath, typ.id, typ.span);
@@ -733,7 +733,16 @@ pub fn walk_ty_param_bound<'v, V: Visitor<'v>>(visitor: &mut V, bound: &'v TyPar
733733
pub fn walk_generic_param<'v, V: Visitor<'v>>(visitor: &mut V, param: &'v GenericParam) {
734734
match *param {
735735
GenericParam::Lifetime(ref ld) => {
736-
visitor.visit_lifetime(&ld.lifetime);
736+
visitor.visit_id(ld.lifetime.id);
737+
match ld.lifetime.name {
738+
LifetimeName::Name(name) => {
739+
visitor.visit_name(ld.lifetime.span, name);
740+
}
741+
LifetimeName::Fresh(_) |
742+
LifetimeName::Static |
743+
LifetimeName::Implicit |
744+
LifetimeName::Underscore => {}
745+
}
737746
walk_list!(visitor, visit_lifetime, &ld.bounds);
738747
}
739748
GenericParam::Type(ref ty_param) => {

‎src/librustc/hir/lowering.rs

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,7 +1214,13 @@ impl<'a> LoweringContext<'a> {
12141214
if let &hir::Ty_::TyBareFn(_) = &t.node {
12151215
let old_collect_elided_lifetimes = self.collect_elided_lifetimes;
12161216
self.collect_elided_lifetimes = false;
1217+
1218+
// Record the "stack height" of `for<'a>` lifetime bindings
1219+
// to be able to later fully undo their introduction.
1220+
let old_len = self.currently_bound_lifetimes.len();
12171221
hir::intravisit::walk_ty(self, t);
1222+
self.currently_bound_lifetimes.truncate(old_len);
1223+
12181224
self.collect_elided_lifetimes = old_collect_elided_lifetimes;
12191225
} else {
12201226
hir::intravisit::walk_ty(self, t);
@@ -1223,28 +1229,25 @@ impl<'a> LoweringContext<'a> {
12231229

12241230
fn visit_poly_trait_ref(
12251231
&mut self,
1226-
polytr: &'v hir::PolyTraitRef,
1227-
_: hir::TraitBoundModifier,
1232+
trait_ref: &'v hir::PolyTraitRef,
1233+
modifier: hir::TraitBoundModifier,
12281234
) {
1235+
// Record the "stack height" of `for<'a>` lifetime bindings
1236+
// to be able to later fully undo their introduction.
12291237
let old_len = self.currently_bound_lifetimes.len();
1238+
hir::intravisit::walk_poly_trait_ref(self, trait_ref, modifier);
1239+
self.currently_bound_lifetimes.truncate(old_len);
1240+
}
12301241

1242+
fn visit_generic_param(&mut self, param: &'v hir::GenericParam) {
12311243
// Record the introduction of 'a in `for<'a> ...`
1232-
for param in &polytr.bound_generic_params {
1233-
if let hir::GenericParam::Lifetime(ref lt_def) = *param {
1234-
// Introduce lifetimes one at a time so that we can handle
1235-
// cases like `fn foo<'d>() -> impl for<'a, 'b: 'a, 'c: 'b + 'd>`
1236-
self.currently_bound_lifetimes.push(lt_def.lifetime.name);
1237-
1238-
// Visit the lifetime bounds
1239-
for lt_bound in &lt_def.bounds {
1240-
self.visit_lifetime(&lt_bound);
1241-
}
1242-
}
1244+
if let hir::GenericParam::Lifetime(ref lt_def) = *param {
1245+
// Introduce lifetimes one at a time so that we can handle
1246+
// cases like `fn foo<'d>() -> impl for<'a, 'b: 'a, 'c: 'b + 'd>`
1247+
self.currently_bound_lifetimes.push(lt_def.lifetime.name);
12431248
}
12441249

1245-
hir::intravisit::walk_trait_ref(self, &polytr.trait_ref);
1246-
1247-
self.currently_bound_lifetimes.truncate(old_len);
1250+
hir::intravisit::walk_generic_param(self, param);
12481251
}
12491252

12501253
fn visit_lifetime(&mut self, lifetime: &'v hir::Lifetime) {

‎src/librustc/hir/map/collector.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -346,12 +346,16 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
346346
});
347347
}
348348

349-
fn visit_generics(&mut self, generics: &'hir Generics) {
350-
for ty_param in generics.ty_params() {
351-
self.insert(ty_param.id, NodeTyParam(ty_param));
349+
fn visit_generic_param(&mut self, param: &'hir GenericParam) {
350+
match *param {
351+
GenericParam::Lifetime(ref ld) => {
352+
self.insert(ld.lifetime.id, NodeLifetime(&ld.lifetime));
353+
}
354+
GenericParam::Type(ref ty_param) => {
355+
self.insert(ty_param.id, NodeTyParam(ty_param));
356+
}
352357
}
353-
354-
intravisit::walk_generics(self, generics);
358+
intravisit::walk_generic_param(self, param);
355359
}
356360

357361
fn visit_trait_item(&mut self, ti: &'hir TraitItem) {

‎src/librustc/middle/resolve_lifetime.rs

Lines changed: 37 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1928,10 +1928,10 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
19281928
}
19291929

19301930
fn visit_generic_param(&mut self, param: &hir::GenericParam) {
1931-
if let hir::GenericParam::Lifetime(ref lifetime_def) = *param {
1932-
for l in &lifetime_def.bounds {
1933-
self.visit_lifetime(l);
1934-
}
1931+
if let hir::GenericParam::Lifetime(_) = *param {
1932+
// FIXME(eddyb) Do we want this? It only makes a difference
1933+
// if this `for<'a>` lifetime parameter is never used.
1934+
self.have_bound_regions = true;
19351935
}
19361936

19371937
intravisit::walk_generic_param(self, param);
@@ -2144,28 +2144,26 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
21442144

21452145
fn check_lifetime_params(&mut self, old_scope: ScopeRef, params: &'tcx [hir::GenericParam]) {
21462146
for (i, lifetime_i) in params.lifetimes().enumerate() {
2147-
for lifetime in params.lifetimes() {
2148-
match lifetime.lifetime.name {
2149-
hir::LifetimeName::Static | hir::LifetimeName::Underscore => {
2150-
let lifetime = lifetime.lifetime;
2151-
let name = lifetime.name.name();
2152-
let mut err = struct_span_err!(
2153-
self.tcx.sess,
2154-
lifetime.span,
2155-
E0262,
2156-
"invalid lifetime parameter name: `{}`",
2157-
name
2158-
);
2159-
err.span_label(
2160-
lifetime.span,
2161-
format!("{} is a reserved lifetime name", name),
2162-
);
2163-
err.emit();
2164-
}
2165-
hir::LifetimeName::Fresh(_)
2166-
| hir::LifetimeName::Implicit
2167-
| hir::LifetimeName::Name(_) => {}
2147+
match lifetime_i.lifetime.name {
2148+
hir::LifetimeName::Static | hir::LifetimeName::Underscore => {
2149+
let lifetime = lifetime_i.lifetime;
2150+
let name = lifetime.name.name();
2151+
let mut err = struct_span_err!(
2152+
self.tcx.sess,
2153+
lifetime.span,
2154+
E0262,
2155+
"invalid lifetime parameter name: `{}`",
2156+
name
2157+
);
2158+
err.span_label(
2159+
lifetime.span,
2160+
format!("{} is a reserved lifetime name", name),
2161+
);
2162+
err.emit();
21682163
}
2164+
hir::LifetimeName::Fresh(_)
2165+
| hir::LifetimeName::Implicit
2166+
| hir::LifetimeName::Name(_) => {}
21692167
}
21702168

21712169
// It is a hard error to shadow a lifetime within the same scope.
@@ -2347,31 +2345,18 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
23472345
| Region::LateBound(_, def_id, _)
23482346
| Region::EarlyBound(_, def_id, _) => {
23492347
// A lifetime declared by the user.
2350-
let def_local_id = self.tcx.hir.as_local_node_id(def_id).unwrap();
2351-
if def_local_id == lifetime_ref.id {
2352-
// This is weird. Because the HIR defines a
2353-
// lifetime *definition* as wrapping a Lifetime,
2354-
// we wind up invoking this method also for the
2355-
// definitions in some cases (notably
2356-
// higher-ranked types). This means that a
2357-
// lifetime with one use (e.g., `for<'a> fn(&'a
2358-
// u32)`) wind up being counted as two uses. To
2359-
// avoid that, we just ignore the lifetime that
2360-
// corresponds to the definition.
2348+
let track_lifetime_uses = self.track_lifetime_uses();
2349+
debug!(
2350+
"insert_lifetime: track_lifetime_uses={}",
2351+
track_lifetime_uses
2352+
);
2353+
if track_lifetime_uses && !self.lifetime_uses.contains_key(&def_id) {
2354+
debug!("insert_lifetime: first use of {:?}", def_id);
2355+
self.lifetime_uses
2356+
.insert(def_id, LifetimeUseSet::One(lifetime_ref));
23612357
} else {
2362-
let track_lifetime_uses = self.track_lifetime_uses();
2363-
debug!(
2364-
"insert_lifetime: track_lifetime_uses={}",
2365-
track_lifetime_uses
2366-
);
2367-
if track_lifetime_uses && !self.lifetime_uses.contains_key(&def_id) {
2368-
debug!("insert_lifetime: first use of {:?}", def_id);
2369-
self.lifetime_uses
2370-
.insert(def_id, LifetimeUseSet::One(lifetime_ref));
2371-
} else {
2372-
debug!("insert_lifetime: many uses of {:?}", def_id);
2373-
self.lifetime_uses.insert(def_id, LifetimeUseSet::Many);
2374-
}
2358+
debug!("insert_lifetime: many uses of {:?}", def_id);
2359+
self.lifetime_uses.insert(def_id, LifetimeUseSet::Many);
23752360
}
23762361
}
23772362
}
@@ -2424,31 +2409,20 @@ fn insert_late_bound_lifetimes(
24242409
let mut appears_in_where_clause = AllCollector {
24252410
regions: FxHashSet(),
24262411
};
2412+
appears_in_where_clause.visit_generics(generics);
24272413

24282414
for param in &generics.params {
24292415
match *param {
24302416
hir::GenericParam::Lifetime(ref lifetime_def) => {
24312417
if !lifetime_def.bounds.is_empty() {
24322418
// `'a: 'b` means both `'a` and `'b` are referenced
2433-
appears_in_where_clause.visit_generic_param(param);
2419+
appears_in_where_clause.regions.insert(lifetime_def.lifetime.name);
24342420
}
24352421
}
2436-
hir::GenericParam::Type(ref ty_param) => {
2437-
walk_list!(
2438-
&mut appears_in_where_clause,
2439-
visit_ty_param_bound,
2440-
&ty_param.bounds
2441-
);
2442-
}
2422+
hir::GenericParam::Type(_) => {}
24432423
}
24442424
}
24452425

2446-
walk_list!(
2447-
&mut appears_in_where_clause,
2448-
visit_where_predicate,
2449-
&generics.where_clause.predicates
2450-
);
2451-
24522426
debug!(
24532427
"insert_late_bound_lifetimes: appears_in_where_clause={:?}",
24542428
appears_in_where_clause.regions

‎src/librustc_passes/ast_validation.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
441441
visit::walk_generics(self, g)
442442
}
443443

444+
fn visit_generic_param(&mut self, param: &'a GenericParam) {
445+
if let GenericParam::Lifetime(ref ld) = *param {
446+
self.check_lifetime(ld.lifetime.ident);
447+
}
448+
visit::walk_generic_param(self, param);
449+
}
450+
444451
fn visit_pat(&mut self, pat: &'a Pat) {
445452
match pat.node {
446453
PatKind::Lit(ref expr) => {

0 commit comments

Comments
 (0)