From a6d6eea4c8ac8b9bd97ecd466418ee739a9a97fa Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 26 Jul 2019 18:30:32 -0400 Subject: [PATCH 1/3] Remove unused scope tracking --- src/librustc_save_analysis/dump_visitor.rs | 25 ++++++---------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 2b349613dc54f..062033304ee89 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -23,7 +23,7 @@ use rustc_data_structures::fx::FxHashSet; use std::path::Path; use std::env; -use syntax::ast::{self, Attribute, NodeId, PatKind, CRATE_NODE_ID}; +use syntax::ast::{self, Attribute, NodeId, PatKind}; use syntax::parse::token; use syntax::visit::{self, Visitor}; use syntax::print::pprust::{ @@ -82,8 +82,6 @@ pub struct DumpVisitor<'l, 'tcx, 'll> { span: SpanUtils<'l>, - cur_scope: NodeId, - // Set of macro definition (callee) spans, and the set // of macro use (callsite) spans. We store these to ensure // we only write one macro def per unique macro definition, and @@ -103,22 +101,11 @@ impl<'l, 'tcx, 'll> DumpVisitor<'l, 'tcx, 'll> { save_ctxt, dumper, span: span_utils, - cur_scope: CRATE_NODE_ID, // mac_defs: FxHashSet::default(), // macro_calls: FxHashSet::default(), } } - fn nest_scope(&mut self, scope_id: NodeId, f: F) - where - F: FnOnce(&mut DumpVisitor<'l, 'tcx, 'll>), - { - let parent_scope = self.cur_scope; - self.cur_scope = scope_id; - f(self); - self.cur_scope = parent_scope; - } - fn nest_tables(&mut self, item_id: NodeId, f: F) where F: FnOnce(&mut DumpVisitor<'l, 'tcx, 'll>), @@ -320,7 +307,7 @@ impl<'l, 'tcx, 'll> DumpVisitor<'l, 'tcx, 'll> { // walk the fn body if let Some(body) = body { - self.nest_tables(id, |v| v.nest_scope(id, |v| v.visit_block(body))); + self.nest_tables(id, |v| v.visit_block(body)); } } @@ -405,7 +392,7 @@ impl<'l, 'tcx, 'll> DumpVisitor<'l, 'tcx, 'll> { self.visit_ty(&ret_ty); } - self.nest_tables(item.id, |v| v.nest_scope(item.id, |v| v.visit_block(&body))); + self.nest_tables(item.id, |v| v.visit_block(&body)); } fn process_static_or_const_item( @@ -1349,7 +1336,7 @@ impl<'l, 'tcx, 'll> Visitor<'l> for DumpVisitor<'l, 'tcx, 'll> { attributes: lower_attributes(attrs.to_owned(), &self.save_ctxt), }, ); - self.nest_scope(id, |v| visit::walk_mod(v, m)); + visit::walk_mod(self, m); } fn visit_item(&mut self, item: &'l ast::Item) { @@ -1404,7 +1391,7 @@ impl<'l, 'tcx, 'll> Visitor<'l> for DumpVisitor<'l, 'tcx, 'll> { } Mod(ref m) => { self.process_mod(item); - self.nest_scope(item.id, |v| visit::walk_mod(v, m)); + visit::walk_mod(self, m); } Ty(ref ty, ref ty_params) => { let qualname = format!("::{}", @@ -1570,7 +1557,7 @@ impl<'l, 'tcx, 'll> Visitor<'l> for DumpVisitor<'l, 'tcx, 'll> { // walk the body self.nest_tables(ex.id, |v| { v.process_formals(&decl.inputs, &id); - v.nest_scope(ex.id, |v| v.visit_expr(body)) + v.visit_expr(body) }); } ast::ExprKind::ForLoop(ref pattern, ref subexpression, ref block, _) => { From 5b822891be160d11b4e53770c1174e68d8b091ca Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 26 Jul 2019 18:36:10 -0400 Subject: [PATCH 2/3] Store dumper directly in Visitor --- src/librustc_save_analysis/dump_visitor.rs | 18 +++++++++++------- src/librustc_save_analysis/dumper.rs | 4 ++-- src/librustc_save_analysis/lib.rs | 22 ++++++---------------- 3 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 062033304ee89..9dfc48c1288c1 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -75,10 +75,10 @@ macro_rules! access_from_vis { }; } -pub struct DumpVisitor<'l, 'tcx, 'll> { +pub struct DumpVisitor<'l, 'tcx> { save_ctxt: SaveContext<'l, 'tcx>, tcx: TyCtxt<'tcx>, - dumper: &'ll mut Dumper, + dumper: Dumper, span: SpanUtils<'l>, @@ -90,12 +90,12 @@ pub struct DumpVisitor<'l, 'tcx, 'll> { // macro_calls: FxHashSet, } -impl<'l, 'tcx, 'll> DumpVisitor<'l, 'tcx, 'll> { +impl<'l, 'tcx> DumpVisitor<'l, 'tcx> { pub fn new( save_ctxt: SaveContext<'l, 'tcx>, - dumper: &'ll mut Dumper, - ) -> DumpVisitor<'l, 'tcx, 'll> { + ) -> DumpVisitor<'l, 'tcx> { let span_utils = SpanUtils::new(&save_ctxt.tcx.sess); + let dumper = Dumper::new(save_ctxt.config.clone()); DumpVisitor { tcx: save_ctxt.tcx, save_ctxt, @@ -106,9 +106,13 @@ impl<'l, 'tcx, 'll> DumpVisitor<'l, 'tcx, 'll> { } } + pub fn into_analysis(self) -> rls_data::Analysis { + self.dumper.into_analysis() + } + fn nest_tables(&mut self, item_id: NodeId, f: F) where - F: FnOnce(&mut DumpVisitor<'l, 'tcx, 'll>), + F: FnOnce(&mut Self), { let item_def_id = self.tcx.hir().local_def_id_from_node_id(item_id); if self.tcx.has_typeck_tables(item_def_id) { @@ -1298,7 +1302,7 @@ impl<'l, 'tcx, 'll> DumpVisitor<'l, 'tcx, 'll> { } } -impl<'l, 'tcx, 'll> Visitor<'l> for DumpVisitor<'l, 'tcx, 'll> { +impl<'l, 'tcx> Visitor<'l> for DumpVisitor<'l, 'tcx> { fn visit_mod(&mut self, m: &'l ast::Mod, span: Span, attrs: &[ast::Attribute], id: NodeId) { // Since we handle explicit modules ourselves in visit_item, this should // only get called for the root module of a crate. diff --git a/src/librustc_save_analysis/dumper.rs b/src/librustc_save_analysis/dumper.rs index 6fb55e6c99055..a0051c30c9775 100644 --- a/src/librustc_save_analysis/dumper.rs +++ b/src/librustc_save_analysis/dumper.rs @@ -22,8 +22,8 @@ impl Dumper { } } - pub fn to_output(self, f: impl FnOnce(&Analysis)) { - f(&self.result) + pub fn into_analysis(self) -> Analysis { + self.result } } diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index ade5e2eca60ba..af69c79cae563 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -39,7 +39,6 @@ use syntax::visit::{self, Visitor}; use syntax::print::pprust::{arg_to_string, ty_to_string}; use syntax_pos::*; -use dumper::Dumper; use dump_visitor::DumpVisitor; use span_utils::SpanUtils; @@ -1076,18 +1075,15 @@ impl<'a> SaveHandler for DumpHandler<'a> { ) { let sess = &save_ctxt.tcx.sess; let (output, file_name) = self.output_file(&save_ctxt); - let mut dumper = Dumper::new(save_ctxt.config.clone()); - let mut visitor = DumpVisitor::new(save_ctxt, &mut dumper); + let mut visitor = DumpVisitor::new(save_ctxt); visitor.dump_crate_info(cratename, krate); visitor.dump_compilation_options(input, cratename); visit::walk_crate(&mut visitor, krate); - dumper.to_output(|analysis| { - if let Err(e) = serde_json::to_writer(output, analysis) { - error!("Can't serialize save-analysis: {:?}", e); - } - }); + if let Err(e) = serde_json::to_writer(output, &visitor.into_analysis()) { + error!("Can't serialize save-analysis: {:?}", e); + } if sess.opts.debugging_opts.emit_artifact_notifications { sess.parse_sess.span_diagnostic @@ -1109,19 +1105,13 @@ impl<'b> SaveHandler for CallbackHandler<'b> { cratename: &str, input: &'l Input, ) { - // We're using the Dumper here because it has the format of the - // save-analysis results that we will pass to the callback. IOW, we are - // using the Dumper to collect the save-analysis results, but not - // actually to dump them to a file. This is all a bit convoluted and - // there is certainly a simpler design here trying to get out (FIXME). - let mut dumper = Dumper::new(save_ctxt.config.clone()); - let mut visitor = DumpVisitor::new(save_ctxt, &mut dumper); + let mut visitor = DumpVisitor::new(save_ctxt); visitor.dump_crate_info(cratename, krate); visitor.dump_compilation_options(input, cratename); visit::walk_crate(&mut visitor, krate); - dumper.to_output(|a| (self.callback)(a)) + (self.callback)(&visitor.into_analysis()) } } From 5ff08569541b94d3ba8d645fb7fc5c1c75c6cda1 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 26 Jul 2019 20:15:31 -0400 Subject: [PATCH 3/3] Simplify SaveHandler trait This extracts the core visiting logic --- src/librustc_save_analysis/dump_visitor.rs | 6 +-- src/librustc_save_analysis/dumper.rs | 4 +- src/librustc_save_analysis/lib.rs | 54 +++++++++------------- 3 files changed, 26 insertions(+), 38 deletions(-) diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 9dfc48c1288c1..6fce7ca1f33fb 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -76,7 +76,7 @@ macro_rules! access_from_vis { } pub struct DumpVisitor<'l, 'tcx> { - save_ctxt: SaveContext<'l, 'tcx>, + pub save_ctxt: SaveContext<'l, 'tcx>, tcx: TyCtxt<'tcx>, dumper: Dumper, @@ -106,8 +106,8 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> { } } - pub fn into_analysis(self) -> rls_data::Analysis { - self.dumper.into_analysis() + pub fn analysis(&self) -> &rls_data::Analysis { + self.dumper.analysis() } fn nest_tables(&mut self, item_id: NodeId, f: F) diff --git a/src/librustc_save_analysis/dumper.rs b/src/librustc_save_analysis/dumper.rs index a0051c30c9775..b80778c8fec7e 100644 --- a/src/librustc_save_analysis/dumper.rs +++ b/src/librustc_save_analysis/dumper.rs @@ -22,8 +22,8 @@ impl Dumper { } } - pub fn into_analysis(self) -> Analysis { - self.result + pub fn analysis(&self) -> &Analysis { + &self.result } } diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index af69c79cae563..5e66b11bfe30d 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -43,7 +43,7 @@ use dump_visitor::DumpVisitor; use span_utils::SpanUtils; use rls_data::{Def, DefKind, ExternalCrateData, GlobalCrateId, MacroRef, Ref, RefKind, Relation, - RelationKind, SpanData, Impl, ImplKind}; + RelationKind, SpanData, Impl, ImplKind, Analysis}; use rls_data::config::Config; use log::{debug, error, info}; @@ -1000,12 +1000,10 @@ impl<'l> Visitor<'l> for PathCollector<'l> { /// Defines what to do with the results of saving the analysis. pub trait SaveHandler { - fn save<'l, 'tcx>( + fn save( &mut self, - save_ctxt: SaveContext<'l, 'tcx>, - krate: &ast::Crate, - cratename: &str, - input: &'l Input, + save_ctxt: &SaveContext<'_, '_>, + analysis: &Analysis, ); } @@ -1065,23 +1063,15 @@ impl<'a> DumpHandler<'a> { } } -impl<'a> SaveHandler for DumpHandler<'a> { - fn save<'l, 'tcx>( +impl SaveHandler for DumpHandler<'_> { + fn save( &mut self, - save_ctxt: SaveContext<'l, 'tcx>, - krate: &ast::Crate, - cratename: &str, - input: &'l Input, + save_ctxt: &SaveContext<'_, '_>, + analysis: &Analysis, ) { let sess = &save_ctxt.tcx.sess; let (output, file_name) = self.output_file(&save_ctxt); - let mut visitor = DumpVisitor::new(save_ctxt); - - visitor.dump_crate_info(cratename, krate); - visitor.dump_compilation_options(input, cratename); - visit::walk_crate(&mut visitor, krate); - - if let Err(e) = serde_json::to_writer(output, &visitor.into_analysis()) { + if let Err(e) = serde_json::to_writer(output, &analysis) { error!("Can't serialize save-analysis: {:?}", e); } @@ -1097,21 +1087,13 @@ pub struct CallbackHandler<'b> { pub callback: &'b mut dyn FnMut(&rls_data::Analysis), } -impl<'b> SaveHandler for CallbackHandler<'b> { - fn save<'l, 'tcx>( +impl SaveHandler for CallbackHandler<'_> { + fn save( &mut self, - save_ctxt: SaveContext<'l, 'tcx>, - krate: &ast::Crate, - cratename: &str, - input: &'l Input, + _: &SaveContext<'_, '_>, + analysis: &Analysis, ) { - let mut visitor = DumpVisitor::new(save_ctxt); - - visitor.dump_crate_info(cratename, krate); - visitor.dump_compilation_options(input, cratename); - visit::walk_crate(&mut visitor, krate); - - (self.callback)(&visitor.into_analysis()) + (self.callback)(analysis) } } @@ -1142,7 +1124,13 @@ pub fn process_crate<'l, 'tcx, H: SaveHandler>( impl_counter: Cell::new(0), }; - handler.save(save_ctxt, krate, cratename, input) + let mut visitor = DumpVisitor::new(save_ctxt); + + visitor.dump_crate_info(cratename, krate); + visitor.dump_compilation_options(input, cratename); + visit::walk_crate(&mut visitor, krate); + + handler.save(&visitor.save_ctxt, &visitor.analysis()) }) }