Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various cleanups to save analysis #63055

Merged
merged 3 commits into from
Jul 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 16 additions & 25 deletions src/librustc_save_analysis/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -75,15 +75,13 @@ macro_rules! access_from_vis {
};
}

pub struct DumpVisitor<'l, 'tcx, 'll> {
save_ctxt: SaveContext<'l, 'tcx>,
pub struct DumpVisitor<'l, 'tcx> {
pub save_ctxt: SaveContext<'l, 'tcx>,
tcx: TyCtxt<'tcx>,
dumper: &'ll mut Dumper,
dumper: Dumper,

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
Expand All @@ -92,36 +90,29 @@ pub struct DumpVisitor<'l, 'tcx, 'll> {
// macro_calls: FxHashSet<Span>,
}

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,
dumper,
span: span_utils,
cur_scope: CRATE_NODE_ID,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, that's weird. So it was unused all along?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the uses (but not the tracking) was removed in 8a2857e by @nrc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, makes sense. Thanks!

// mac_defs: FxHashSet::default(),
// macro_calls: FxHashSet::default(),
}
}

fn nest_scope<F>(&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;
pub fn analysis(&self) -> &rls_data::Analysis {
self.dumper.analysis()
}

fn nest_tables<F>(&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) {
Expand Down Expand Up @@ -320,7 +311,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));
}
}

Expand Down Expand Up @@ -405,7 +396,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(
Expand Down Expand Up @@ -1311,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.
Expand Down Expand Up @@ -1349,7 +1340,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) {
Expand Down Expand Up @@ -1404,7 +1395,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!("::{}",
Expand Down Expand Up @@ -1570,7 +1561,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, _) => {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_save_analysis/dumper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ impl Dumper {
}
}

pub fn to_output(self, f: impl FnOnce(&Analysis)) {
f(&self.result)
pub fn analysis(&self) -> &Analysis {
&self.result
}
}

Expand Down
68 changes: 23 additions & 45 deletions src/librustc_save_analysis/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,11 @@ 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;

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};
Expand Down Expand Up @@ -1001,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,
);
}

Expand Down Expand Up @@ -1066,28 +1063,17 @@ 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 dumper = Dumper::new(save_ctxt.config.clone());
let mut visitor = DumpVisitor::new(save_ctxt, &mut dumper);

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, &analysis) {
error!("Can't serialize save-analysis: {:?}", e);
}

if sess.opts.debugging_opts.emit_artifact_notifications {
sess.parse_sess.span_diagnostic
Expand All @@ -1101,27 +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,
) {
// 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);

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)(analysis)
}
}

Expand Down Expand Up @@ -1152,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())
})
}

Expand Down