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

dedup for duplicate suggestions #118057

Merged
merged 1 commit into from
Dec 9, 2023
Merged

dedup for duplicate suggestions #118057

merged 1 commit into from
Dec 9, 2023

Conversation

bvanjoi
Copy link
Contributor

@bvanjoi bvanjoi commented Nov 19, 2023

Fixes #118048

An easy fix.

@rustbot
Copy link
Collaborator

rustbot commented Nov 19, 2023

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 19, 2023
LL | }
LL | }
LL |
LL ~ foo!(T);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

previously it displayed here as foo!(TT).

@bvanjoi
Copy link
Contributor Author

bvanjoi commented Nov 19, 2023

r? @compiler-errors

@rustbot rustbot assigned compiler-errors and unassigned davidtwco Nov 19, 2023
@cjgillot
Copy link
Contributor

What about a more permanent solution in the diagnostics infra?

@bvanjoi
Copy link
Contributor Author

bvanjoi commented Nov 20, 2023

Yes, I think that's sound advice. It can help us eliminate duplicate suggestions.

@bvanjoi bvanjoi changed the title dedup for placeholder typs dedup for duplicate suggestions Nov 20, 2023
| ^
| |
| not allowed in type signatures
| not allowed in type signatures
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's worth noting that for this case, there's a duplicate label generated by placeholder_types. As we did previously, the method to eliminate the duplication is to add placeholder_types.dedup().

applicability: Applicability,
style: SuggestionStyle,
) -> &mut Self {
suggestion.dedup();

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we dedup after sorting? In case we have [(first span, first stuff), (second span, second stuff), (first span, first stuff)].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great tips! Before this, I always assumed that Vec::dedup was equivalent to Vec::from(HashSet::from(vector)).

Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not sort suggestions twice. They are already sorted by span a few lines below.

@cjgillot
Copy link
Contributor

cjgillot commented Dec 9, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Dec 9, 2023

📌 Commit 199098b has been approved by cjgillot

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2023
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#117953 (Add more SIMD platform-intrinsics)
 - rust-lang#118057 (dedup for duplicate suggestions)
 - rust-lang#118638 (More `rustc_mir_dataflow` cleanups)
 - rust-lang#118702 (Strengthen well known check-cfg names and values test)
 - rust-lang#118734 (Unescaping cleanups)
 - rust-lang#118766 (Lower some forgotten spans)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0865eef into rust-lang:master Dec 9, 2023
11 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 9, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2023
Rollup merge of rust-lang#118057 - bvanjoi:fix-118048, r=cjgillot

dedup for duplicate suggestions

Fixes rust-lang#118048

An easy fix.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 15, 2024
…mpiler-errors

Fix the dedup error because of spans from suggestion

Fixes rust-lang#116502

I believe this kind of issue is supposed resolved by rust-lang#118057, but the `==` in `span` respect syntax context, here we should only care that they point to the same bytes of source text, so should use `source_equal`.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 15, 2024
Rollup merge of rust-lang#125135 - chenyukang:yukang-fix-116502, r=compiler-errors

Fix the dedup error because of spans from suggestion

Fixes rust-lang#116502

I believe this kind of issue is supposed resolved by rust-lang#118057, but the `==` in `span` respect syntax context, here we should only care that they point to the same bytes of source text, so should use `source_equal`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E0121 overlapping spans / bad placeholder
6 participants