Skip to content

Conversation

gurry
Copy link
Contributor

@gurry gurry commented Jul 14, 2024

Fixes #127441

Now when we encounter something like foo(a : impl ?Sized + ?Sized), instead of suggesting removal of both bounds and leaving foo(a: impl ) behind, we suggest changing the first bound to Sized and removing the second bound, resulting in foo(a: impl Sized).

Although the issue was reported for impl trait types, it also occurred with regular param bounds. So if we encounter foo<T: ?Sized + ?Sized>(a: T) we now detect that all the bounds are ?Sized and therefore emit the suggestion to remove the entire predicate : ?Sized + ?Sized resulting in foo<T>(a: T).

Lastly, if we encounter a situation where some of the bounds are something other than ?Sized, then we emit separate removal suggestions for each ?Sized bound. E.g. if we see foo(a: impl ?Sized + Bar + ?Sized) or foo<T: ?Sized + Bar + ?Sized>(a: T) we emit suggestions such that the user will be left with foo(a : impl Bar) or foo<T: Bar>(a: T) respectively.

@rustbot
Copy link
Collaborator

rustbot commented Jul 14, 2024

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jul 14, 2024
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 24, 2024

📌 Commit e13eb37 has been approved by compiler-errors

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 Jul 24, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 24, 2024
…mpiler-errors

Fix malformed suggestion for repeated maybe unsized bounds

Fixes rust-lang#127441

Now when we encounter something like `foo(a : impl ?Sized + ?Sized)`, instead of suggesting removal of both bounds and leaving `foo(a: impl )` behind, we suggest changing the first bound to `Sized` and removing the second bound, resulting in `foo(a: impl Sized)`.

Although the issue was reported for impl trait types, it also occurred with regular param bounds. So if we encounter `foo<T: ?Sized + ?Sized>(a: T)` we now detect that all the bounds are `?Sized` and therefore emit the suggestion to remove the entire predicate `: ?Sized + ?Sized` resulting in `foo<T>(a: T)`.

Lastly, if we encounter a situation where some of the bounds are something other than `?Sized`, then we emit separate removal suggestions for each `?Sized` bound. E.g. if we see `foo(a: impl ?Sized + Bar + ?Sized)` or `foo<T: ?Sized + Bar + ?Sized>(a: T)` we emit suggestions such that the user will be left with `foo(a : impl Bar)` or `foo<T: Bar>(a: T)` respectively.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 24, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126548 (Improved clarity of documentation for std::fs::create_dir_all)
 - rust-lang#127528 (Replace ASCII control chars with Unicode Control Pictures)
 - rust-lang#127717 (Fix malformed suggestion for repeated maybe unsized bounds)
 - rust-lang#128046 (Fix some `#[cfg_attr(not(doc), repr(..))]`)
 - rust-lang#128122 (Mark `missing_fragment_specifier` as `FutureReleaseErrorReportInDeps`)
 - rust-lang#128135 (std: use duplicate thread local state in tests)
 - rust-lang#128140 (Remove Unnecessary `.as_str()` Conversions)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 24, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#122192 (Do not try to reveal hidden types when trying to prove auto-traits in the defining scope)
 - rust-lang#126042 (Implement `unsigned_signed_diff`)
 - rust-lang#126548 (Improved clarity of documentation for std::fs::create_dir_all)
 - rust-lang#127717 (Fix malformed suggestion for repeated maybe unsized bounds)
 - rust-lang#128046 (Fix some `#[cfg_attr(not(doc), repr(..))]`)
 - rust-lang#128122 (Mark `missing_fragment_specifier` as `FutureReleaseErrorReportInDeps`)
 - rust-lang#128135 (std: use duplicate thread local state in tests)
 - rust-lang#128140 (Remove Unnecessary `.as_str()` Conversions)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2ff33bb into rust-lang:master Jul 25, 2024
@rustbot rustbot added this to the 1.82.0 milestone Jul 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2024
Rollup merge of rust-lang#127717 - gurry:127441-stray-impl-sugg, r=compiler-errors

Fix malformed suggestion for repeated maybe unsized bounds

Fixes rust-lang#127441

Now when we encounter something like `foo(a : impl ?Sized + ?Sized)`, instead of suggesting removal of both bounds and leaving `foo(a: impl )` behind, we suggest changing the first bound to `Sized` and removing the second bound, resulting in `foo(a: impl Sized)`.

Although the issue was reported for impl trait types, it also occurred with regular param bounds. So if we encounter `foo<T: ?Sized + ?Sized>(a: T)` we now detect that all the bounds are `?Sized` and therefore emit the suggestion to remove the entire predicate `: ?Sized + ?Sized` resulting in `foo<T>(a: T)`.

Lastly, if we encounter a situation where some of the bounds are something other than `?Sized`, then we emit separate removal suggestions for each `?Sized` bound. E.g. if we see `foo(a: impl ?Sized + Bar + ?Sized)` or `foo<T: ?Sized + Bar + ?Sized>(a: T)` we emit suggestions such that the user will be left with `foo(a : impl Bar)` or `foo<T: Bar>(a: T)` respectively.
@gurry gurry deleted the 127441-stray-impl-sugg branch July 25, 2024 07:06
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.

rustc suggests removing all trait bounds leaving stray impl
5 participants