Rollup merge of rust-lang#40369 - petrochenkov:segspan, r=eddyb

Give spans to individual path segments in AST

And use these spans in path resolution diagnostics.

The spans are spans of identifiers in segments, not whole segments. I'm not sure what spans are more useful in general, but identifier spans are a better fit for resolve errors.

HIR still doesn't have spans.

Fixes rust-lang#38927 (comment) rust-lang#38890 (comment)

r? @nrc @eddyb

Author: Ariel Ben-Yehuda

src/librustc_resolve/lib.rs

@@ -407,7 +407,7 @@ enum PathSource<'a> {
     // Trait paths in bounds or impls.
     Trait,
     // Expression paths `path`, with optional parent context.
-    Expr(Option<&'a ExprKind>),
+    Expr(Option<&'a Expr>),
     // Paths in path patterns `Path`.
     Pat,
     // Paths in struct expressions and patterns `Path { .. }`.
@@ -464,7 +464,7 @@ impl<'a> PathSource<'a> {
                 ValueNS => "method or associated constant",
                 MacroNS => bug!("associated macro"),
             },
-            PathSource::Expr(parent) => match parent {
+            PathSource::Expr(parent) => match parent.map(|p| &p.node) {
                 // "function" here means "anything callable" rather than `Def::Fn`,
                 // this is not precise but usually more helpful than just "value".
                 Some(&ExprKind::Call(..)) => "function",
@@ -2194,14 +2194,16 @@ impl<'a> Resolver<'a> {
                           source: PathSource)
                           -> PathResolution {
         let segments = &path.segments.iter().map(|seg| seg.identifier).collect::<Vec<_>>();
-        self.smart_resolve_path_fragment(id, qself, segments, path.span, source)
+        let ident_span = path.segments.last().map_or(path.span, |seg| seg.span);
+        self.smart_resolve_path_fragment(id, qself, segments, path.span, ident_span, source)
     }
 
     fn smart_resolve_path_fragment(&mut self,
                                    id: NodeId,
                                    qself: Option<&QSelf>,
                                    path: &[Ident],
                                    span: Span,
+                                   ident_span: Span,
                                    source: PathSource)
                                    -> PathResolution {
         let ns = source.namespace();
@@ -2213,9 +2215,9 @@ impl<'a> Resolver<'a> {
             let expected = source.descr_expected();
             let path_str = names_to_string(path);
             let code = source.error_code(def.is_some());
-            let (base_msg, fallback_label) = if let Some(def) = def {
+            let (base_msg, fallback_label, base_span) = if let Some(def) = def {
                 (format!("expected {}, found {} `{}`", expected, def.kind_name(), path_str),
-                 format!("not a {}", expected))
+                 format!("not a {}", expected), span)
             } else {
                 let item_str = path[path.len() - 1];
                 let (mod_prefix, mod_str) = if path.len() == 1 {
@@ -2231,9 +2233,9 @@ impl<'a> Resolver<'a> {
                     (mod_prefix, format!("`{}`", names_to_string(mod_path)))
                 };
                 (format!("cannot find {} `{}` in {}{}", expected, item_str, mod_prefix, mod_str),
-                 format!("not found in {}", mod_str))
+                 format!("not found in {}", mod_str), ident_span)
             };
-            let mut err = this.session.struct_span_err_with_code(span, &base_msg, code);
+            let mut err = this.session.struct_span_err_with_code(base_span, &base_msg, code);
 
             // Emit special messages for unresolved `Self` and `self`.
             if is_self_type(path, ns) {
@@ -2291,15 +2293,15 @@ impl<'a> Resolver<'a> {
                         err.span_label(span, &format!("type aliases cannot be used for traits"));
                         return err;
                     }
-                    (Def::Mod(..), PathSource::Expr(Some(parent))) => match *parent {
+                    (Def::Mod(..), PathSource::Expr(Some(parent))) => match parent.node {
                         ExprKind::Field(_, ident) => {
-                            err.span_label(span, &format!("did you mean `{}::{}`?",
-                                                           path_str, ident.node));
+                            err.span_label(parent.span, &format!("did you mean `{}::{}`?",
+                                                                 path_str, ident.node));
                             return err;
                         }
                         ExprKind::MethodCall(ident, ..) => {
-                            err.span_label(span, &format!("did you mean `{}::{}(...)`?",
-                                                           path_str, ident.node));
+                            err.span_label(parent.span, &format!("did you mean `{}::{}(...)`?",
+                                                                 path_str, ident.node));
                             return err;
                         }
                         _ => {}
@@ -2324,12 +2326,12 @@ impl<'a> Resolver<'a> {
 
             // Try Levenshtein if nothing else worked.
             if let Some(candidate) = this.lookup_typo_candidate(path, ns, is_expected) {
-                err.span_label(span, &format!("did you mean `{}`?", candidate));
+                err.span_label(ident_span, &format!("did you mean `{}`?", candidate));
                 return err;
             }
 
             // Fallback label.
-            err.span_label(span, &fallback_label);
+            err.span_label(base_span, &fallback_label);
             err
         };
         let report_errors = |this: &mut Self, def: Option<Def>| {
@@ -2449,7 +2451,7 @@ impl<'a> Resolver<'a> {
             // Make sure `A::B` in `<T as A>::B::C` is a trait item.
             let ns = if qself.position + 1 == path.len() { ns } else { TypeNS };
             let res = self.smart_resolve_path_fragment(id, None, &path[..qself.position + 1],
-                                                       span, PathSource::TraitItem(ns));
+                                                       span, span, PathSource::TraitItem(ns));
             return Some(PathResolution::with_unresolved_segments(
                 res.base_def(), res.unresolved_segments() + path.len() - qself.position - 1
             ));
@@ -2807,7 +2809,7 @@ impl<'a> Resolver<'a> {
                                        path: &[Ident],
                                        ns: Namespace,
                                        filter_fn: FilterFn)
-                                       -> Option<String>
+                                       -> Option<Symbol>
         where FilterFn: Fn(Def) -> bool
     {
         let add_module_candidates = |module: Module, names: &mut Vec<Name>| {
@@ -2821,7 +2823,7 @@ impl<'a> Resolver<'a> {
         };
 
         let mut names = Vec::new();
-        let prefix_str = if path.len() == 1 {
+        if path.len() == 1 {
             // Search in lexical scope.
             // Walk backwards up the ribs in scope and collect candidates.
             for rib in self.ribs[ns].iter().rev() {
@@ -2855,21 +2857,19 @@ impl<'a> Resolver<'a> {
                     names.push(*name);
                 }
             }
-            String::new()
         } else {
             // Search in module.
             let mod_path = &path[..path.len() - 1];
             if let PathResult::Module(module) = self.resolve_path(mod_path, Some(TypeNS), None) {
                 add_module_candidates(module, &mut names);
             }
-            names_to_string(mod_path) + "::"
-        };
+        }
 
         let name = path[path.len() - 1].name;
         // Make sure error reporting is deterministic.
         names.sort_by_key(|name| name.as_str());
         match find_best_match_for_name(names.iter(), &name.as_str(), None) {
-            Some(found) if found != name => Some(format!("{}{}", prefix_str, found)),
+            Some(found) if found != name => Some(found),
             _ => None,
         }
     }
@@ -2892,7 +2892,7 @@ impl<'a> Resolver<'a> {
         self.with_resolved_label(label, id, |this| this.visit_block(block));
     }
 
-    fn resolve_expr(&mut self, expr: &Expr, parent: Option<&ExprKind>) {
+    fn resolve_expr(&mut self, expr: &Expr, parent: Option<&Expr>) {
         // First, record candidate traits for this expression if it could
         // result in the invocation of a method call.
 
@@ -2973,11 +2973,11 @@ impl<'a> Resolver<'a> {
 
             // Equivalent to `visit::walk_expr` + passing some context to children.
             ExprKind::Field(ref subexpression, _) => {
-                self.resolve_expr(subexpression, Some(&expr.node));
+                self.resolve_expr(subexpression, Some(expr));
             }
             ExprKind::MethodCall(_, ref types, ref arguments) => {
                 let mut arguments = arguments.iter();
-                self.resolve_expr(arguments.next().unwrap(), Some(&expr.node));
+                self.resolve_expr(arguments.next().unwrap(), Some(expr));
                 for argument in arguments {
                     self.resolve_expr(argument, None);
                 }
@@ -2993,7 +2993,7 @@ impl<'a> Resolver<'a> {
                 });
             }
             ExprKind::Call(ref callee, ref arguments) => {
-                self.resolve_expr(callee, Some(&expr.node));
+                self.resolve_expr(callee, Some(expr));
                 for argument in arguments {
                     self.resolve_expr(argument, None);
                 }
@@ -3124,11 +3124,10 @@ impl<'a> Resolver<'a> {
                 if ident.name == lookup_name && ns == namespace {
                     if filter_fn(name_binding.def()) {
                         // create the path
-                        let span = name_binding.span;
                         let mut segms = path_segments.clone();
-                        segms.push(ident.into());
+                        segms.push(ast::PathSegment::from_ident(ident, name_binding.span));
                         let path = Path {
-                            span: span,
+                            span: name_binding.span,
                             segments: segms,
                         };
                         // the entity is accessible in the following cases:
@@ -3148,7 +3147,7 @@ impl<'a> Resolver<'a> {
                 if let Some(module) = name_binding.module() {
                     // form the path
                     let mut path_segments = path_segments.clone();
-                    path_segments.push(ident.into());
+                    path_segments.push(ast::PathSegment::from_ident(ident, name_binding.span));
 
                     if !in_module_is_extern || name_binding.vis == ty::Visibility::Public {
                         // add the module to the lookup

src/librustc_resolve/macros.rs

@@ -110,8 +110,11 @@ impl<'a> base::Resolver for Resolver<'a> {
                     path.segments[0].identifier.name = keywords::CrateRoot.name();
                     let module = self.0.resolve_crate_var(ident.ctxt);
                     if !module.is_local() {
+                        let span = path.segments[0].span;
                         path.segments.insert(1, match module.kind {
-                            ModuleKind::Def(_, name) => ast::Ident::with_empty_ctxt(name).into(),
+                            ModuleKind::Def(_, name) => ast::PathSegment::from_ident(
+                                ast::Ident::with_empty_ctxt(name), span
+                            ),
                             _ => unreachable!(),
                         })
                     }
@@ -499,7 +502,6 @@ impl<'a> Resolver<'a> {
             };
             let ident = Ident::from_str(name);
             self.lookup_typo_candidate(&vec![ident], MacroNS, is_macro)
-                .as_ref().map(|s| Symbol::intern(s))
         });
 
         if let Some(suggestion) = suggestion {

src/libsyntax/ast.rs

@@ -134,7 +134,7 @@ impl Path {
     pub fn from_ident(s: Span, identifier: Ident) -> Path {
         Path {
             span: s,
-            segments: vec![identifier.into()],
+            segments: vec![PathSegment::from_ident(identifier, s)],
         }
     }
 
@@ -159,6 +159,8 @@ impl Path {
 pub struct PathSegment {
     /// The identifier portion of this path segment.
     pub identifier: Ident,
+    /// Span of the segment identifier.
+    pub span: Span,
 
     /// Type/lifetime parameters attached to this path. They come in
     /// two flavors: `Path<A,B,C>` and `Path(A,B) -> C`. Note that
@@ -170,16 +172,14 @@ pub struct PathSegment {
     pub parameters: Option<P<PathParameters>>,
 }
 
-impl From<Ident> for PathSegment {
-    fn from(id: Ident) -> Self {
-        PathSegment { identifier: id, parameters: None }
-    }
-}
-
 impl PathSegment {
+    pub fn from_ident(ident: Ident, span: Span) -> Self {
+        PathSegment { identifier: ident, span: span, parameters: None }
+    }
     pub fn crate_root() -> Self {
         PathSegment {
             identifier: keywords::CrateRoot.ident(),
+            span: DUMMY_SP,
             parameters: None,
         }
     }

src/libsyntax/ext/build.rs

@@ -38,11 +38,11 @@ pub trait AstBuilder {
 
     fn qpath(&self, self_type: P<ast::Ty>,
              trait_path: ast::Path,
-             ident: ast::Ident)
+             ident: ast::SpannedIdent)
              -> (ast::QSelf, ast::Path);
     fn qpath_all(&self, self_type: P<ast::Ty>,
                 trait_path: ast::Path,
-                ident: ast::Ident,
+                ident: ast::SpannedIdent,
                 lifetimes: Vec<ast::Lifetime>,
                 types: Vec<P<ast::Ty>>,
                 bindings: Vec<ast::TypeBinding>)
@@ -323,7 +323,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
             segments.push(ast::PathSegment::crate_root());
         }
 
-        segments.extend(idents.into_iter().map(Into::into));
+        segments.extend(idents.into_iter().map(|i| ast::PathSegment::from_ident(i, sp)));
         let parameters = if lifetimes.is_empty() && types.is_empty() && bindings.is_empty() {
             None
         } else {
@@ -333,7 +333,11 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
                 bindings: bindings,
             })))
         };
-        segments.push(ast::PathSegment { identifier: last_identifier, parameters: parameters });
+        segments.push(ast::PathSegment {
+            identifier: last_identifier,
+            span: sp,
+            parameters: parameters
+        });
         ast::Path {
             span: sp,
             segments: segments,
@@ -346,7 +350,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
     fn qpath(&self,
              self_type: P<ast::Ty>,
              trait_path: ast::Path,
-             ident: ast::Ident)
+             ident: ast::SpannedIdent)
              -> (ast::QSelf, ast::Path) {
         self.qpath_all(self_type, trait_path, ident, vec![], vec![], vec![])
     }
@@ -357,7 +361,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
     fn qpath_all(&self,
                  self_type: P<ast::Ty>,
                  trait_path: ast::Path,
-                 ident: ast::Ident,
+                 ident: ast::SpannedIdent,
                  lifetimes: Vec<ast::Lifetime>,
                  types: Vec<P<ast::Ty>>,
                  bindings: Vec<ast::TypeBinding>)
@@ -369,7 +373,8 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
             bindings: bindings,
         };
         path.segments.push(ast::PathSegment {
-            identifier: ident,
+            identifier: ident.node,
+            span: ident.span,
             parameters: Some(P(ast::PathParameters::AngleBracketed(parameters))),
         });
 

src/libsyntax/fold.rs

@@ -434,8 +434,9 @@ pub fn noop_fold_usize<T: Folder>(i: usize, _: &mut T) -> usize {
 
 pub fn noop_fold_path<T: Folder>(Path { segments, span }: Path, fld: &mut T) -> Path {
     Path {
-        segments: segments.move_map(|PathSegment {identifier, parameters}| PathSegment {
+        segments: segments.move_map(|PathSegment {identifier, span, parameters}| PathSegment {
             identifier: fld.fold_ident(identifier),
+            span: fld.new_span(span),
             parameters: parameters.map(|ps| ps.map(|ps| fld.fold_path_parameters(ps))),
         }),
         span: fld.new_span(span)