From c1d3e441a8c6d24bc61d3deb22a0909401767ada Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Wed, 1 Nov 2017 17:59:06 +1300 Subject: [PATCH 1/4] save-analysis: handle function types in bounds This special cases the function type sugar in paths and deals with traits bounds as just the path parts. That required refactoring the path collector to distinguish between variable decls and references in patterns, basically just to please the borrow checker. cc https://github.com/nrc/rls-analysis/issues/37 --- src/librustc_save_analysis/dump_visitor.rs | 84 +++++++++++----------- src/librustc_save_analysis/lib.rs | 55 +++++++++----- 2 files changed, 81 insertions(+), 58 deletions(-) diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 4eac4398c1827..11f62b4bd804e 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -318,16 +318,15 @@ 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")); @@ -335,8 +334,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> { 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![], @@ -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 { @@ -783,7 +774,7 @@ 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) { let path_data = self.save_ctxt.get_path_data(id, path); if generated_code(path.span) && path_data.is_none() { return; @@ -798,6 +789,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) => { @@ -904,7 +916,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(), @@ -924,10 +936,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 // 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")); @@ -935,7 +947,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> { kind: DefKind::Local, id, span, - name: path_to_string(p), + name: i.to_string(), qualname, value: typ, parent: None, @@ -1263,7 +1275,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 { @@ -1430,15 +1442,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 { "".to_string() }; @@ -1451,18 +1460,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, @@ -1474,19 +1481,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); diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index cf2cad1b38c42..9769f3905c7b6 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -614,6 +614,19 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> { } pub fn get_path_data(&self, id: NodeId, path: &ast::Path) -> Option { + // 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); @@ -639,6 +652,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) | @@ -818,29 +841,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 {}: {:?} {:?}", @@ -854,9 +879,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)); } _ => {} } From 82a8968ce09f1fd15291f2b3e51a89178a6d0674 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Thu, 2 Nov 2017 09:22:44 +1300 Subject: [PATCH 2/4] save-analysis: handle types in turbofish fixes https://github.com/nrc/rls-analysis/issues/52 --- src/librustc_save_analysis/dump_visitor.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 11f62b4bd804e..9a91feb8ebb3a 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -775,6 +775,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> { } 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; @@ -862,7 +863,10 @@ 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]) { + fn process_method_call(&mut self, + ex: &'l ast::Expr, + seg: &'l ast::PathSegment, + args: &'l [P]) { if let Some(mcd) = self.save_ctxt.get_expr_data(ex) { down_cast_data!(mcd, RefData, ex.span); if !generated_code(ex.span) { @@ -870,6 +874,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_list!(self, visit_expr, args); } @@ -1330,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); From 20c64e8155b0c29e8cbb4b9a8c99fd0a94a3caa2 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Thu, 2 Nov 2017 11:28:13 +1300 Subject: [PATCH 3/4] save-analysis: corrects reference for tuple struct and unit struct literals Fixes https://github.com/nrc/rls-analysis/issues/77 --- src/librustc_save_analysis/lib.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index 9769f3905c7b6..63dde75678719 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -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, ..), .. }) => { @@ -643,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 { @@ -678,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); From 5d3be12a4d43b73c79e81adc964766a706bb5a07 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Thu, 2 Nov 2017 14:35:39 +1300 Subject: [PATCH 4/4] save-analysis: fix issue with sub-exprs in for loops Fixes https://github.com/nrc/rls-analysis/issues/78 --- src/librustc_save_analysis/dump_visitor.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 9a91feb8ebb3a..8b6f7eae9c722 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -1415,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);