Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle more span edge cases in generics diagnostics #83759

Merged
merged 5 commits into from
May 13, 2021
Merged

Handle more span edge cases in generics diagnostics #83759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b1c8835
Add more precise span informations to generic types
SkiFire13 May 12, 2021
338dc1f
Fix diagnostics spans for missing generics in edge cases
SkiFire13 May 12, 2021
2cedccb
Fix diagnostics spans for missing lifetimes in edge cases
SkiFire13 May 12, 2021
566012e
Update wrong-number-of-args test to cover more edge cases
SkiFire13 May 12, 2021
7f5ad61
Bless tests
SkiFire13 May 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ impl ParenthesizedArgs {
.cloned()
.map(|input| AngleBracketedArg::Arg(GenericArg::Type(input)))
.collect();
AngleBracketedArgs { span: self.span, args }
AngleBracketedArgs { span: self.inputs_span, args }
}
}

Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ use rustc_span::edition::Edition;
use rustc_span::hygiene::ExpnId;
use rustc_span::source_map::{respan, DesugaringKind};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;
use rustc_span::{Span, DUMMY_SP};
use rustc_target::spec::abi::Abi;

use smallvec::{smallvec, SmallVec};
Expand Down Expand Up @@ -2084,6 +2084,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
args: &[],
bindings: arena_vec![self; self.output_ty_binding(span, output_ty)],
parenthesized: false,
span_ext: DUMMY_SP,
});

hir::GenericBound::LangItemTrait(
Expand Down Expand Up @@ -2788,6 +2789,7 @@ struct GenericArgsCtor<'hir> {
args: SmallVec<[hir::GenericArg<'hir>; 4]>,
bindings: &'hir [hir::TypeBinding<'hir>],
parenthesized: bool,
span: Span,
}

impl<'hir> GenericArgsCtor<'hir> {
Expand All @@ -2800,6 +2802,7 @@ impl<'hir> GenericArgsCtor<'hir> {
args: arena.alloc_from_iter(self.args),
bindings: self.bindings,
parenthesized: self.parenthesized,
span_ext: self.span,
}
}
}
67 changes: 44 additions & 23 deletions compiler/rustc_ast_lowering/src/path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_hir::GenericArg;
use rustc_session::lint::builtin::ELIDED_LIFETIMES_IN_PATHS;
use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_span::symbol::Ident;
use rustc_span::Span;
use rustc_span::{BytePos, Span, DUMMY_SP};

use smallvec::smallvec;
use tracing::debug;
Expand Down Expand Up @@ -267,23 +267,34 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
},
}
} else {
self.lower_angle_bracketed_parameter_data(&Default::default(), param_mode, itctx)
(
GenericArgsCtor {
args: Default::default(),
bindings: &[],
parenthesized: false,
span: path_span.shrink_to_hi(),
},
param_mode == ParamMode::Optional,
)
};

let has_lifetimes =
generic_args.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_)));
let first_generic_span = generic_args
.args
.iter()
.map(|a| a.span())
.chain(generic_args.bindings.iter().map(|b| b.span))
.next();
if !generic_args.parenthesized && !has_lifetimes {
// Note: these spans are used for diagnostics when they can't be inferred.
// See rustc_resolve::late::lifetimes::LifetimeContext::add_missing_lifetime_specifiers_label
let elided_lifetime_span = if generic_args.span.is_empty() {
// If there are no brackets, use the identifier span.
segment.ident.span
} else if generic_args.is_empty() {
// If there are brackets, but not generic arguments, then use the opening bracket
generic_args.span.with_hi(generic_args.span.lo() + BytePos(1))
} else {
// Else use an empty span right after the opening bracket.
generic_args.span.with_lo(generic_args.span.lo() + BytePos(1)).shrink_to_lo()
};
generic_args.args = self
.elided_path_lifetimes(
first_generic_span.map_or(segment.ident.span, |s| s.shrink_to_lo()),
expected_lifetimes,
)
.elided_path_lifetimes(elided_lifetime_span, expected_lifetimes)
.map(GenericArg::Lifetime)
.chain(generic_args.args.into_iter())
.collect();
Expand All @@ -292,15 +303,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let no_non_lt_args = generic_args.args.len() == expected_lifetimes;
let no_bindings = generic_args.bindings.is_empty();
let (incl_angl_brckt, insertion_sp, suggestion) = if no_non_lt_args && no_bindings {
// If there are no (non-implicit) generic args or associated type
// bindings, our suggestion includes the angle brackets.
// If there are no generic args, our suggestion can include the angle brackets.
(true, path_span.shrink_to_hi(), format!("<{}>", anon_lt_suggestion))
} else {
// Otherwise (sorry, this is kind of gross) we need to infer the
// place to splice in the `'_, ` from the generics that do exist.
let first_generic_span = first_generic_span
.expect("already checked that non-lifetime args or bindings exist");
(false, first_generic_span.shrink_to_lo(), format!("{}, ", anon_lt_suggestion))
// Otherwise we'll insert a `'_, ` right after the opening bracket.
let span = generic_args
.span
.with_lo(generic_args.span.lo() + BytePos(1))
.shrink_to_lo();
(false, span, format!("{}, ", anon_lt_suggestion))
};
match self.anonymous_lifetime_mode {
// In create-parameter mode we error here because we don't want to support
Expand Down Expand Up @@ -362,7 +373,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
hir_id: Some(id),
res: Some(self.lower_res(res)),
infer_args,
args: if generic_args.is_empty() {
args: if generic_args.is_empty() && generic_args.span.is_empty() {
None
} else {
Some(self.arena.alloc(generic_args.into_generic_args(self.arena)))
Expand Down Expand Up @@ -395,7 +406,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
AngleBracketedArg::Arg(_) => None,
}));
let ctor = GenericArgsCtor { args, bindings, parenthesized: false };
let ctor = GenericArgsCtor { args, bindings, parenthesized: false, span: data.span };
(ctor, !has_non_lt_args && param_mode == ParamMode::Optional)
}

Expand All @@ -420,7 +431,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let args = smallvec![GenericArg::Type(this.ty_tup(*inputs_span, inputs))];
let binding = this.output_ty_binding(output_ty.span, output_ty);
(
GenericArgsCtor { args, bindings: arena_vec![this; binding], parenthesized: true },
GenericArgsCtor {
args,
bindings: arena_vec![this; binding],
parenthesized: true,
span: data.inputs_span,
},
false,
)
})
Expand All @@ -436,7 +452,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let kind = hir::TypeBindingKind::Equality { ty };
let args = arena_vec![self;];
let bindings = arena_vec![self;];
let gen_args = self.arena.alloc(hir::GenericArgs { args, bindings, parenthesized: false });
let gen_args = self.arena.alloc(hir::GenericArgs {
args,
bindings,
parenthesized: false,
span_ext: DUMMY_SP,
});
hir::TypeBinding { hir_id: self.next_id(), gen_args, span, ident, kind }
}
}
41 changes: 16 additions & 25 deletions compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub use rustc_ast::{CaptureBy, Movability, Mutability};
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
use rustc_data_structures::sync::{par_for_each_in, Send, Sync};
use rustc_macros::HashStable_Generic;
use rustc_span::source_map::{SourceMap, Spanned};
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{def_id::LocalDefId, BytePos};
use rustc_span::{MultiSpan, Span, DUMMY_SP};
Expand Down Expand Up @@ -314,11 +314,18 @@ pub struct GenericArgs<'hir> {
/// This is required mostly for pretty-printing and diagnostics,
/// but also for changing lifetime elision rules to be "function-like".
pub parenthesized: bool,
/// The span encompassing arguments and the surrounding brackets `<>` or `()`
/// Foo<A, B, AssocTy = D> Fn(T, U, V) -> W
/// ^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^
/// Note that this may be:
/// - empty, if there are no generic brackets (but there may be hidden lifetimes)
/// - dummy, if this was generated while desugaring
pub span_ext: Span,
}

impl GenericArgs<'_> {
pub const fn none() -> Self {
Self { args: &[], bindings: &[], parenthesized: false }
Self { args: &[], bindings: &[], parenthesized: false, span_ext: DUMMY_SP }
}

pub fn inputs(&self) -> &[Ty<'_>] {
Expand Down Expand Up @@ -356,33 +363,17 @@ impl GenericArgs<'_> {
own_counts
}

/// The span encompassing the text inside the surrounding brackets.
/// It will also include bindings if they aren't in the form `-> Ret`
/// Returns `None` if the span is empty (e.g. no brackets) or dummy
pub fn span(&self) -> Option<Span> {
self.args
.iter()
.filter(|arg| !arg.is_synthetic())
.map(|arg| arg.span())
.reduce(|span1, span2| span1.to(span2))
let span_ext = self.span_ext()?;
Some(span_ext.with_lo(span_ext.lo() + BytePos(1)).with_hi(span_ext.hi() - BytePos(1)))
}

/// Returns span encompassing arguments and their surrounding `<>` or `()`
pub fn span_ext(&self, sm: &SourceMap) -> Option<Span> {
let mut span = self.span()?;

let (o, c) = if self.parenthesized { ('(', ')') } else { ('<', '>') };

if let Ok(snippet) = sm.span_to_snippet(span) {
let snippet = snippet.as_bytes();

if snippet[0] != (o as u8) || snippet[snippet.len() - 1] != (c as u8) {
span = sm.span_extend_to_prev_char(span, o, true);
span = span.with_lo(span.lo() - BytePos(1));

span = sm.span_extend_to_next_char(span, c, true);
span = span.with_hi(span.hi() + BytePos(1));
}
}

Some(span)
pub fn span_ext(&self) -> Option<Span> {
Some(self.span_ext).filter(|span| !span.is_empty())
}

pub fn is_empty(&self) -> bool {
Expand Down
34 changes: 28 additions & 6 deletions compiler/rustc_resolve/src/late/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1821,7 +1821,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
crate fn add_missing_lifetime_specifiers_label(
&self,
err: &mut DiagnosticBuilder<'_>,
spans_with_counts: Vec<(Span, usize)>,
mut spans_with_counts: Vec<(Span, usize)>,
lifetime_names: &FxHashSet<Symbol>,
lifetime_spans: Vec<Span>,
params: &[ElisionFailureInfo],
Expand All @@ -1831,13 +1831,21 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
.map(|(span, _)| self.tcx.sess.source_map().span_to_snippet(*span).ok())
.collect();

for (span, count) in &spans_with_counts {
// Empty generics are marked with a span of "<", but since from now on
// that information is in the snippets it can be removed from the spans.
for ((span, _), snippet) in spans_with_counts.iter_mut().zip(&snippets) {
if snippet.as_deref() == Some("<") {
*span = span.shrink_to_hi();
}
}

for &(span, count) in &spans_with_counts {
err.span_label(
*span,
span,
format!(
"expected {} lifetime parameter{}",
if *count == 1 { "named".to_string() } else { count.to_string() },
pluralize!(*count),
if count == 1 { "named".to_string() } else { count.to_string() },
pluralize!(count),
),
);
}
Expand Down Expand Up @@ -1982,6 +1990,14 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
.collect::<Vec<_>>()
.join(", "),
)
} else if snippet == "<" || snippet == "(" {
(
span.shrink_to_hi(),
std::iter::repeat("'static")
.take(count)
.collect::<Vec<_>>()
.join(", "),
)
} else {
(
span.shrink_to_hi(),
Expand All @@ -1990,7 +2006,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
std::iter::repeat("'static")
.take(count)
.collect::<Vec<_>>()
.join(", ")
.join(", "),
),
)
}
Expand Down Expand Up @@ -2045,6 +2061,9 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
Some("&") => Some(Box::new(|name| format!("&{} ", name))),
Some("'_") => Some(Box::new(|n| n.to_string())),
Some("") => Some(Box::new(move |n| format!("{}, ", n).repeat(count))),
Some("<") => Some(Box::new(move |n| {
std::iter::repeat(n).take(count).collect::<Vec<_>>().join(", ")
})),
Some(snippet) if !snippet.ends_with('>') => Some(Box::new(move |name| {
format!(
"{}<{}>",
Expand All @@ -2071,6 +2090,9 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
Some("") => {
Some(std::iter::repeat("'a, ").take(count).collect::<Vec<_>>().join(""))
}
Some("<") => {
Some(std::iter::repeat("'a").take(count).collect::<Vec<_>>().join(", "))
}
Some(snippet) => Some(format!(
"{}<{}>",
snippet,
Expand Down
Loading