clippy fix: non_canonical_clone_impl

[hkBst]

Fixes:

warning: non-canonical implementation of `clone` on a `Copy` type
   --> library/core/src/clone.rs:682:33
    |
682 |           fn clone(&self) -> Self {
    |  _________________________________^
683 | |             self
684 | |         }
    | |_________^ help: change this to: `{ *self }`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_canonical_clone_impl
    = note: requested on the command line with `-W clippy::non-canonical-clone-impl`

warning: non-canonical implementation of `clone` on a `Copy` type
   --> library/core/src/convert/mod.rs:935:35
    |
935 |       fn clone(&self) -> Infallible {
    |  ___________________________________^
936 | |         match *self {}
937 | |     }
    | |_____^ help: change this to: `{ *self }`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_canonical_clone_impl

warning: non-canonical implementation of `clone` on a `Copy` type
   --> library/core/src/marker.rs:856:29
    |
856 |       fn clone(&self) -> Self {
    |  _____________________________^
857 | |         Self
858 | |     }
    | |_____^ help: change this to: `{ *self }`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_canonical_clone_impl

[rustbot]

r? @scottmcm

rustbot has assigned @scottmcm.
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

[scottmcm]

TBH, it's not really obvious to me that this is better for all these types.

Like here having the match emphasizes that this is unreachable better than just *self does, to me -- in MIR it's the difference between _0 = copy *_1; Return and Unreachable, for example.

And for phantomdata just being _0 = PhantomData; is more obviously address-uncaring than _0 = copy *_1; is.

Do you disagree, and think these are better? Or were you just doing what clippy said?

[hkBst]

TBH, it's not really obvious to me that this is better for all these types.

That is fair (and I don't necessarily disagree), but perhaps the wrong way to view this change.

My goal is to enable a broader set of clippy lints for the compiler, specifically all of clippy::suspicious. This would have prevented bugs like #146940.

But in order to get there, all the positives need to be dealt with in some way. The possibilities for that are:

• a simple code change (like this PR proposes)
• a change to the lint category (like multiple_bound_locations lint should not be in suspicious category rust-clippy#15736)
• a change to the lint behavior (empty_loop triggers on function prototypes in core rust-clippy#15200, crate_in_macro_def lints on a macro use rust-clippy#15620)
• an allow/expect annotation.

Like here having the match emphasizes that this is unreachable better than just *self does, to me -- in MIR it's the difference between _0 = copy *_1; Return and Unreachable, for example.

My thinking was that the argument being Infallible is enough. If it is important to emphasize this, then perhaps unreachable!() would be the better implementation?

I'm intrigued about the MIR implications, but I must confess knowing almost nothing about MIR. If it is important, can you explain why?

And for phantomdata just being _0 = PhantomData; is more obviously address-uncaring than _0 = copy *_1; is.

Again, I'm intrigued, but I'm not sure what you're getting at here, besides that you think this decreases the quality of the MIR.

Do you disagree, and think these are better?

I think that these changes are largely neutral by themselves, but they are a small step towards an end goal which does have positive value. I'm happy to discuss alternative ways of dealing with these.

Or were you just doing what clippy said?

I hope I have addressed this.