Do not dereference uninhabited types refs in Clone implementations

[samueltardieu]

A reference to an uninhabited type should never be dereferenced: this is UB. Copy should not be implemented on such a type, and an upcoming Clippy lint (uninhabited_reference) may flag such
dereferences as suspicious.

Since those types are not structs, they do not need to get Copy and Clone implementations. A missing! macro limits code duplication.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[JohnTitor]

Thanks! @bors r+

[bors]

📌 Commit a693257 has been approved by JohnTitor

It is now in the queue for this repository.

[bors]

⌛ Testing commit a693257 with merge b4686c6...

[bors]

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing b4686c6 to main...

[Nadrieril]

A reference to an uninhabited type should never be dereferenced: this is UB

This is not quite correct: if the &! is safe, then the code can't be reached so dereferencing it is ok. In other words, the Clone impl is sound because it can assume that &self obeys its safety invariant (which means that clone will never be called). A different way to see why this is not UB is that this does not require an unsafe {} block.

Now, this Clone impl could trigger UB if it was passed a &self that breaks the safety invariant. But that's the responsibility of the caller, not the impl.

[samueltardieu]

A reference to an uninhabited type should never be dereferenced: this is UB

This is not quite correct: if the &! is safe, then the code can't be reached so dereferencing it is ok. In other words, the Clone impl is sound because it can assume that &self obeys its safety invariant (which means that clone will never be called). A different way to see why this is not UB is that this does not require an unsafe {} block.

Now, this Clone impl could trigger UB if it was passed a &self that breaks the safety invariant. But that's the responsibility of the caller, not the impl.

I agree. However, in the context of libc which necessarily deals with a lot of unsafe, obtaining such a reference (for example a &mut FILE from a *mut FILE) is quite easy and a user could shoot themselves in the foot by calling .clone() on them. This is why I think that removing those Clone implementations was the best think to do anyway.

[Nadrieril]

Oh I see! Then I totally agree, having these impls is a big footgun. Well the whole thing is quite footgunny but that too