Adt copy suggestions #94375

WaffleLapkin · 2022-02-25T20:49:21Z

Previously we've only suggested adding Copy bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type

Can be copy
All predicates that need to be satisfied for that are based on type params

i.e. we will suggest T: Copy for Option<T>, but won't suggest anything for Option<String>.

An example:

fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) {
    (t, t)
}

New error (current compiler doesn't provide help:):

error[E0382]: use of moved value: `t`
 --> t.rs:2:9
  |
1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) {
  |                 - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait
2 |     (t, t)
  |      -  ^ value used here after move
  |      |
  |      value moved here
  |
help: consider restricting type parameter `T`
  |
1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) {
  |               ++++++

Fixes #93623
r? @estebank
@rustbot label +A-diagnostics +A-suggestion-diagnostics +C-enhancement

I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

estebank

I need to do another pass, only skimmed, but it seems good!

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

estebank · 2022-02-26T15:35:59Z

src/test/ui/moves/use_of_moved_value_copy_suggestions.stderr

+help: consider further restricting type parameter `T`
+   |
+LL |     T: B, T: Trait, T: Copy
+   |         ~~~~~~~~~~~~~~~~~~~
+
+error[E0382]: use of moved value: `t`
+  --> $DIR/use_of_moved_value_copy_suggestions.rs:57:9
+   |
+LL | fn duplicate_custom_4<T: A>(t: S<T>) -> (S<T>, S<T>)
+   |                             - move occurs because `t` has type `S<T>`, which does not implement the `Copy` trait
+...
+LL |     (t, t)
+   |      -  ^ value used here after move
+   |      |
+   |      value moved here
+   |
+help: consider further restricting this bound
+   |
+LL |     T: B + Trait + Copy,
+   |          ++++++++++++++


Interesting that these two suggestions are different format but for the same thing.

It tries to preserve layout of bounds. It actually currently works the same way in simpler cases (when type is type param), though I couldn't find any tests for this: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3c458e11e88b2d22e32e6ebd0a3efae1

src/test/ui/moves/use_of_moved_value_copy_suggestions.rs

estebank · 2022-02-28T22:37:38Z

src/test/ui/moves/use_of_moved_value_copy_suggestions.stderr

+help: consider restricting type parameters
+   |
+LL | fn duplicate_tup2<A: Copy, B: Copy>(t: (A, B)) -> ((A, B), (A, B)) {
+   |                    ++++++   ++++++


Ah, re: run-rustfix I think these won't be properly applied so they'd need to be split as well to their own test file (but try it anyways, they might have fixed rustfix since I last looked).

It looks like rustfix was fixed 👀

Add `rustc_middle::ty::suggest_constraining_type_params` that suggests adding multiple constraints. `suggest_constraining_type_param` now just forwards params to this new function.

Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this commit we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. Future work: it would be nice to also suggest adding `.clone()` calls

WaffleLapkin · 2022-03-01T14:49:52Z

Made the test // run-rustfix + rebased on master. I believe this PR is ready :)

estebank · 2022-03-01T16:55:07Z

@bors r+

bors · 2022-03-01T16:55:09Z

📌 Commit f0a16b8 has been approved by estebank

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? `@estebank` `@rustbot` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? ``@estebank`` ``@rustbot`` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? ```@estebank``` ```@rustbot``` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? ````@estebank```` ````@rustbot```` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? `````@estebank````` `````@rustbot````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? ``````@estebank`````` ``````@rustbot`````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? ```````@estebank``````` ```````@rustbot``````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? ````````@estebank```````` ````````@rustbot```````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

…ebank Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes rust-lang#93623 r? `````````@estebank````````` `````````@rustbot````````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')

Rollup of 9 pull requests Successful merges: - rust-lang#92061 (update char signess for openbsd) - rust-lang#93072 (Compatible variants suggestion with desugaring) - rust-lang#93354 (Add documentation about `BorrowedFd::to_owned`.) - rust-lang#93663 (Rename `BorrowedFd::borrow_raw_fd` to `BorrowedFd::borrow_raw`.) - rust-lang#94375 (Adt copy suggestions) - rust-lang#94433 (Improve allowness of the unexpected_cfgs lint) - rust-lang#94499 (Documentation was missed when demoting Windows XP to no_std only) - rust-lang#94505 (Restore the local filter on mono item sorting) - rust-lang#94529 (Unused doc comments blocks) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…ons, r=estebank Remove redundant code from copy-suggestions Follow up to rust-lang#94375, just remove some code that is not necessary anymore. This may make the perf of such suggestions a little bit worse, but I don't think this is significant. r? `@estebank`

rust-highfive assigned estebank Feb 25, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 25, 2022

WaffleLapkin marked this pull request as draft February 25, 2022 20:49

estebank reviewed Feb 26, 2022

View reviewed changes

estebank reviewed Feb 28, 2022

View reviewed changes

estebank approved these changes Feb 28, 2022

View reviewed changes

WaffleLapkin added 4 commits March 1, 2022 13:43

Add helper function to suggest multiple constraints

4c7fb9e

Add `rustc_middle::ty::suggest_constraining_type_params` that suggests adding multiple constraints. `suggest_constraining_type_param` now just forwards params to this new function.

Add a test for Adt copy suggestions

879efa8

Use rustfix in copy suggestion test

f0a16b8

WaffleLapkin force-pushed the copy-suggestion branch from 46f80e8 to f0a16b8 Compare March 1, 2022 13:58

WaffleLapkin marked this pull request as ready for review March 1, 2022 14:48

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 Mar 1, 2022

matthiaskrgr mentioned this pull request Mar 1, 2022

Rollup of 7 pull requests #94491

Closed

Dylan-DPC mentioned this pull request Mar 2, 2022

Rollup of 8 pull requests #94520

Closed

Dylan-DPC mentioned this pull request Mar 2, 2022

Rollup of 8 pull requests #94521

Closed

Dylan-DPC mentioned this pull request Mar 2, 2022

Rollup of 7 pull requests #94525

Closed

matthiaskrgr mentioned this pull request Mar 2, 2022

Rollup of 11 pull requests #94531

Closed

matthiaskrgr mentioned this pull request Mar 2, 2022

Rollup of 10 pull requests #94532

Closed

matthiaskrgr mentioned this pull request Mar 2, 2022

Rollup of 8 pull requests #94533

Closed

matthiaskrgr mentioned this pull request Mar 2, 2022

Rollup of 9 pull requests #94535

Closed

This was referenced Mar 2, 2022

Rollup of 8 pull requests #94540

Closed

Rollup of 9 pull requests #94541

Merged

bors merged commit 7537b20 into rust-lang:master Mar 3, 2022

rustbot added this to the 1.61.0 milestone Mar 3, 2022

WaffleLapkin deleted the copy-suggestion branch March 3, 2022 09:11

compiler-errors mentioned this pull request Mar 7, 2022

nightly: ICE: borrow checker panic instead of error on borrow of moved value #94662

Closed

WaffleLapkin mentioned this pull request Mar 7, 2022

Remove redundant code from copy-suggestions #94698

Merged

WaffleLapkin mentioned this pull request Mar 19, 2022

Clone suggestions #95115

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adt copy suggestions #94375

Adt copy suggestions #94375

WaffleLapkin commented Feb 25, 2022

estebank left a comment

estebank Feb 26, 2022

WaffleLapkin Feb 27, 2022 •

edited

Loading

estebank Feb 28, 2022

WaffleLapkin Mar 1, 2022

WaffleLapkin commented Mar 1, 2022

estebank commented Mar 1, 2022

bors commented Mar 1, 2022

Adt copy suggestions #94375

Adt copy suggestions #94375

Conversation

WaffleLapkin commented Feb 25, 2022

estebank left a comment

Choose a reason for hiding this comment

estebank Feb 26, 2022

Choose a reason for hiding this comment

WaffleLapkin Feb 27, 2022 • edited Loading

Choose a reason for hiding this comment

estebank Feb 28, 2022

Choose a reason for hiding this comment

WaffleLapkin Mar 1, 2022

Choose a reason for hiding this comment

WaffleLapkin commented Mar 1, 2022

estebank commented Mar 1, 2022

bors commented Mar 1, 2022

WaffleLapkin Feb 27, 2022 •

edited

Loading