Auto merge of #43826 - kennytm:fix-43796-mis-calculated-spans, r=petrochenkov

Fix "Mis-calculated spans" errors from `-Z save-analysis` + refactoring

Removed the path span extraction methods from `SpanUtils`:

* spans_with_brackets
* spans_for_path_segments
* spans_for_ty_params

Use the `span` fields in `PathSegment` and `TyParam` instead.

(Note that since it processes `ast::Path` not a qualified path (`hir::QPath` / `ast::QSelf`), UFCS path will be flattened: `<Foo as a::b::c::Trait>::D::E::F::g` will be seen as `a::b::c::Trait::D::E::F::g`.)

Fix #43796. Close #41478.

r? @nrc

4 files changed, 33 insertions, 123 deletions

diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs
index f89d59b5a44..658b6dbc4e3 100644
--- a/src/librustc_save_analysis/dump_visitor.rs
+++ b/src/librustc_save_analysis/dump_visitor.rs
@@ -27,7 +27,6 @@
 use rustc::hir::def::Def as HirDef;
 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;
 
@@ -62,7 +61,6 @@ macro_rules! down_cast_data {
 
 pub struct DumpVisitor<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> {
     save_ctxt: SaveContext<'l, 'tcx>,
-    sess: &'l Session,
     tcx: TyCtxt<'l, 'tcx, 'tcx>,
     dumper: &'ll mut JsonDumper<O>,
 
@@ -84,7 +82,6 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
                -> DumpVisitor<'l, 'tcx, 'll, O> {
         let span_utils = SpanUtils::new(&save_ctxt.tcx.sess);
         DumpVisitor {
-            sess: &save_ctxt.tcx.sess,
             tcx: save_ctxt.tcx,
             save_ctxt: save_ctxt,
             dumper: dumper,
@@ -147,39 +144,15 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
     // For each prefix, we return the span for the last segment in the prefix and
     // a str representation of the entire prefix.
     fn process_path_prefixes(&self, path: &ast::Path) -> Vec<(Span, String)> {
-        let spans = self.span.spans_for_path_segments(path);
         let segments = &path.segments[if path.is_global() { 1 } else { 0 }..];
 
-        // Paths to enums seem to not match their spans - the span includes all the
-        // variants too. But they seem to always be at the end, so I hope we can cope with
-        // always using the first ones. So, only error out if we don't have enough spans.
-        // What could go wrong...?
-        if spans.len() < segments.len() {
-            if generated_code(path.span) {
-                return vec![];
-            }
-            error!("Mis-calculated spans for path '{}'. Found {} spans, expected {}. Found spans:",
-                   path_to_string(path),
-                   spans.len(),
-                   segments.len());
-            for s in &spans {
-                let loc = self.sess.codemap().lookup_char_pos(s.lo);
-                error!("    '{}' in {}, line {}",
-                       self.span.snippet(*s),
-                       loc.file.name,
-                       loc.line);
-            }
-            error!("    master span: {:?}: `{}`", path.span, self.span.snippet(path.span));
-            return vec![];
-        }
-
-        let mut result: Vec<(Span, String)> = vec![];
+        let mut result = Vec::with_capacity(segments.len());
 
         let mut segs = vec![];
-        for (i, (seg, span)) in segments.iter().zip(&spans).enumerate() {
+        for (i, seg) in segments.iter().enumerate() {
             segs.push(seg.clone());
             let sub_path = ast::Path {
-                span: *span, // span for the last segment
+                span: seg.span, // span for the last segment
                 segments: segs,
             };
             let qualname = if i == 0 && path.is_global() {
@@ -187,7 +160,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
             } else {
                 path_to_string(&sub_path)
             };
-            result.push((*span, qualname));
+            result.push((seg.span, qualname));
             segs = sub_path.segments;
         }
 
@@ -437,13 +410,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
                               full_span: Span,
                               prefix: &str,
                               id: NodeId) {
-        // We can't only use visit_generics since we don't have spans for param
-        // bindings, so we reparse the full_span to get those sub spans.
-        // However full span is the entire enum/fn/struct block, so we only want
-        // the first few to match the number of generics we're looking for.
-        let param_sub_spans = self.span.spans_for_ty_params(full_span,
-                                                            (generics.ty_params.len() as isize));
-        for (param, param_ss) in generics.ty_params.iter().zip(param_sub_spans) {
+        for param in &generics.ty_params {
+            let param_ss = param.span;
             let name = escape(self.span.snippet(param_ss));
             // Append $id to name to make sure each one is unique
             let qualname = format!("{}::{}${}",
diff --git a/src/librustc_save_analysis/span_utils.rs b/src/librustc_save_analysis/span_utils.rs
index e771da2ed4c..631907b28f8 100644
--- a/src/librustc_save_analysis/span_utils.rs
+++ b/src/librustc_save_analysis/span_utils.rs
@@ -16,7 +16,6 @@ use std::cell::Cell;
 use std::env;
 use std::path::Path;
 
-use syntax::ast;
 use syntax::parse::lexer::{self, StringReader};
 use syntax::parse::token::{self, Token};
 use syntax::symbol::keywords;
@@ -207,75 +206,6 @@ impl<'a> SpanUtils<'a> {
         result
     }
 
-    // Reparse span and return an owned vector of sub spans of the first limit
-    // identifier tokens in the given nesting level.
-    // example with Foo<Bar<T,V>, Bar<T,V>>
-    // Nesting = 0: all idents outside of angle brackets: [Foo]
-    // Nesting = 1: idents within one level of angle brackets: [Bar, Bar]
-    pub fn spans_with_brackets(&self, span: Span, nesting: isize, limit: isize) -> Vec<Span> {
-        let mut result: Vec<Span> = vec![];
-
-        let mut toks = self.retokenise_span(span);
-        // We keep track of how many brackets we're nested in
-        let mut angle_count: isize = 0;
-        let mut bracket_count: isize = 0;
-        let mut found_ufcs_sep = false;
-        loop {
-            let ts = toks.real_token();
-            if ts.tok == token::Eof {
-                if angle_count != 0 || bracket_count != 0 {
-                    if generated_code(span) {
-                        return vec![];
-                    }
-                    let loc = self.sess.codemap().lookup_char_pos(span.lo);
-                    span_bug!(span,
-                              "Mis-counted brackets when breaking path? \
-                               Parsing '{}' in {}, line {}",
-                              self.snippet(span),
-                              loc.file.name,
-                              loc.line);
-                }
-                return result
-            }
-            if (result.len() as isize) == limit {
-                return result;
-            }
-            bracket_count += match ts.tok {
-                token::OpenDelim(token::Bracket) => 1,
-                token::CloseDelim(token::Bracket) => -1,
-                _ => 0,
-            };
-            if bracket_count > 0 {
-                continue;
-            }
-            angle_count += match ts.tok {
-                token::Lt => 1,
-                token::Gt => -1,
-                token::BinOp(token::Shl) => 2,
-                token::BinOp(token::Shr) => -2,
-                _ => 0,
-            };
-
-            // Ignore the `>::` in `<Type as Trait>::AssocTy`.
-
-            // The root cause of this hack is that the AST representation of
-            // qpaths is horrible. It treats <A as B>::C as a path with two
-            // segments, B and C and notes that there is also a self type A at
-            // position 0. Because we don't have spans for individual idents,
-            // only the whole path, we have to iterate over the tokens in the
-            // path, trying to pull out the non-nested idents (e.g., avoiding 'a
-            // in `<A as B<'a>>::C`). So we end up with a span for `B>::C` from
-            // the start of the first ident to the end of the path.
-            if !found_ufcs_sep && angle_count == -1 {
-                found_ufcs_sep = true;
-                angle_count += 1;
-            }
-            if ts.tok.is_ident() && angle_count == nesting {
-                result.push(ts.sp);
-            }
-        }
-    }
-
     pub fn sub_span_before_token(&self, span: Span, tok: Token) -> Option<Span> {
         let mut toks = self.retokenise_span(span);
         let mut prev = toks.real_token();
@@ -330,21 +260,6 @@ impl<'a> SpanUtils<'a> {
         }
     }
 
-
-    // Returns a list of the spans of idents in a path.
-    // E.g., For foo::bar<x,t>::baz, we return [foo, bar, baz] (well, their spans)
-    pub fn spans_for_path_segments(&self, path: &ast::Path) -> Vec<Span> {
-        self.spans_with_brackets(path.span, 0, -1)
-    }
-
-    // Return an owned vector of the subspans of the param identifier
-    // tokens found in span.
-    pub fn spans_for_ty_params(&self, span: Span, number: isize) -> Vec<Span> {
-        // Type params are nested within one level of brackets:
-        // i.e. we want Vec<A, B> from Foo<A, B<T,U>>
-        self.spans_with_brackets(span, 1, number)
-    }
-
     // // Return the name for a macro definition (identifier after first `!`)
     // pub fn span_for_macro_def_name(&self, span: Span) -> Option<Span> {
     //     let mut toks = self.retokenise_span(span);
diff --git a/src/test/run-make/issues-41478-43796/Makefile b/src/test/run-make/issues-41478-43796/Makefile
new file mode 100644
index 00000000000..f9735253ab6
--- /dev/null
+++ b/src/test/run-make/issues-41478-43796/Makefile
@@ -0,0 +1,8 @@
+-include ../tools.mk
+
+all:
+	# Work in /tmp, because we need to create the `save-analysis-temp` folder.
+	cp a.rs $(TMPDIR)/
+	cd $(TMPDIR) && $(RUSTC) -Zsave-analysis $(TMPDIR)/a.rs 2> $(TMPDIR)/stderr.txt || ( cat $(TMPDIR)/stderr.txt && exit 1 )
+	[ ! -s $(TMPDIR)/stderr.txt ] || ( cat $(TMPDIR)/stderr.txt && exit 1 )
+	[ -f $(TMPDIR)/save-analysis/liba.json ] || ( ls -la $(TMPDIR) && exit 1 )
diff --git a/src/test/run-make/issues-41478-43796/a.rs b/src/test/run-make/issues-41478-43796/a.rs
new file mode 100644
index 00000000000..9d95f8b2585
--- /dev/null
+++ b/src/test/run-make/issues-41478-43796/a.rs
@@ -0,0 +1,19 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![crate_type = "lib"]
+pub struct V<S>(S);
+pub trait An {
+    type U;
+}
+pub trait F<A> {
+}
+impl<A: An> F<A> for V<<A as An>::U> {
+}