Auto merge of #55521 - nrc:rls-fix, r=petrochenkov

save-analysis: bug fix and optimisation.

The first commit fixes a bug in name resolution and save-analysis (introduced in #54145) and removes an unused parameter. This fixes the RLS tests, which are currently blocking distribution of the RLS. The second commit removes macro uses from save-analysis data, since these are never used, they just take up space.

r? @petrochenkov

4 files changed, 33 insertions, 31 deletions

diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
index c3c65816b26..b6621e0962c 100644
--- a/src/librustc/hir/lowering.rs
+++ b/src/librustc/hir/lowering.rs
@@ -1754,7 +1754,6 @@ impl<'a> LoweringContext<'a> {
         &mut self,
         def: Def,
         p: &Path,
-        ident: Option<Ident>,
         param_mode: ParamMode,
         explicit_owner: Option<NodeId>,
     ) -> hir::Path {
@@ -1773,7 +1772,6 @@ impl<'a> LoweringContext<'a> {
                         explicit_owner,
                     )
                 })
-                .chain(ident.map(|ident| hir::PathSegment::from_ident(ident)))
                 .collect(),
             span: p.span,
         }
@@ -1781,7 +1779,7 @@ impl<'a> LoweringContext<'a> {
 
     fn lower_path(&mut self, id: NodeId, p: &Path, param_mode: ParamMode) -> hir::Path {
         let def = self.expect_full_def(id);
-        self.lower_path_extra(def, p, None, param_mode, None)
+        self.lower_path_extra(def, p, param_mode, None)
     }
 
     fn lower_path_segment(
@@ -3014,7 +3012,7 @@ impl<'a> LoweringContext<'a> {
                     self.with_hir_id_owner(new_node_id, |this| {
                         let new_id = this.lower_node_id(new_node_id);
                         let path =
-                            this.lower_path_extra(def, &path, None, ParamMode::Explicit, None);
+                            this.lower_path_extra(def, &path, ParamMode::Explicit, None);
                         let item = hir::ItemKind::Use(P(path), hir::UseKind::Single);
                         let vis_kind = match vis.node {
                             hir::VisibilityKind::Public => hir::VisibilityKind::Public,
@@ -3053,7 +3051,7 @@ impl<'a> LoweringContext<'a> {
                 }
 
                 let path =
-                    P(self.lower_path_extra(ret_def, &path, None, ParamMode::Explicit, None));
+                    P(self.lower_path_extra(ret_def, &path, ParamMode::Explicit, None));
                 hir::ItemKind::Use(path, hir::UseKind::Single)
             }
             UseTreeKind::Glob => {
@@ -3140,7 +3138,7 @@ impl<'a> LoweringContext<'a> {
                 // the stability of `use a::{};`, to avoid it showing up as
                 // a re-export by accident when `pub`, e.g. in documentation.
                 let def = self.expect_full_def_from_use(id).next().unwrap_or(Def::Err);
-                let path = P(self.lower_path_extra(def, &prefix, None, ParamMode::Explicit, None));
+                let path = P(self.lower_path_extra(def, &prefix, ParamMode::Explicit, None));
                 *vis = respan(prefix.span.shrink_to_lo(), hir::VisibilityKind::Inherited);
                 hir::ItemKind::Use(path, hir::UseKind::ListStem)
             }
@@ -4550,7 +4548,6 @@ impl<'a> LoweringContext<'a> {
                     path: P(self.lower_path_extra(
                         def,
                         path,
-                        None,
                         ParamMode::Explicit,
                         explicit_owner,
                     )),
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index ebd87e87ff6..094488f3af3 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -3589,7 +3589,17 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         );
 
         for (i, &Segment { ident, id }) in path.iter().enumerate() {
-            debug!("resolve_path ident {} {:?}", i, ident);
+            debug!("resolve_path ident {} {:?} {:?}", i, ident, id);
+            let record_segment_def = |this: &mut Self, def| {
+                if record_used {
+                    if let Some(id) = id {
+                        if !this.def_map.contains_key(&id) {
+                            assert!(id != ast::DUMMY_NODE_ID, "Trying to resolve dummy id");
+                            this.record_def(id, PathResolution::new(def));
+                        }
+                    }
+                }
+            };
 
             let is_last = i == path.len() - 1;
             let ns = if is_last { opt_ns.unwrap_or(TypeNS) } else { TypeNS };
@@ -3673,6 +3683,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                     // we found a local variable or type param
                     Some(LexicalScopeBinding::Def(def))
                             if opt_ns == Some(TypeNS) || opt_ns == Some(ValueNS) => {
+                        record_segment_def(self, def);
                         return PathResult::NonModule(PathResolution::with_unresolved_segments(
                             def, path.len() - 1
                         ));
@@ -3690,14 +3701,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                     let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(def);
                     if let Some(next_module) = binding.module() {
                         module = Some(ModuleOrUniformRoot::Module(next_module));
-                        if record_used {
-                            if let Some(id) = id {
-                                if !self.def_map.contains_key(&id) {
-                                    assert!(id != ast::DUMMY_NODE_ID, "Trying to resolve dummy id");
-                                    self.record_def(id, PathResolution::new(def));
-                                }
-                            }
-                        }
+                        record_segment_def(self, def);
                     } else if def == Def::ToolMod && i + 1 != path.len() {
                         let def = Def::NonMacroAttr(NonMacroAttrKind::Tool);
                         return PathResult::NonModule(PathResolution::new(def));
diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs
index c0b718e4863..9bc3fbe7c24 100644
--- a/src/librustc_save_analysis/dump_visitor.rs
+++ b/src/librustc_save_analysis/dump_visitor.rs
@@ -92,7 +92,7 @@ pub struct DumpVisitor<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> {
     // we only write one macro def per unique macro definition, and
     // one macro use per unique callsite span.
     // mac_defs: FxHashSet<Span>,
-    macro_calls: FxHashSet<Span>,
+    // macro_calls: FxHashSet<Span>,
 }
 
 impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
@@ -108,7 +108,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
             span: span_utils,
             cur_scope: CRATE_NODE_ID,
             // mac_defs: FxHashSet::default(),
-            macro_calls: FxHashSet::default(),
+            // macro_calls: FxHashSet::default(),
         }
     }
 
@@ -771,8 +771,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);
-        if generated_code(path.span) {
+        if self.span.filter_generated(path.span) {
             return;
         }
         self.dump_path_ref(id, path);
@@ -1031,18 +1030,20 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
     /// If the span is not macro-generated, do nothing, else use callee and
     /// callsite spans to record macro definition and use data, using the
     /// mac_uses and mac_defs sets to prevent multiples.
-    fn process_macro_use(&mut self, span: Span) {
-        let source_span = span.source_callsite();
-        if !self.macro_calls.insert(source_span) {
-            return;
-        }
+    fn process_macro_use(&mut self, _span: Span) {
+        // FIXME if we're not dumping the defs (see below), there is no point
+        // dumping refs either.
+        // let source_span = span.source_callsite();
+        // if !self.macro_calls.insert(source_span) {
+        //     return;
+        // }
 
-        let data = match self.save_ctxt.get_macro_use_data(span) {
-            None => return,
-            Some(data) => data,
-        };
+        // let data = match self.save_ctxt.get_macro_use_data(span) {
+        //     None => return,
+        //     Some(data) => data,
+        // };
 
-        self.dumper.macro_use(data);
+        // self.dumper.macro_use(data);
 
         // FIXME write the macro def
         // let mut hasher = DefaultHasher::new();
diff --git a/src/librustc_save_analysis/json_dumper.rs b/src/librustc_save_analysis/json_dumper.rs
index e14ac73ee10..d2354f38e26 100644
--- a/src/librustc_save_analysis/json_dumper.rs
+++ b/src/librustc_save_analysis/json_dumper.rs
@@ -93,7 +93,7 @@ impl<'b, O: DumpOutput + 'b> JsonDumper<O> {
         self.result.compilation = Some(data);
     }
 
-    pub fn macro_use(&mut self, data: MacroRef) {
+    pub fn _macro_use(&mut self, data: MacroRef) {
         if self.config.pub_only || self.config.reachable_only {
             return;
         }