Auto merge of #43533 - nrc:macro-save, r=jseyfried,

Three small fixes for save-analysis

First commit does some naive deduplication of macro uses. We end up with lots of duplication here because of the weird way we get this data (we extract a use for every span generated by a macro use).

Second commit is basically a typo fix.

Third commit is a bit interesting, it partially reverts a change from #40939 where temporary variables in format! (and thus println!) got a span with the primary pointing at the value stored into the temporary (e.g., `x` in `println!("...", x)`). If `format!` had a definition it should point at the temporary in the macro def, but since it is built-in, that is not possible (for now), so `DUMMY_SP` is the best we can do (using the span in the callee really breaks save-analysis because it thinks `x` is a definition as well as a reference).

There aren't a test for this stuff because: the deduplication is filtered by any of the users of save-analysis, so it is purely an efficiency change. I couldn't actually find an example for the second commit that we have any machinery to test, and the third commit is tested by the RLS, so there will be a test once I update the RLS version and and uncomment the previously failing tests).

r? @jseyfried

src/Cargo.lock

@@ -1508,6 +1508,7 @@ dependencies = [
  "rls-span 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc 0.0.0",
  "rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rustc_data_structures 0.0.0",
  "rustc_typeck 0.0.0",
  "syntax 0.0.0",
  "syntax_pos 0.0.0",

src/librustc_save_analysis/Cargo.toml

@@ -11,6 +11,7 @@ crate-type = ["dylib"]
 [dependencies]
 log = "0.3"
 rustc = { path = "../librustc" }
+rustc_data_structures = { path = "../librustc_data_structures" }
 rustc_typeck = { path = "../librustc_typeck" }
 syntax = { path = "../libsyntax" }
 syntax_pos = { path = "../libsyntax_pos" }

src/librustc_save_analysis/dump_visitor.rs

@@ -29,6 +29,7 @@ use rustc::hir::def_id::DefId;
 use rustc::hir::map::Node;
 use rustc::session::Session;
 use rustc::ty::{self, TyCtxt};
+use rustc_data_structures::fx::FxHashSet;
 
 use std::path::Path;
 
@@ -74,6 +75,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: HashSet<Span>,
+    macro_calls: FxHashSet<Span>,
 }
 
 impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
@@ -89,6 +91,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
             span: span_utils.clone(),
             cur_scope: CRATE_NODE_ID,
             // mac_defs: HashSet::new(),
+            macro_calls: FxHashSet(),
         }
     }
 
@@ -972,11 +975,19 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
     /// 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.contains(&source_span) {
+            return;
+        }
+        self.macro_calls.insert(source_span);
+
         let data = match self.save_ctxt.get_macro_use_data(span) {
             None => return,
             Some(data) => data,
         };
 
+        self.dumper.macro_use(data);
+
         // FIXME write the macro def
         // let mut hasher = DefaultHasher::new();
         // data.callee_span.hash(&mut hasher);
@@ -996,7 +1007,6 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
         //         }.lower(self.tcx));
         //     }
         // }
-        self.dumper.macro_use(data);
     }
 
     fn process_trait_item(&mut self, trait_item: &'l ast::TraitItem, trait_id: DefId) {

src/librustc_save_analysis/lib.rs

@@ -23,6 +23,7 @@
 
 #[macro_use] extern crate log;
 #[macro_use] extern crate syntax;
+extern crate rustc_data_structures;
 extern crate rustc_serialize;
 extern crate rustc_typeck;
 extern crate syntax_pos;

src/librustc_save_analysis/span_utils.rs

@@ -398,9 +398,10 @@ impl<'a> SpanUtils<'a> {
             return false;
         }
         // If sub_span is none, filter out generated code.
-        if sub_span.is_none() {
-            return true;
-        }
+        let sub_span = match sub_span {
+            Some(ss) => ss,
+            None => return true,
+        };
 
         //If the span comes from a fake filemap, filter it.
         if !self.sess.codemap().lookup_char_pos(parent.lo).file.is_real_file() {
@@ -409,7 +410,7 @@ impl<'a> SpanUtils<'a> {
 
         // Otherwise, a generated span is deemed invalid if it is not a sub-span of the root
         // callsite. This filters out macro internal variables and most malformed spans.
-        !parent.source_callsite().contains(parent)
+        !parent.source_callsite().contains(sub_span)
     }
 }
 

src/libsyntax_ext/format.rs

@@ -20,7 +20,7 @@ use syntax::ext::build::AstBuilder;
 use syntax::parse::token;
 use syntax::ptr::P;
 use syntax::symbol::{Symbol, keywords};
-use syntax_pos::Span;
+use syntax_pos::{Span, DUMMY_SP};
 use syntax::tokenstream;
 
 use std::collections::{HashMap, HashSet};
@@ -558,8 +558,10 @@ impl<'a, 'b> Context<'a, 'b> {
         // passed to this function.
         for (i, e) in self.args.into_iter().enumerate() {
             let name = self.ecx.ident_of(&format!("__arg{}", i));
-            let span =
-                Span { ctxt: e.span.ctxt.apply_mark(self.ecx.current_expansion.mark), ..e.span };
+            let span = Span {
+                ctxt: e.span.ctxt.apply_mark(self.ecx.current_expansion.mark),
+                ..DUMMY_SP
+            };
             pats.push(self.ecx.pat_ident(span, name));
             for ref arg_ty in self.arg_unique_types[i].iter() {
                 locals.push(Context::format_arg(self.ecx, self.macsp, e.span, arg_ty, name));