Skip to content

Commit 631e989

Browse files
borsbors
committed
Auto merge of rust-lang#83759 - SkiFire13:fix-diag, r=estebank
Handle more span edge cases in generics diagnostics This should fix invalid suggestions that didn't account for empty bracket pairs (`<>`) or type bindings.
2 parents 9daf546 + 7f5ad61 commit 631e989

File tree

13 files changed

+909
-128
lines changed

13 files changed

+909
-128
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ impl ParenthesizedArgs {
278278
.cloned()
279279
.map(|input| AngleBracketedArg::Arg(GenericArg::Type(input)))
280280
.collect();
281-
AngleBracketedArgs { span: self.span, args }
281+
AngleBracketedArgs { span: self.inputs_span, args }
282282
}
283283
}
284284

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ use rustc_span::edition::Edition;
6262
use rustc_span::hygiene::ExpnId;
6363
use rustc_span::source_map::{respan, DesugaringKind};
6464
use rustc_span::symbol::{kw, sym, Ident, Symbol};
65-
use rustc_span::Span;
65+
use rustc_span::{Span, DUMMY_SP};
6666
use rustc_target::spec::abi::Abi;
6767

6868
use smallvec::{smallvec, SmallVec};
@@ -2084,6 +2084,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
20842084
args: &[],
20852085
bindings: arena_vec![self; self.output_ty_binding(span, output_ty)],
20862086
parenthesized: false,
2087+
span_ext: DUMMY_SP,
20872088
});
20882089

20892090
hir::GenericBound::LangItemTrait(
@@ -2788,6 +2789,7 @@ struct GenericArgsCtor<'hir> {
27882789
args: SmallVec<[hir::GenericArg<'hir>; 4]>,
27892790
bindings: &'hir [hir::TypeBinding<'hir>],
27902791
parenthesized: bool,
2792+
span: Span,
27912793
}
27922794

27932795
impl<'hir> GenericArgsCtor<'hir> {
@@ -2800,6 +2802,7 @@ impl<'hir> GenericArgsCtor<'hir> {
28002802
args: arena.alloc_from_iter(self.args),
28012803
bindings: self.bindings,
28022804
parenthesized: self.parenthesized,
2805+
span_ext: self.span,
28032806
}
28042807
}
28052808
}

‎compiler/rustc_ast_lowering/src/path.rs

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_hir::GenericArg;
1010
use rustc_session::lint::builtin::ELIDED_LIFETIMES_IN_PATHS;
1111
use rustc_session::lint::BuiltinLintDiagnostics;
1212
use rustc_span::symbol::Ident;
13-
use rustc_span::Span;
13+
use rustc_span::{BytePos, Span, DUMMY_SP};
1414

1515
use smallvec::smallvec;
1616
use tracing::debug;
@@ -267,23 +267,34 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
267267
},
268268
}
269269
} else {
270-
self.lower_angle_bracketed_parameter_data(&Default::default(), param_mode, itctx)
270+
(
271+
GenericArgsCtor {
272+
args: Default::default(),
273+
bindings: &[],
274+
parenthesized: false,
275+
span: path_span.shrink_to_hi(),
276+
},
277+
param_mode == ParamMode::Optional,
278+
)
271279
};
272280

273281
let has_lifetimes =
274282
generic_args.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_)));
275-
let first_generic_span = generic_args
276-
.args
277-
.iter()
278-
.map(|a| a.span())
279-
.chain(generic_args.bindings.iter().map(|b| b.span))
280-
.next();
281283
if !generic_args.parenthesized && !has_lifetimes {
284+
// Note: these spans are used for diagnostics when they can't be inferred.
285+
// See rustc_resolve::late::lifetimes::LifetimeContext::add_missing_lifetime_specifiers_label
286+
let elided_lifetime_span = if generic_args.span.is_empty() {
287+
// If there are no brackets, use the identifier span.
288+
segment.ident.span
289+
} else if generic_args.is_empty() {
290+
// If there are brackets, but not generic arguments, then use the opening bracket
291+
generic_args.span.with_hi(generic_args.span.lo() + BytePos(1))
292+
} else {
293+
// Else use an empty span right after the opening bracket.
294+
generic_args.span.with_lo(generic_args.span.lo() + BytePos(1)).shrink_to_lo()
295+
};
282296
generic_args.args = self
283-
.elided_path_lifetimes(
284-
first_generic_span.map_or(segment.ident.span, |s| s.shrink_to_lo()),
285-
expected_lifetimes,
286-
)
297+
.elided_path_lifetimes(elided_lifetime_span, expected_lifetimes)
287298
.map(GenericArg::Lifetime)
288299
.chain(generic_args.args.into_iter())
289300
.collect();
@@ -292,15 +303,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
292303
let no_non_lt_args = generic_args.args.len() == expected_lifetimes;
293304
let no_bindings = generic_args.bindings.is_empty();
294305
let (incl_angl_brckt, insertion_sp, suggestion) = if no_non_lt_args && no_bindings {
295-
// If there are no (non-implicit) generic args or associated type
296-
// bindings, our suggestion includes the angle brackets.
306+
// If there are no generic args, our suggestion can include the angle brackets.
297307
(true, path_span.shrink_to_hi(), format!("<{}>", anon_lt_suggestion))
298308
} else {
299-
// Otherwise (sorry, this is kind of gross) we need to infer the
300-
// place to splice in the `'_, ` from the generics that do exist.
301-
let first_generic_span = first_generic_span
302-
.expect("already checked that non-lifetime args or bindings exist");
303-
(false, first_generic_span.shrink_to_lo(), format!("{}, ", anon_lt_suggestion))
309+
// Otherwise we'll insert a `'_, ` right after the opening bracket.
310+
let span = generic_args
311+
.span
312+
.with_lo(generic_args.span.lo() + BytePos(1))
313+
.shrink_to_lo();
314+
(false, span, format!("{}, ", anon_lt_suggestion))
304315
};
305316
match self.anonymous_lifetime_mode {
306317
// In create-parameter mode we error here because we don't want to support
@@ -362,7 +373,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
362373
hir_id: Some(id),
363374
res: Some(self.lower_res(res)),
364375
infer_args,
365-
args: if generic_args.is_empty() {
376+
args: if generic_args.is_empty() && generic_args.span.is_empty() {
366377
None
367378
} else {
368379
Some(self.arena.alloc(generic_args.into_generic_args(self.arena)))
@@ -395,7 +406,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
395406
}
396407
AngleBracketedArg::Arg(_) => None,
397408
}));
398-
let ctor = GenericArgsCtor { args, bindings, parenthesized: false };
409+
let ctor = GenericArgsCtor { args, bindings, parenthesized: false, span: data.span };
399410
(ctor, !has_non_lt_args && param_mode == ParamMode::Optional)
400411
}
401412

@@ -420,7 +431,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
420431
let args = smallvec![GenericArg::Type(this.ty_tup(*inputs_span, inputs))];
421432
let binding = this.output_ty_binding(output_ty.span, output_ty);
422433
(
423-
GenericArgsCtor { args, bindings: arena_vec![this; binding], parenthesized: true },
434+
GenericArgsCtor {
435+
args,
436+
bindings: arena_vec![this; binding],
437+
parenthesized: true,
438+
span: data.inputs_span,
439+
},
424440
false,
425441
)
426442
})
@@ -436,7 +452,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
436452
let kind = hir::TypeBindingKind::Equality { ty };
437453
let args = arena_vec![self;];
438454
let bindings = arena_vec![self;];
439-
let gen_args = self.arena.alloc(hir::GenericArgs { args, bindings, parenthesized: false });
455+
let gen_args = self.arena.alloc(hir::GenericArgs {
456+
args,
457+
bindings,
458+
parenthesized: false,
459+
span_ext: DUMMY_SP,
460+
});
440461
hir::TypeBinding { hir_id: self.next_id(), gen_args, span, ident, kind }
441462
}
442463
}

‎compiler/rustc_hir/src/hir.rs

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ pub use rustc_ast::{CaptureBy, Movability, Mutability};
1212
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
1313
use rustc_data_structures::sync::{par_for_each_in, Send, Sync};
1414
use rustc_macros::HashStable_Generic;
15-
use rustc_span::source_map::{SourceMap, Spanned};
15+
use rustc_span::source_map::Spanned;
1616
use rustc_span::symbol::{kw, sym, Ident, Symbol};
1717
use rustc_span::{def_id::LocalDefId, BytePos};
1818
use rustc_span::{MultiSpan, Span, DUMMY_SP};
@@ -314,11 +314,18 @@ pub struct GenericArgs<'hir> {
314314
/// This is required mostly for pretty-printing and diagnostics,
315315
/// but also for changing lifetime elision rules to be "function-like".
316316
pub parenthesized: bool,
317+
/// The span encompassing arguments and the surrounding brackets `<>` or `()`
318+
/// Foo<A, B, AssocTy = D> Fn(T, U, V) -> W
319+
/// ^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^
320+
/// Note that this may be:
321+
/// - empty, if there are no generic brackets (but there may be hidden lifetimes)
322+
/// - dummy, if this was generated while desugaring
323+
pub span_ext: Span,
317324
}
318325

319326
impl GenericArgs<'_> {
320327
pub const fn none() -> Self {
321-
Self { args: &[], bindings: &[], parenthesized: false }
328+
Self { args: &[], bindings: &[], parenthesized: false, span_ext: DUMMY_SP }
322329
}
323330

324331
pub fn inputs(&self) -> &[Ty<'_>] {
@@ -356,33 +363,17 @@ impl GenericArgs<'_> {
356363
own_counts
357364
}
358365

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

367374
/// Returns span encompassing arguments and their surrounding `<>` or `()`
368-
pub fn span_ext(&self, sm: &SourceMap) -> Option<Span> {
369-
let mut span = self.span()?;
370-
371-
let (o, c) = if self.parenthesized { ('(', ')') } else { ('<', '>') };
372-
373-
if let Ok(snippet) = sm.span_to_snippet(span) {
374-
let snippet = snippet.as_bytes();
375-
376-
if snippet[0] != (o as u8) || snippet[snippet.len() - 1] != (c as u8) {
377-
span = sm.span_extend_to_prev_char(span, o, true);
378-
span = span.with_lo(span.lo() - BytePos(1));
379-
380-
span = sm.span_extend_to_next_char(span, c, true);
381-
span = span.with_hi(span.hi() + BytePos(1));
382-
}
383-
}
384-
385-
Some(span)
375+
pub fn span_ext(&self) -> Option<Span> {
376+
Some(self.span_ext).filter(|span| !span.is_empty())
386377
}
387378

388379
pub fn is_empty(&self) -> bool {

‎compiler/rustc_resolve/src/late/diagnostics.rs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1821,7 +1821,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
18211821
crate fn add_missing_lifetime_specifiers_label(
18221822
&self,
18231823
err: &mut DiagnosticBuilder<'_>,
1824-
spans_with_counts: Vec<(Span, usize)>,
1824+
mut spans_with_counts: Vec<(Span, usize)>,
18251825
lifetime_names: &FxHashSet<Symbol>,
18261826
lifetime_spans: Vec<Span>,
18271827
params: &[ElisionFailureInfo],
@@ -1831,13 +1831,21 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
18311831
.map(|(span, _)| self.tcx.sess.source_map().span_to_snippet(*span).ok())
18321832
.collect();
18331833

1834-
for (span, count) in &spans_with_counts {
1834+
// Empty generics are marked with a span of "<", but since from now on
1835+
// that information is in the snippets it can be removed from the spans.
1836+
for ((span, _), snippet) in spans_with_counts.iter_mut().zip(&snippets) {
1837+
if snippet.as_deref() == Some("<") {
1838+
*span = span.shrink_to_hi();
1839+
}
1840+
}
1841+
1842+
for &(span, count) in &spans_with_counts {
18351843
err.span_label(
1836-
*span,
1844+
span,
18371845
format!(
18381846
"expected {} lifetime parameter{}",
1839-
if *count == 1 { "named".to_string() } else { count.to_string() },
1840-
pluralize!(*count),
1847+
if count == 1 { "named".to_string() } else { count.to_string() },
1848+
pluralize!(count),
18411849
),
18421850
);
18431851
}
@@ -1982,6 +1990,14 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
19821990
.collect::<Vec<_>>()
19831991
.join(", "),
19841992
)
1993+
} else if snippet == "<" || snippet == "(" {
1994+
(
1995+
span.shrink_to_hi(),
1996+
std::iter::repeat("'static")
1997+
.take(count)
1998+
.collect::<Vec<_>>()
1999+
.join(", "),
2000+
)
19852001
} else {
19862002
(
19872003
span.shrink_to_hi(),
@@ -1990,7 +2006,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
19902006
std::iter::repeat("'static")
19912007
.take(count)
19922008
.collect::<Vec<_>>()
1993-
.join(", ")
2009+
.join(", "),
19942010
),
19952011
)
19962012
}
@@ -2045,6 +2061,9 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
20452061
Some("&") => Some(Box::new(|name| format!("&{} ", name))),
20462062
Some("'_") => Some(Box::new(|n| n.to_string())),
20472063
Some("") => Some(Box::new(move |n| format!("{}, ", n).repeat(count))),
2064+
Some("<") => Some(Box::new(move |n| {
2065+
std::iter::repeat(n).take(count).collect::<Vec<_>>().join(", ")
2066+
})),
20482067
Some(snippet) if !snippet.ends_with('>') => Some(Box::new(move |name| {
20492068
format!(
20502069
"{}<{}>",
@@ -2071,6 +2090,9 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
20712090
Some("") => {
20722091
Some(std::iter::repeat("'a, ").take(count).collect::<Vec<_>>().join(""))
20732092
}
2093+
Some("<") => {
2094+
Some(std::iter::repeat("'a").take(count).collect::<Vec<_>>().join(", "))
2095+
}
20742096
Some(snippet) => Some(format!(
20752097
"{}<{}>",
20762098
snippet,

0 commit comments

Comments
 (0)