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

Fix a bunch of minor save-analysis bugs

r? @eddyb

2 files changed, 112 insertions, 65 deletions

diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs
index 41288c88ede..23f6e6b46ba 100644
--- a/src/librustc_save_analysis/dump_visitor.rs
+++ b/src/librustc_save_analysis/dump_visitor.rs
@@ -322,16 +322,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"));
 
@@ -339,8 +338,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![],
@@ -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) {
         let field_data = self.save_ctxt.get_field_data(field, parent_id);
         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);
         if generated_code(path.span) && path_data.is_none() {
             return;
@@ -807,6 +799,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) => {
@@ -859,7 +872,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<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) {
@@ -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_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);
         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(),
@@ -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
             // 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"));
 
@@ -944,7 +969,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,
@@ -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 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 {
@@ -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);
                 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);
 
@@ -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);
                 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);
@@ -1441,15 +1466,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()
                     };
@@ -1462,18 +1484,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,
@@ -1485,19 +1505,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 f98a89781f7..5ae1ba92262 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, ..), .. }) => {
@@ -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);
@@ -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 {
@@ -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) |
@@ -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);
@@ -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 {}: {:?} {:?}",
@@ -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));
             }
             _ => {}
         }