Skip to content

Commit

Permalink
Rollup merge of rust-lang#40369 - petrochenkov:segspan, r=eddyb
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Ariel Ben-Yehuda authored Mar 9, 2017
2 parents 72fc6d4 + 7cfe20c commit a1b148e
Show file tree
Hide file tree
Showing 14 changed files with 124 additions and 88 deletions.
57 changes: 28 additions & 29 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { .. }`.
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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();
Expand All @@ -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 {
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}
_ => {}
Expand All @@ -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>| {
Expand Down Expand Up @@ -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
));
Expand Down Expand Up @@ -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>| {
Expand All @@ -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() {
Expand Down Expand Up @@ -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,
}
}
Expand All @@ -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.

Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
})
}
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 7 additions & 7 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
}
}

Expand All @@ -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
Expand All @@ -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,
}
}
Expand Down
19 changes: 12 additions & 7 deletions src/libsyntax/ext/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>)
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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![])
}
Expand All @@ -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>)
Expand All @@ -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))),
});

Expand Down
3 changes: 2 additions & 1 deletion src/libsyntax/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit a1b148e

Please sign in to comment.