Skip to content

Fix a bunch of minor save-analysis bugs #45709

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 4, 2017
Merged

Fix a bunch of minor save-analysis bugs #45709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1d3e44
save-analysis: handle function types in bounds
nrc Nov 1, 2017
82a8968
save-analysis: handle types in turbofish
nrc Nov 1, 2017
20c64e8
save-analysis: corrects reference for tuple struct and unit struct li…
nrc Nov 1, 2017
5d3be12
save-analysis: fix issue with sub-exprs in for loops
nrc Nov 2, 2017
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
107 changes: 60 additions & 47 deletions src/librustc_save_analysis/dump_visitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,25 +318,24 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
let mut collector = PathCollector::new();
collector.visit_pat(&arg.pat);
let span_utils = self.span.clone();
for &(id, ref p, ..) in &collector.collected_paths {

for (id, i, sp, ..) in collector.collected_idents {
let hir_id = self.tcx.hir.node_to_hir_id(id);
let typ = match self.save_ctxt.tables.node_id_to_type_opt(hir_id) {
Some(s) => s.to_string(),
None => continue,
};
// get the span only for the name of the variable (I hope the path is only ever a
// variable name, but who knows?)
let sub_span = span_utils.span_for_last_ident(p.span);
if !self.span.filter_generated(sub_span, p.span) {
let sub_span = span_utils.span_for_last_ident(sp);
if !self.span.filter_generated(sub_span, sp) {
let id = ::id_from_node_id(id, &self.save_ctxt);
let span = self.span_from_span(sub_span.expect("No span found for variable"));

self.dumper.dump_def(false, Def {
kind: DefKind::Local,
id,
span,
name: path_to_string(p),
qualname: format!("{}::{}", qualname, path_to_string(p)),
name: i.to_string(),
qualname: format!("{}::{}", qualname, i.to_string()),
value: typ,
parent: None,
children: vec![],
Expand Down Expand Up @@ -391,14 +390,6 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
}
}

fn process_trait_ref(&mut self, trait_ref: &'l ast::TraitRef) {
let trait_ref_data = self.save_ctxt.get_trait_ref_data(trait_ref);
if let Some(trait_ref_data) = trait_ref_data {
self.dumper.dump_ref(trait_ref_data);
}
self.process_path(trait_ref.ref_id, &trait_ref.path);
}

fn process_struct_field_def(&mut self, field: &ast::StructField, parent_id: NodeId) {
let field_data = self.save_ctxt.get_field_data(field, parent_id);
if let Some(field_data) = field_data {
Expand Down Expand Up @@ -783,7 +774,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
}
}

fn process_path(&mut self, id: NodeId, path: &ast::Path) {
fn process_path(&mut self, id: NodeId, path: &'l ast::Path) {
debug!("process_path {:?}", path);
let path_data = self.save_ctxt.get_path_data(id, path);
if generated_code(path.span) && path_data.is_none() {
return;
Expand All @@ -798,6 +790,27 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {

self.dumper.dump_ref(path_data);

// Type parameters
for seg in &path.segments {
if let Some(ref params) = seg.parameters {
match **params {
ast::PathParameters::AngleBracketed(ref data) => {
for t in &data.types {
self.visit_ty(t);
}
}
ast::PathParameters::Parenthesized(ref data) => {
for t in &data.inputs {
self.visit_ty(t);
}
if let Some(ref t) = data.output {
self.visit_ty(t);
}
}
}
}
}

// Modules or types in the path prefix.
match self.save_ctxt.get_path_def(id) {
HirDef::Method(did) => {
Expand Down Expand Up @@ -850,14 +863,26 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
walk_list!(self, visit_expr, base);
}

fn process_method_call(&mut self, ex: &'l ast::Expr, args: &'l [P<ast::Expr>]) {
fn process_method_call(&mut self,
ex: &'l ast::Expr,
seg: &'l ast::PathSegment,
args: &'l [P<ast::Expr>]) {
if let Some(mcd) = self.save_ctxt.get_expr_data(ex) {
down_cast_data!(mcd, RefData, ex.span);
if !generated_code(ex.span) {
self.dumper.dump_ref(mcd);
}
}

// Explicit types in the turbo-fish.
if let Some(ref params) = seg.parameters {
if let ast::PathParameters::AngleBracketed(ref data) = **params {
for t in &data.types {
self.visit_ty(t);
}
}
}

// walk receiver and args
walk_list!(self, visit_expr, args);
}
Expand Down Expand Up @@ -904,7 +929,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
collector.visit_pat(&p);
self.visit_pat(&p);

for &(id, ref p, immut) in &collector.collected_paths {
for (id, i, sp, immut) in collector.collected_idents {
let mut value = match immut {
ast::Mutability::Immutable => value.to_string(),
_ => String::new(),
Expand All @@ -924,18 +949,18 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {

// Get the span only for the name of the variable (I hope the path
// is only ever a variable name, but who knows?).
let sub_span = self.span.span_for_last_ident(p.span);
let sub_span = self.span.span_for_last_ident(sp);
// Rust uses the id of the pattern for var lookups, so we'll use it too.
if !self.span.filter_generated(sub_span, p.span) {
let qualname = format!("{}${}", path_to_string(p), id);
if !self.span.filter_generated(sub_span, sp) {
let qualname = format!("{}${}", i.to_string(), id);
let id = ::id_from_node_id(id, &self.save_ctxt);
let span = self.span_from_span(sub_span.expect("No span found for variable"));

self.dumper.dump_def(false, Def {
kind: DefKind::Local,
id,
span,
name: path_to_string(p),
name: i.to_string(),
qualname,
value: typ,
parent: None,
Expand Down Expand Up @@ -1263,7 +1288,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
for param in generics.ty_params.iter() {
for bound in param.bounds.iter() {
if let ast::TraitTyParamBound(ref trait_ref, _) = *bound {
self.process_trait_ref(&trait_ref.trait_ref);
self.process_path(trait_ref.trait_ref.ref_id, &trait_ref.trait_ref.path)
}
}
if let Some(ref ty) = param.default {
Expand Down Expand Up @@ -1318,7 +1343,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
let def = self.save_ctxt.get_path_def(hir_expr.id);
self.process_struct_lit(ex, path, fields, adt.variant_of_def(def), base)
}
ast::ExprKind::MethodCall(.., ref args) => self.process_method_call(ex, args),
ast::ExprKind::MethodCall(ref seg, ref args) => self.process_method_call(ex, seg, args),
ast::ExprKind::Field(ref sub_ex, _) => {
self.visit_expr(&sub_ex);

Expand Down Expand Up @@ -1390,15 +1415,15 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
let value = self.span.snippet(subexpression.span);
self.process_var_decl(pattern, value);
debug!("for loop, walk sub-expr: {:?}", subexpression.node);
visit::walk_expr(self, subexpression);
self.visit_expr(subexpression);
visit::walk_block(self, block);
}
ast::ExprKind::IfLet(ref pattern, ref subexpression, ref block, ref opt_else) => {
let value = self.span.snippet(subexpression.span);
self.process_var_decl(pattern, value);
visit::walk_expr(self, subexpression);
self.visit_expr(subexpression);
visit::walk_block(self, block);
opt_else.as_ref().map(|el| visit::walk_expr(self, el));
opt_else.as_ref().map(|el| self.visit_expr(el));
}
ast::ExprKind::Repeat(ref element, ref count) => {
self.visit_expr(element);
Expand Down Expand Up @@ -1430,15 +1455,12 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
self.visit_pat(&pattern);
}

// This is to get around borrow checking, because we need mut self to call process_path.
let mut paths_to_process = vec![];

// process collected paths
for &(id, ref p, immut) in &collector.collected_paths {
for (id, i, sp, immut) in collector.collected_idents {
match self.save_ctxt.get_path_def(id) {
HirDef::Local(id) => {
let mut value = if immut == ast::Mutability::Immutable {
self.span.snippet(p.span).to_string()
self.span.snippet(sp).to_string()
} else {
"<mutable>".to_string()
};
Expand All @@ -1451,18 +1473,16 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
value.push_str(": ");
value.push_str(&typ);

assert!(p.segments.len() == 1,
"qualified path for local variable def in arm");
if !self.span.filter_generated(Some(p.span), p.span) {
let qualname = format!("{}${}", path_to_string(p), id);
if !self.span.filter_generated(Some(sp), sp) {
let qualname = format!("{}${}", i.to_string(), id);
let id = ::id_from_node_id(id, &self.save_ctxt);
let span = self.span_from_span(p.span);
let span = self.span_from_span(sp);

self.dumper.dump_def(false, Def {
kind: DefKind::Local,
id,
span,
name: path_to_string(p),
name: i.to_string(),
qualname,
value: typ,
parent: None,
Expand All @@ -1474,19 +1494,12 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
});
}
}
HirDef::StructCtor(..) | HirDef::VariantCtor(..) |
HirDef::Const(..) | HirDef::AssociatedConst(..) |
HirDef::Struct(..) | HirDef::Variant(..) |
HirDef::TyAlias(..) | HirDef::AssociatedTy(..) |
HirDef::SelfTy(..) => {
paths_to_process.push((id, p.clone()))
}
def => error!("unexpected definition kind when processing collected paths: {:?}",
def => error!("unexpected definition kind when processing collected idents: {:?}",
def),
}
}

for &(id, ref path) in &paths_to_process {
for (id, ref path) in collector.collected_paths {
self.process_path(id, path);
}
walk_list!(self, visit_expr, &arm.guard);
Expand Down
70 changes: 52 additions & 18 deletions src/librustc_save_analysis/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,8 +579,8 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
Node::NodeItem(&hir::Item { node: hir::ItemUse(ref path, _), .. }) |
Node::NodeVisibility(&hir::Visibility::Restricted { ref path, .. }) => path.def,

Node::NodeExpr(&hir::Expr { node: hir::ExprPath(ref qpath), .. }) |
Node::NodeExpr(&hir::Expr { node: hir::ExprStruct(ref qpath, ..), .. }) |
Node::NodeExpr(&hir::Expr { node: hir::ExprPath(ref qpath), .. }) |
Node::NodePat(&hir::Pat { node: hir::PatKind::Path(ref qpath), .. }) |
Node::NodePat(&hir::Pat { node: hir::PatKind::Struct(ref qpath, ..), .. }) |
Node::NodePat(&hir::Pat { node: hir::PatKind::TupleStruct(ref qpath, ..), .. }) => {
Expand Down Expand Up @@ -614,6 +614,19 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
}

pub fn get_path_data(&self, id: NodeId, path: &ast::Path) -> Option<Ref> {
// Returns true if the path is function type sugar, e.g., `Fn(A) -> B`.
fn fn_type(path: &ast::Path) -> bool {
if path.segments.len() != 1 {
return false;
}
if let Some(ref params) = path.segments[0].parameters {
if let ast::PathParameters::Parenthesized(_) = **params {
return true;
}
}
false
}

let def = self.get_path_def(id);
let sub_span = self.span_utils.span_for_last_ident(path.span);
filter!(self.span_utils, sub_span, path.span, None);
Expand All @@ -630,7 +643,6 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
HirDef::Static(..) |
HirDef::Const(..) |
HirDef::AssociatedConst(..) |
HirDef::StructCtor(..) |
HirDef::VariantCtor(..) => {
let span = self.span_from_span(sub_span.unwrap());
Some(Ref {
Expand All @@ -639,6 +651,16 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
ref_id: id_from_def_id(def.def_id()),
})
}
HirDef::Trait(def_id) if fn_type(path) => {
// Function type bounds are desugared in the parser, so we have to
// special case them here.
let fn_span = self.span_utils.span_for_first_ident(path.span);
fn_span.map(|span| Ref {
kind: RefKind::Type,
span: self.span_from_span(span),
ref_id: id_from_def_id(def_id),
})
}
HirDef::Struct(def_id) |
HirDef::Variant(def_id, ..) |
HirDef::Union(def_id) |
Expand All @@ -655,6 +677,18 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
ref_id: id_from_def_id(def_id),
})
}
HirDef::StructCtor(def_id, _) => {
// This is a reference to a tuple struct where the def_id points
// to an invisible constructor function. That is not a very useful
// def, so adjust to point to the tuple struct itself.
let span = self.span_from_span(sub_span.unwrap());
let parent_def_id = self.tcx.parent_def_id(def_id).unwrap();
Some(Ref {
kind: RefKind::Type,
span,
ref_id: id_from_def_id(parent_def_id),
})
}
HirDef::Method(decl_id) => {
let sub_span = self.span_utils.sub_span_for_meth_name(path.span);
filter!(self.span_utils, sub_span, path.span, None);
Expand Down Expand Up @@ -818,29 +852,31 @@ fn make_signature(decl: &ast::FnDecl, generics: &ast::Generics) -> String {
sig
}

// An AST visitor for collecting paths from patterns.
struct PathCollector {
// The Row field identifies the kind of pattern.
collected_paths: Vec<(NodeId, ast::Path, ast::Mutability)>,
// An AST visitor for collecting paths (e.g., the names of structs) and formal
// variables (idents) from patterns.
struct PathCollector<'l> {
collected_paths: Vec<(NodeId, &'l ast::Path)>,
collected_idents: Vec<(NodeId, ast::Ident, Span, ast::Mutability)>,
}

impl PathCollector {
fn new() -> PathCollector {
PathCollector { collected_paths: vec![] }
impl<'l> PathCollector<'l> {
fn new() -> PathCollector<'l> {
PathCollector {
collected_paths: vec![],
collected_idents: vec![],
}
}
}

impl<'a> Visitor<'a> for PathCollector {
fn visit_pat(&mut self, p: &ast::Pat) {
impl<'l, 'a: 'l> Visitor<'a> for PathCollector<'l> {
fn visit_pat(&mut self, p: &'a ast::Pat) {
match p.node {
PatKind::Struct(ref path, ..) => {
self.collected_paths.push((p.id, path.clone(),
ast::Mutability::Mutable));
self.collected_paths.push((p.id, path));
}
PatKind::TupleStruct(ref path, ..) |
PatKind::Path(_, ref path) => {
self.collected_paths.push((p.id, path.clone(),
ast::Mutability::Mutable));
self.collected_paths.push((p.id, path));
}
PatKind::Ident(bm, ref path1, _) => {
debug!("PathCollector, visit ident in pat {}: {:?} {:?}",
Expand All @@ -854,9 +890,7 @@ impl<'a> Visitor<'a> for PathCollector {
ast::BindingMode::ByRef(_) => ast::Mutability::Immutable,
ast::BindingMode::ByValue(mt) => mt,
};
// collect path for either visit_local or visit_arm
let path = ast::Path::from_ident(path1.span, path1.node);
self.collected_paths.push((p.id, path, immut));
self.collected_idents.push((p.id, path1.node, path1.span, immut));
}
_ => {}
}
Expand Down