Auto merge of #45709 - nrc:rls-bugs-2, r=eddyb

Fix a bunch of minor save-analysis bugs

r? @eddyb
This commit is contained in:
bors 2017-11-04 15:30:20 +00:00
commit 98e4b6845f
2 changed files with 112 additions and 65 deletions

View File

@ -322,16 +322,15 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
let mut collector = PathCollector::new(); let mut collector = PathCollector::new();
collector.visit_pat(&arg.pat); collector.visit_pat(&arg.pat);
let span_utils = self.span.clone(); 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 hir_id = self.tcx.hir.node_to_hir_id(id);
let typ = match self.save_ctxt.tables.node_id_to_type_opt(hir_id) { let typ = match self.save_ctxt.tables.node_id_to_type_opt(hir_id) {
Some(s) => s.to_string(), Some(s) => s.to_string(),
None => continue, None => continue,
}; };
// get the span only for the name of the variable (I hope the path is only ever a let sub_span = span_utils.span_for_last_ident(sp);
// variable name, but who knows?) if !self.span.filter_generated(sub_span, sp) {
let sub_span = span_utils.span_for_last_ident(p.span);
if !self.span.filter_generated(sub_span, p.span) {
let id = ::id_from_node_id(id, &self.save_ctxt); let id = ::id_from_node_id(id, &self.save_ctxt);
let span = self.span_from_span(sub_span.expect("No span found for variable")); let span = self.span_from_span(sub_span.expect("No span found for variable"));
@ -339,8 +338,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
kind: DefKind::Local, kind: DefKind::Local,
id, id,
span, span,
name: path_to_string(p), name: i.to_string(),
qualname: format!("{}::{}", qualname, path_to_string(p)), qualname: format!("{}::{}", qualname, i.to_string()),
value: typ, value: typ,
parent: None, parent: None,
children: vec![], children: vec![],
@ -395,14 +394,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) { 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); let field_data = self.save_ctxt.get_field_data(field, parent_id);
if let Some(field_data) = field_data { if let Some(field_data) = field_data {
@ -792,7 +783,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); let path_data = self.save_ctxt.get_path_data(id, path);
if generated_code(path.span) && path_data.is_none() { if generated_code(path.span) && path_data.is_none() {
return; return;
@ -807,6 +799,27 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
self.dumper.dump_ref(path_data); 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. // Modules or types in the path prefix.
match self.save_ctxt.get_path_def(id) { match self.save_ctxt.get_path_def(id) {
HirDef::Method(did) => { HirDef::Method(did) => {
@ -859,7 +872,10 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
walk_list!(self, visit_expr, base); 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) { if let Some(mcd) = self.save_ctxt.get_expr_data(ex) {
down_cast_data!(mcd, RefData, ex.span); down_cast_data!(mcd, RefData, ex.span);
if !generated_code(ex.span) { if !generated_code(ex.span) {
@ -867,6 +883,15 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
} }
} }
// 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 receiver and args
walk_list!(self, visit_expr, args); walk_list!(self, visit_expr, args);
} }
@ -913,7 +938,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
collector.visit_pat(&p); collector.visit_pat(&p);
self.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 { let mut value = match immut {
ast::Mutability::Immutable => value.to_string(), ast::Mutability::Immutable => value.to_string(),
_ => String::new(), _ => String::new(),
@ -933,10 +958,10 @@ 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 // Get the span only for the name of the variable (I hope the path
// is only ever a variable name, but who knows?). // 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. // 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) { if !self.span.filter_generated(sub_span, sp) {
let qualname = format!("{}${}", path_to_string(p), id); let qualname = format!("{}${}", i.to_string(), id);
let id = ::id_from_node_id(id, &self.save_ctxt); let id = ::id_from_node_id(id, &self.save_ctxt);
let span = self.span_from_span(sub_span.expect("No span found for variable")); let span = self.span_from_span(sub_span.expect("No span found for variable"));
@ -944,7 +969,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
kind: DefKind::Local, kind: DefKind::Local,
id, id,
span, span,
name: path_to_string(p), name: i.to_string(),
qualname, qualname,
value: typ, value: typ,
parent: None, parent: None,
@ -1274,7 +1299,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
for param in generics.ty_params.iter() { for param in generics.ty_params.iter() {
for bound in param.bounds.iter() { for bound in param.bounds.iter() {
if let ast::TraitTyParamBound(ref trait_ref, _) = *bound { 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 { if let Some(ref ty) = param.default {
@ -1329,7 +1354,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); let def = self.save_ctxt.get_path_def(hir_expr.id);
self.process_struct_lit(ex, path, fields, adt.variant_of_def(def), base) 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, _) => { ast::ExprKind::Field(ref sub_ex, _) => {
self.visit_expr(&sub_ex); self.visit_expr(&sub_ex);
@ -1401,15 +1426,15 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
let value = self.span.snippet(subexpression.span); let value = self.span.snippet(subexpression.span);
self.process_var_decl(pattern, value); self.process_var_decl(pattern, value);
debug!("for loop, walk sub-expr: {:?}", subexpression.node); debug!("for loop, walk sub-expr: {:?}", subexpression.node);
visit::walk_expr(self, subexpression); self.visit_expr(subexpression);
visit::walk_block(self, block); visit::walk_block(self, block);
} }
ast::ExprKind::IfLet(ref pattern, ref subexpression, ref block, ref opt_else) => { ast::ExprKind::IfLet(ref pattern, ref subexpression, ref block, ref opt_else) => {
let value = self.span.snippet(subexpression.span); let value = self.span.snippet(subexpression.span);
self.process_var_decl(pattern, value); self.process_var_decl(pattern, value);
visit::walk_expr(self, subexpression); self.visit_expr(subexpression);
visit::walk_block(self, block); 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) => { ast::ExprKind::Repeat(ref element, ref count) => {
self.visit_expr(element); self.visit_expr(element);
@ -1441,15 +1466,12 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
self.visit_pat(&pattern); 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 // 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) { match self.save_ctxt.get_path_def(id) {
HirDef::Local(id) => { HirDef::Local(id) => {
let mut value = if immut == ast::Mutability::Immutable { let mut value = if immut == ast::Mutability::Immutable {
self.span.snippet(p.span).to_string() self.span.snippet(sp).to_string()
} else { } else {
"<mutable>".to_string() "<mutable>".to_string()
}; };
@ -1462,18 +1484,16 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
value.push_str(": "); value.push_str(": ");
value.push_str(&typ); value.push_str(&typ);
assert!(p.segments.len() == 1, if !self.span.filter_generated(Some(sp), sp) {
"qualified path for local variable def in arm"); let qualname = format!("{}${}", i.to_string(), id);
if !self.span.filter_generated(Some(p.span), p.span) {
let qualname = format!("{}${}", path_to_string(p), id);
let id = ::id_from_node_id(id, &self.save_ctxt); 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 { self.dumper.dump_def(false, Def {
kind: DefKind::Local, kind: DefKind::Local,
id, id,
span, span,
name: path_to_string(p), name: i.to_string(),
qualname, qualname,
value: typ, value: typ,
parent: None, parent: None,
@ -1485,19 +1505,12 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
}); });
} }
} }
HirDef::StructCtor(..) | HirDef::VariantCtor(..) | def => error!("unexpected definition kind when processing collected idents: {:?}",
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), def),
} }
} }
for &(id, ref path) in &paths_to_process { for (id, ref path) in collector.collected_paths {
self.process_path(id, path); self.process_path(id, path);
} }
walk_list!(self, visit_expr, &arm.guard); walk_list!(self, visit_expr, &arm.guard);

View File

@ -579,8 +579,8 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
Node::NodeItem(&hir::Item { node: hir::ItemUse(ref path, _), .. }) | Node::NodeItem(&hir::Item { node: hir::ItemUse(ref path, _), .. }) |
Node::NodeVisibility(&hir::Visibility::Restricted { ref path, .. }) => path.def, 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::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::Path(ref qpath), .. }) |
Node::NodePat(&hir::Pat { node: hir::PatKind::Struct(ref qpath, ..), .. }) | Node::NodePat(&hir::Pat { node: hir::PatKind::Struct(ref qpath, ..), .. }) |
Node::NodePat(&hir::Pat { node: hir::PatKind::TupleStruct(ref qpath, ..), .. }) => { Node::NodePat(&hir::Pat { node: hir::PatKind::TupleStruct(ref qpath, ..), .. }) => {
@ -614,6 +614,19 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
} }
pub fn get_path_data(&self, id: NodeId, path: &ast::Path) -> Option<Ref> { 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 def = self.get_path_def(id);
let sub_span = self.span_utils.span_for_last_ident(path.span); let sub_span = self.span_utils.span_for_last_ident(path.span);
filter!(self.span_utils, sub_span, path.span, None); filter!(self.span_utils, sub_span, path.span, None);
@ -630,7 +643,6 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
HirDef::Static(..) | HirDef::Static(..) |
HirDef::Const(..) | HirDef::Const(..) |
HirDef::AssociatedConst(..) | HirDef::AssociatedConst(..) |
HirDef::StructCtor(..) |
HirDef::VariantCtor(..) => { HirDef::VariantCtor(..) => {
let span = self.span_from_span(sub_span.unwrap()); let span = self.span_from_span(sub_span.unwrap());
Some(Ref { Some(Ref {
@ -639,6 +651,16 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
ref_id: id_from_def_id(def.def_id()), 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::Struct(def_id) |
HirDef::Variant(def_id, ..) | HirDef::Variant(def_id, ..) |
HirDef::Union(def_id) | HirDef::Union(def_id) |
@ -655,6 +677,18 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
ref_id: id_from_def_id(def_id), 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) => { HirDef::Method(decl_id) => {
let sub_span = self.span_utils.sub_span_for_meth_name(path.span); let sub_span = self.span_utils.sub_span_for_meth_name(path.span);
filter!(self.span_utils, sub_span, path.span, None); filter!(self.span_utils, sub_span, path.span, None);
@ -818,29 +852,31 @@ fn make_signature(decl: &ast::FnDecl, generics: &ast::Generics) -> String {
sig sig
} }
// An AST visitor for collecting paths from patterns. // An AST visitor for collecting paths (e.g., the names of structs) and formal
struct PathCollector { // variables (idents) from patterns.
// The Row field identifies the kind of pattern. struct PathCollector<'l> {
collected_paths: Vec<(NodeId, ast::Path, ast::Mutability)>, collected_paths: Vec<(NodeId, &'l ast::Path)>,
collected_idents: Vec<(NodeId, ast::Ident, Span, ast::Mutability)>,
} }
impl PathCollector { impl<'l> PathCollector<'l> {
fn new() -> PathCollector { fn new() -> PathCollector<'l> {
PathCollector { collected_paths: vec![] } PathCollector {
collected_paths: vec![],
collected_idents: vec![],
}
} }
} }
impl<'a> Visitor<'a> for PathCollector { impl<'l, 'a: 'l> Visitor<'a> for PathCollector<'l> {
fn visit_pat(&mut self, p: &ast::Pat) { fn visit_pat(&mut self, p: &'a ast::Pat) {
match p.node { match p.node {
PatKind::Struct(ref path, ..) => { PatKind::Struct(ref path, ..) => {
self.collected_paths.push((p.id, path.clone(), self.collected_paths.push((p.id, path));
ast::Mutability::Mutable));
} }
PatKind::TupleStruct(ref path, ..) | PatKind::TupleStruct(ref path, ..) |
PatKind::Path(_, ref path) => { PatKind::Path(_, ref path) => {
self.collected_paths.push((p.id, path.clone(), self.collected_paths.push((p.id, path));
ast::Mutability::Mutable));
} }
PatKind::Ident(bm, ref path1, _) => { PatKind::Ident(bm, ref path1, _) => {
debug!("PathCollector, visit ident in pat {}: {:?} {:?}", debug!("PathCollector, visit ident in pat {}: {:?} {:?}",
@ -854,9 +890,7 @@ impl<'a> Visitor<'a> for PathCollector {
ast::BindingMode::ByRef(_) => ast::Mutability::Immutable, ast::BindingMode::ByRef(_) => ast::Mutability::Immutable,
ast::BindingMode::ByValue(mt) => mt, ast::BindingMode::ByValue(mt) => mt,
}; };
// collect path for either visit_local or visit_arm self.collected_idents.push((p.id, path1.node, path1.span, immut));
let path = ast::Path::from_ident(path1.span, path1.node);
self.collected_paths.push((p.id, path, immut));
} }
_ => {} _ => {}
} }