Skip to content

Commit

Permalink
Rollup merge of rust-lang#128852 - folkertdev:multipart-suggestion-st…
Browse files Browse the repository at this point in the history
…able-sort, r=compiler-errors

use stable sort to sort multipart diagnostics

I think a stable sort should be used to sort the different parts of a multipart selection. The current unstable sort uses the text of the suggestion as a tie-breaker. That just doesn't seem right, and the order of the input is a better choice I think, because it gives the diagnostic author more control.

This came up when I was building a suggestion where

```rust
fn foo() {}
```

must be turned into an unsafe function, and an attribute must be added

```rust
#[target_feature(enable = "...")]
unsafe fn foo() {}
```

In this example, the two suggestions occur at the same position, but the order is extremely important: unsafe must come after the attribute. But the situation changes if there is a pub/pub(crate), and if the unsafe is already present. It just out that because of the suggestion text, there is no way for me to order the suggestions correctly.

This change probably should be tested, but are there tests of the diagnostics code itself in the tests?

r? `@estebank`
  • Loading branch information
matthiaskrgr authored Aug 9, 2024
2 parents 40db4e7 + 38874a6 commit 44db568
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1124,8 +1124,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
err.multipart_suggestion(
"consider moving the expression out of the loop so it is only moved once",
vec![
(parent.span, "value".to_string()),
(span.shrink_to_lo(), format!("let mut value = {value};{indent}")),
(parent.span, "value".to_string()),
],
Applicability::MaybeIncorrect,
);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -920,8 +920,8 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
applicability: Applicability,
style: SuggestionStyle,
) -> &mut Self {
suggestion.sort_unstable();
suggestion.dedup_by(|(s1, m1), (s2, m2)| s1.source_equal(*s2) && m1 == m2);
let mut seen = crate::FxHashSet::default();
suggestion.retain(|(span, msg)| seen.insert((span.lo(), span.hi(), msg.clone())));

let parts = suggestion
.into_iter()
Expand Down

0 comments on commit 44db568

Please sign in to comment.