From 5c76b64546a0bb8f087562e4d19d62b828c8d6d0 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 30 May 2018 14:36:53 +0300 Subject: [PATCH] rustc: don't visit lifetime parameters through visit_lifetime. --- src/librustc/hir/intravisit.rs | 13 ++- src/librustc/hir/lowering.rs | 35 +++--- src/librustc/hir/map/collector.rs | 14 ++- src/librustc/middle/resolve_lifetime.rs | 100 +++++++----------- src/librustc_passes/ast_validation.rs | 7 ++ src/libsyntax/visit.rs | 4 +- .../underscore-lifetime-binders.rs | 1 - src/test/run-pass/issue-51185.rs | 17 +++ 8 files changed, 102 insertions(+), 89 deletions(-) create mode 100644 src/test/run-pass/issue-51185.rs diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 5a49ee30d9c70..5471568d0af04 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -580,8 +580,8 @@ pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) { walk_list!(visitor, visit_ty, tuple_element_types); } TyBareFn(ref function_declaration) => { - visitor.visit_fn_decl(&function_declaration.decl); walk_list!(visitor, visit_generic_param, &function_declaration.generic_params); + visitor.visit_fn_decl(&function_declaration.decl); } TyPath(ref qpath) => { 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 pub fn walk_generic_param<'v, V: Visitor<'v>>(visitor: &mut V, param: &'v GenericParam) { match *param { GenericParam::Lifetime(ref ld) => { - visitor.visit_lifetime(&ld.lifetime); + visitor.visit_id(ld.lifetime.id); + match ld.lifetime.name { + LifetimeName::Name(name) => { + visitor.visit_name(ld.lifetime.span, name); + } + LifetimeName::Fresh(_) | + LifetimeName::Static | + LifetimeName::Implicit | + LifetimeName::Underscore => {} + } walk_list!(visitor, visit_lifetime, &ld.bounds); } GenericParam::Type(ref ty_param) => { diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index f24a803bac317..af269078b64e1 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1214,7 +1214,13 @@ impl<'a> LoweringContext<'a> { if let &hir::Ty_::TyBareFn(_) = &t.node { let old_collect_elided_lifetimes = self.collect_elided_lifetimes; self.collect_elided_lifetimes = false; + + // Record the "stack height" of `for<'a>` lifetime bindings + // to be able to later fully undo their introduction. + let old_len = self.currently_bound_lifetimes.len(); hir::intravisit::walk_ty(self, t); + self.currently_bound_lifetimes.truncate(old_len); + self.collect_elided_lifetimes = old_collect_elided_lifetimes; } else { hir::intravisit::walk_ty(self, t); @@ -1223,28 +1229,25 @@ impl<'a> LoweringContext<'a> { fn visit_poly_trait_ref( &mut self, - polytr: &'v hir::PolyTraitRef, - _: hir::TraitBoundModifier, + trait_ref: &'v hir::PolyTraitRef, + modifier: hir::TraitBoundModifier, ) { + // Record the "stack height" of `for<'a>` lifetime bindings + // to be able to later fully undo their introduction. let old_len = self.currently_bound_lifetimes.len(); + hir::intravisit::walk_poly_trait_ref(self, trait_ref, modifier); + self.currently_bound_lifetimes.truncate(old_len); + } + fn visit_generic_param(&mut self, param: &'v hir::GenericParam) { // Record the introduction of 'a in `for<'a> ...` - for param in &polytr.bound_generic_params { - if let hir::GenericParam::Lifetime(ref lt_def) = *param { - // Introduce lifetimes one at a time so that we can handle - // cases like `fn foo<'d>() -> impl for<'a, 'b: 'a, 'c: 'b + 'd>` - self.currently_bound_lifetimes.push(lt_def.lifetime.name); - - // Visit the lifetime bounds - for lt_bound in <_def.bounds { - self.visit_lifetime(<_bound); - } - } + if let hir::GenericParam::Lifetime(ref lt_def) = *param { + // Introduce lifetimes one at a time so that we can handle + // cases like `fn foo<'d>() -> impl for<'a, 'b: 'a, 'c: 'b + 'd>` + self.currently_bound_lifetimes.push(lt_def.lifetime.name); } - hir::intravisit::walk_trait_ref(self, &polytr.trait_ref); - - self.currently_bound_lifetimes.truncate(old_len); + hir::intravisit::walk_generic_param(self, param); } fn visit_lifetime(&mut self, lifetime: &'v hir::Lifetime) { diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index 8f28dfab1e772..13df1ced6032e 100644 --- a/src/librustc/hir/map/collector.rs +++ b/src/librustc/hir/map/collector.rs @@ -346,12 +346,16 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { }); } - fn visit_generics(&mut self, generics: &'hir Generics) { - for ty_param in generics.ty_params() { - self.insert(ty_param.id, NodeTyParam(ty_param)); + fn visit_generic_param(&mut self, param: &'hir GenericParam) { + match *param { + GenericParam::Lifetime(ref ld) => { + self.insert(ld.lifetime.id, NodeLifetime(&ld.lifetime)); + } + GenericParam::Type(ref ty_param) => { + self.insert(ty_param.id, NodeTyParam(ty_param)); + } } - - intravisit::walk_generics(self, generics); + intravisit::walk_generic_param(self, param); } fn visit_trait_item(&mut self, ti: &'hir TraitItem) { diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index 53d51d9429fb5..c82597813e2e3 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -1928,10 +1928,10 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } fn visit_generic_param(&mut self, param: &hir::GenericParam) { - if let hir::GenericParam::Lifetime(ref lifetime_def) = *param { - for l in &lifetime_def.bounds { - self.visit_lifetime(l); - } + if let hir::GenericParam::Lifetime(_) = *param { + // FIXME(eddyb) Do we want this? It only makes a difference + // if this `for<'a>` lifetime parameter is never used. + self.have_bound_regions = true; } intravisit::walk_generic_param(self, param); @@ -2144,28 +2144,26 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { fn check_lifetime_params(&mut self, old_scope: ScopeRef, params: &'tcx [hir::GenericParam]) { for (i, lifetime_i) in params.lifetimes().enumerate() { - for lifetime in params.lifetimes() { - match lifetime.lifetime.name { - hir::LifetimeName::Static | hir::LifetimeName::Underscore => { - let lifetime = lifetime.lifetime; - let name = lifetime.name.name(); - let mut err = struct_span_err!( - self.tcx.sess, - lifetime.span, - E0262, - "invalid lifetime parameter name: `{}`", - name - ); - err.span_label( - lifetime.span, - format!("{} is a reserved lifetime name", name), - ); - err.emit(); - } - hir::LifetimeName::Fresh(_) - | hir::LifetimeName::Implicit - | hir::LifetimeName::Name(_) => {} + match lifetime_i.lifetime.name { + hir::LifetimeName::Static | hir::LifetimeName::Underscore => { + let lifetime = lifetime_i.lifetime; + let name = lifetime.name.name(); + let mut err = struct_span_err!( + self.tcx.sess, + lifetime.span, + E0262, + "invalid lifetime parameter name: `{}`", + name + ); + err.span_label( + lifetime.span, + format!("{} is a reserved lifetime name", name), + ); + err.emit(); } + hir::LifetimeName::Fresh(_) + | hir::LifetimeName::Implicit + | hir::LifetimeName::Name(_) => {} } // It is a hard error to shadow a lifetime within the same scope. @@ -2347,31 +2345,18 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { | Region::LateBound(_, def_id, _) | Region::EarlyBound(_, def_id, _) => { // A lifetime declared by the user. - let def_local_id = self.tcx.hir.as_local_node_id(def_id).unwrap(); - if def_local_id == lifetime_ref.id { - // This is weird. Because the HIR defines a - // lifetime *definition* as wrapping a Lifetime, - // we wind up invoking this method also for the - // definitions in some cases (notably - // higher-ranked types). This means that a - // lifetime with one use (e.g., `for<'a> fn(&'a - // u32)`) wind up being counted as two uses. To - // avoid that, we just ignore the lifetime that - // corresponds to the definition. + let track_lifetime_uses = self.track_lifetime_uses(); + debug!( + "insert_lifetime: track_lifetime_uses={}", + track_lifetime_uses + ); + if track_lifetime_uses && !self.lifetime_uses.contains_key(&def_id) { + debug!("insert_lifetime: first use of {:?}", def_id); + self.lifetime_uses + .insert(def_id, LifetimeUseSet::One(lifetime_ref)); } else { - let track_lifetime_uses = self.track_lifetime_uses(); - debug!( - "insert_lifetime: track_lifetime_uses={}", - track_lifetime_uses - ); - if track_lifetime_uses && !self.lifetime_uses.contains_key(&def_id) { - debug!("insert_lifetime: first use of {:?}", def_id); - self.lifetime_uses - .insert(def_id, LifetimeUseSet::One(lifetime_ref)); - } else { - debug!("insert_lifetime: many uses of {:?}", def_id); - self.lifetime_uses.insert(def_id, LifetimeUseSet::Many); - } + debug!("insert_lifetime: many uses of {:?}", def_id); + self.lifetime_uses.insert(def_id, LifetimeUseSet::Many); } } } @@ -2424,31 +2409,20 @@ fn insert_late_bound_lifetimes( let mut appears_in_where_clause = AllCollector { regions: FxHashSet(), }; + appears_in_where_clause.visit_generics(generics); for param in &generics.params { match *param { hir::GenericParam::Lifetime(ref lifetime_def) => { if !lifetime_def.bounds.is_empty() { // `'a: 'b` means both `'a` and `'b` are referenced - appears_in_where_clause.visit_generic_param(param); + appears_in_where_clause.regions.insert(lifetime_def.lifetime.name); } } - hir::GenericParam::Type(ref ty_param) => { - walk_list!( - &mut appears_in_where_clause, - visit_ty_param_bound, - &ty_param.bounds - ); - } + hir::GenericParam::Type(_) => {} } } - walk_list!( - &mut appears_in_where_clause, - visit_where_predicate, - &generics.where_clause.predicates - ); - debug!( "insert_late_bound_lifetimes: appears_in_where_clause={:?}", appears_in_where_clause.regions diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index abbd399a944a0..39e0a5392580b 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -441,6 +441,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> { visit::walk_generics(self, g) } + fn visit_generic_param(&mut self, param: &'a GenericParam) { + if let GenericParam::Lifetime(ref ld) = *param { + self.check_lifetime(ld.lifetime.ident); + } + visit::walk_generic_param(self, param); + } + fn visit_pat(&mut self, pat: &'a Pat) { match pat.node { PatKind::Lit(ref expr) => { diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index fdf8e52bbddd7..dca0e2f634c74 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -318,8 +318,8 @@ pub fn walk_ty<'a, V: Visitor<'a>>(visitor: &mut V, typ: &'a Ty) { walk_list!(visitor, visit_ty, tuple_element_types); } TyKind::BareFn(ref function_declaration) => { - walk_fn_decl(visitor, &function_declaration.decl); walk_list!(visitor, visit_generic_param, &function_declaration.generic_params); + walk_fn_decl(visitor, &function_declaration.decl); } TyKind::Path(ref maybe_qself, ref path) => { if let Some(ref qself) = *maybe_qself { @@ -485,7 +485,7 @@ pub fn walk_ty_param_bound<'a, V: Visitor<'a>>(visitor: &mut V, bound: &'a TyPar pub fn walk_generic_param<'a, V: Visitor<'a>>(visitor: &mut V, param: &'a GenericParam) { match *param { GenericParam::Lifetime(ref l) => { - visitor.visit_lifetime(&l.lifetime); + visitor.visit_ident(l.lifetime.ident); walk_list!(visitor, visit_lifetime, &l.bounds); walk_list!(visitor, visit_attribute, &*l.attrs); } diff --git a/src/test/compile-fail/underscore-lifetime-binders.rs b/src/test/compile-fail/underscore-lifetime-binders.rs index eb00ab5f67afe..6a6698957d9d1 100644 --- a/src/test/compile-fail/underscore-lifetime-binders.rs +++ b/src/test/compile-fail/underscore-lifetime-binders.rs @@ -23,7 +23,6 @@ impl<'a> Meh<'a> for u8 {} fn meh() -> Box Meh<'_>> //~ ERROR invalid lifetime parameter name: `'_` //~^ ERROR missing lifetime specifier -//~^^ ERROR missing lifetime specifier { Box::new(5u8) } diff --git a/src/test/run-pass/issue-51185.rs b/src/test/run-pass/issue-51185.rs new file mode 100644 index 0000000000000..8e286ad1419f2 --- /dev/null +++ b/src/test/run-pass/issue-51185.rs @@ -0,0 +1,17 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn foo() -> impl Into fn(&'a ())> { + (|_| {}) as for<'a> fn(&'a ()) +} + +fn main() { + foo().into()(&()); +}