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.

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

Remove overlapping parts of multipart suggestions #106916

Merged
merged 4 commits into from Jan 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
71 changes: 44 additions & 27 deletions compiler/rustc_errors/src/diagnostic.rs
Expand Up @@ -629,19 +629,27 @@ impl Diagnostic {
applicability: Applicability,
style: SuggestionStyle,
) -> &mut Self {
assert!(!suggestion.is_empty());
debug_assert!(
!(suggestion.iter().any(|(sp, text)| sp.is_empty() && text.is_empty())),
"Span must not be empty and have no suggestion"
let mut parts = suggestion
.into_iter()
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect::<Vec<_>>();

parts.sort_unstable_by_key(|part| part.span);

assert!(!parts.is_empty());
debug_assert_eq!(
parts.iter().find(|part| part.span.is_empty() && part.snippet.is_empty()),
None,
"Span must not be empty and have no suggestion",
);
debug_assert_eq!(
parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)),
None,
"suggestion must not have overlapping parts",
);

self.push_suggestion(CodeSuggestion {
substitutions: vec![Substitution {
parts: suggestion
.into_iter()
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect(),
}],
substitutions: vec![Substitution { parts }],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 would small vec be good here?

msg: self.subdiagnostic_message_to_diagnostic_message(msg),
style,
applicability,
Expand Down Expand Up @@ -802,25 +810,34 @@ impl Diagnostic {
suggestions: impl IntoIterator<Item = Vec<(Span, String)>>,
applicability: Applicability,
) -> &mut Self {
let suggestions: Vec<_> = suggestions.into_iter().collect();
debug_assert!(
!(suggestions
.iter()
.flatten()
.any(|(sp, suggestion)| sp.is_empty() && suggestion.is_empty())),
"Span must not be empty and have no suggestion"
);
let substitutions = suggestions
.into_iter()
.map(|sugg| {
let mut parts = sugg
.into_iter()
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect::<Vec<_>>();

parts.sort_unstable_by_key(|part| part.span);

assert!(!parts.is_empty());
debug_assert_eq!(
parts.iter().find(|part| part.span.is_empty() && part.snippet.is_empty()),
None,
"Span must not be empty and have no suggestion",
);
debug_assert_eq!(
parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)),
None,
"suggestion must not have overlapping parts",
);

Substitution { parts }
})
.collect();

self.push_suggestion(CodeSuggestion {
substitutions: suggestions
.into_iter()
.map(|sugg| Substitution {
parts: sugg
.into_iter()
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect(),
})
.collect(),
substitutions,
msg: self.subdiagnostic_message_to_diagnostic_message(msg),
style: SuggestionStyle::ShowCode,
applicability,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_errors/src/lib.rs
Expand Up @@ -3,6 +3,7 @@
//! This module contains the code for creating and emitting diagnostics.

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(array_windows)]
#![feature(drain_filter)]
#![feature(if_let_guard)]
#![feature(is_terminal)]
Expand Down
24 changes: 18 additions & 6 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Expand Up @@ -10,7 +10,7 @@ use crate::mbe::transcribe::transcribe;

use rustc_ast as ast;
use rustc_ast::token::{self, Delimiter, NonterminalKind, Token, TokenKind, TokenKind::*};
use rustc_ast::tokenstream::{DelimSpan, TokenStream};
use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree};
use rustc_ast::{NodeId, DUMMY_NODE_ID};
use rustc_ast_pretty::pprust;
use rustc_attr::{self as attr, TransparencyError};
Expand Down Expand Up @@ -212,7 +212,6 @@ fn expand_macro<'cx>(
};
let arm_span = rhses[i].span();

let rhs_spans = rhs.tts.iter().map(|t| t.span()).collect::<Vec<_>>();
// rhs has holes ( `$id` and `$(...)` that need filled)
let mut tts = match transcribe(cx, &named_matches, &rhs, rhs_span, transparency) {
Ok(tts) => tts,
Expand All @@ -224,12 +223,25 @@ fn expand_macro<'cx>(

// Replace all the tokens for the corresponding positions in the macro, to maintain
// proper positions in error reporting, while maintaining the macro_backtrace.
if rhs_spans.len() == tts.len() {
if tts.len() == rhs.tts.len() {
tts = tts.map_enumerated(|i, tt| {
let mut tt = tt.clone();
let mut sp = rhs_spans[i];
sp = sp.with_ctxt(tt.span().ctxt());
tt.set_span(sp);
let rhs_tt = &rhs.tts[i];
let ctxt = tt.span().ctxt();
match (&mut tt, rhs_tt) {
// preserve the delim spans if able
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

(
TokenTree::Delimited(target_sp, ..),
mbe::TokenTree::Delimited(source_sp, ..),
) => {
target_sp.open = source_sp.open.with_ctxt(ctxt);
target_sp.close = source_sp.close.with_ctxt(ctxt);
}
_ => {
let sp = rhs_tt.span().with_ctxt(ctxt);
tt.set_span(sp);
}
}
tt
});
}
Expand Down
37 changes: 30 additions & 7 deletions compiler/rustc_lint/src/builtin.rs
Expand Up @@ -2173,13 +2173,31 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
dropped_predicate_count += 1;
}

if drop_predicate && !in_where_clause {
lint_spans.push(predicate_span);
} else if drop_predicate && i + 1 < num_predicates {
// If all the bounds on a predicate were inferable and there are
// further predicates, we want to eat the trailing comma.
let next_predicate_span = hir_generics.predicates[i + 1].span();
where_lint_spans.push(predicate_span.to(next_predicate_span.shrink_to_lo()));
if drop_predicate {
if !in_where_clause {
lint_spans.push(predicate_span);
} else if predicate_span.from_expansion() {
// Don't try to extend the span if it comes from a macro expansion.
where_lint_spans.push(predicate_span);
} else if i + 1 < num_predicates {
// If all the bounds on a predicate were inferable and there are
// further predicates, we want to eat the trailing comma.
let next_predicate_span = hir_generics.predicates[i + 1].span();
if next_predicate_span.from_expansion() {
where_lint_spans.push(predicate_span);
} else {
where_lint_spans
.push(predicate_span.to(next_predicate_span.shrink_to_lo()));
}
} else {
// Eat the optional trailing comma after the last predicate.
let where_span = hir_generics.where_clause_span;
if where_span.from_expansion() {
where_lint_spans.push(predicate_span);
} else {
where_lint_spans.push(predicate_span.to(where_span.shrink_to_hi()));
}
}
} else {
where_lint_spans.extend(self.consolidate_outlives_bound_spans(
predicate_span.shrink_to_lo(),
Expand Down Expand Up @@ -2223,6 +2241,11 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
Applicability::MaybeIncorrect
};

// Due to macros, there might be several predicates with the same span
// and we only want to suggest removing them once.
lint_spans.sort_unstable();
lint_spans.dedup();

cx.emit_spanned_lint(
EXPLICIT_OUTLIVES_REQUIREMENTS,
lint_spans.clone(),
Expand Down
4 changes: 4 additions & 0 deletions src/tools/clippy/clippy_lints/src/format_args.rs
Expand Up @@ -311,6 +311,10 @@ fn check_uninlined_args(
// in those cases, make the code suggestion hidden
let multiline_fix = fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span));

// Suggest removing each argument only once, for example in `format!("{0} {0}", arg)`.
fixes.sort_unstable_by_key(|(span, _)| *span);
fixes.dedup_by_key(|(span, _)| *span);

span_lint_and_then(
cx,
UNINLINED_FORMAT_ARGS,
Expand Down
23 changes: 9 additions & 14 deletions tests/ui/const-generics/min_const_generics/macro-fail.stderr
Expand Up @@ -8,7 +8,7 @@ LL | fn make_marker() -> impl Marker<gimme_a_const!(marker)> {
| in this macro invocation
...
LL | ($rusty: ident) => {{ let $rusty = 3; *&$rusty }}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected type
| ^ expected type
|
= note: this error originates in the macro `gimme_a_const` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand All @@ -22,26 +22,21 @@ LL | Example::<gimme_a_const!(marker)>
| in this macro invocation
...
LL | ($rusty: ident) => {{ let $rusty = 3; *&$rusty }}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected type
| ^ expected type
|
= note: this error originates in the macro `gimme_a_const` (in Nightly builds, run with -Z macro-backtrace for more info)

error: expected type, found `{`
--> $DIR/macro-fail.rs:4:10
|
LL | () => {{
| __________^
LL | |
LL | | const X: usize = 1337;
LL | | X
LL | | }}
| |___^ expected type
LL | () => {{
| ^ expected type
...
LL | let _fail = Example::<external_macro!()>;
| -----------------
| |
| this macro call doesn't expand to a type
| in this macro invocation
LL | let _fail = Example::<external_macro!()>;
| -----------------
| |
| this macro call doesn't expand to a type
| in this macro invocation
|
= note: this error originates in the macro `external_macro` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/imports/import-prefix-macro-1.stderr
Expand Up @@ -2,7 +2,7 @@ error: expected one of `::`, `;`, or `as`, found `{`
--> $DIR/import-prefix-macro-1.rs:11:27
|
LL | ($p: path) => (use $p {S, Z});
| ^^^^^^ expected one of `::`, `;`, or `as`
| ^ expected one of `::`, `;`, or `as`
...
LL | import! { a::b::c }
| ------------------- in this macro invocation
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/parser/issues/issue-44406.stderr
Expand Up @@ -21,8 +21,8 @@ LL | foo!(true);
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
help: if `bar` is a struct, use braces as delimiters
|
LL | bar { }
| ~
LL | bar { baz: $rest }
| ~ ~
help: if `bar` is a function, use the arguments directly
|
LL - bar(baz: $rest)
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/rust-2018/edition-lint-infer-outlives-multispan.rs
Expand Up @@ -365,4 +365,24 @@ mod unions {
}
}

// https://github.com/rust-lang/rust/issues/106870
mod multiple_predicates_with_same_span {
macro_rules! m {
($($name:ident)+) => {
struct Inline<'a, $($name: 'a,)+>(&'a ($($name,)+));
//~^ ERROR: outlives requirements can be inferred
struct FullWhere<'a, $($name,)+>(&'a ($($name,)+)) where $($name: 'a,)+;
//~^ ERROR: outlives requirements can be inferred
struct PartialWhere<'a, $($name,)+>(&'a ($($name,)+)) where (): Sized, $($name: 'a,)+;
//~^ ERROR: outlives requirements can be inferred
struct Interleaved<'a, $($name,)+>(&'a ($($name,)+))
where
(): Sized,
$($name: 'a, $name: 'a, )+ //~ ERROR: outlives requirements can be inferred
$($name: 'a, $name: 'a, )+;
}
}
m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
}

fn main() {}
58 changes: 57 additions & 1 deletion tests/ui/rust-2018/edition-lint-infer-outlives-multispan.stderr
Expand Up @@ -819,5 +819,61 @@ LL - union BeeWhereAyTeeYooWhereOutlivesAyIsDebugBee<'a, 'b, T, U> where U:
LL + union BeeWhereAyTeeYooWhereOutlivesAyIsDebugBee<'a, 'b, T, U> where U: Debug, {
|

error: aborting due to 68 previous errors
error: outlives requirements can be inferred
--> $DIR/edition-lint-infer-outlives-multispan.rs:372:38
|
LL | struct Inline<'a, $($name: 'a,)+>(&'a ($($name,)+));
| ^^^^ help: remove these bounds
...
LL | m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
| --------------------------------------------------------- in this macro invocation
|
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

error: outlives requirements can be inferred
--> $DIR/edition-lint-infer-outlives-multispan.rs:374:64
|
LL | struct FullWhere<'a, $($name,)+>(&'a ($($name,)+)) where $($name: 'a,)+;
| ^^^^^^^^^^^^^^^^^^ help: remove these bounds
...
LL | m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
| --------------------------------------------------------- in this macro invocation
|
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

error: outlives requirements can be inferred
--> $DIR/edition-lint-infer-outlives-multispan.rs:376:86
|
LL | struct PartialWhere<'a, $($name,)+>(&'a ($($name,)+)) where (): Sized, $($name: 'a,)+;
| ^^^^^^^^^ help: remove these bounds
...
LL | m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
| --------------------------------------------------------- in this macro invocation
|
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

error: outlives requirements can be inferred
--> $DIR/edition-lint-infer-outlives-multispan.rs:381:19
|
LL | $($name: 'a, $name: 'a, )+
| ^^^^^^^^^ ^^^^^^^^^
LL | $($name: 'a, $name: 'a, )+;
| ^^^^^^^^^ ^^^^^^^^^
...
LL | m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
| ---------------------------------------------------------
| |
| in this macro invocation
| in this macro invocation
| in this macro invocation
| in this macro invocation
|
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
help: remove these bounds
|
LL ~ $(, , )+
LL ~ $(, , )+;
|

error: aborting due to 72 previous errors

9 changes: 9 additions & 0 deletions tests/ui/rust-2018/edition-lint-infer-outlives.fixed
Expand Up @@ -791,5 +791,14 @@ struct StaticRef<T: 'static> {
field: &'static T
}

struct TrailingCommaInWhereClause<'a, T, U>
where
T: 'a,

//~^ ERROR outlives requirements can be inferred
{
tee: T,
yoo: &'a U
}

fn main() {}
9 changes: 9 additions & 0 deletions tests/ui/rust-2018/edition-lint-infer-outlives.rs
Expand Up @@ -791,5 +791,14 @@ struct StaticRef<T: 'static> {
field: &'static T
}

struct TrailingCommaInWhereClause<'a, T, U>
where
T: 'a,
U: 'a,
//~^ ERROR outlives requirements can be inferred
{
tee: T,
yoo: &'a U
}

fn main() {}