Skip to content

transmute_undefined_repr is wrong, transmute does not require a defined layout #8417

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

Closed
dtolnay opened this issue Feb 12, 2022 · 1 comment · Fixed by #8425
Closed

transmute_undefined_repr is wrong, transmute does not require a defined layout #8417

dtolnay opened this issue Feb 12, 2022 · 1 comment · Fixed by #8425
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@dtolnay
Copy link
Member

dtolnay commented Feb 12, 2022

Summary

The description of transmute_undefined_repr says:

What it does
Checks for transmutes either to or from a type which does not have a defined representation.

Why is this bad?
The results of such a transmute are not defined.

I think this is not correct. Transmute does not require either side to have a defined representation. It only requires that both sides have the same representation, which can be guaranteed without defining what that representation is.

Lint Name

transmute_undefined_repr

Reproducer

#[repr(C)]
struct Wrapper(String);

fn main() {
    let string = String::new();
    let _ = unsafe { std::mem::transmute::<String, Wrapper>(string) };
}
$ cargo clippy

error: transmute from `std::string::String` which has an undefined layout
 --> src/main.rs:6:22
  |
6 |     let _ = unsafe { std::mem::transmute::<String, Wrapper>(string) };
  |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[deny(clippy::transmute_undefined_repr)]` on by default
  = note: the contained type `std::vec::Vec<u8>` has an undefined layout
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#transmute_undefined_repr

Version

rustc 1.60.0-nightly (e789f3a3a 2022-02-11)
binary: rustc
commit-hash: e789f3a3a3d96ebf99b7bbd95011527e5be32a11
commit-date: 2022-02-11
host: x86_64-unknown-linux-gnu
release: 1.60.0-nightly
LLVM version: 13.0.0

Additional Labels

No response

@dtolnay dtolnay added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Feb 12, 2022
bors added a commit that referenced this issue Feb 12, 2022
Downgrade transmute_undefined_repr to nursery

Reason: #8417. I am skeptical of this lint but maybe there is a narrower subset of types on which it is useful, so keeping it for now but moving to nursery for further development.

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: Remove [`transmute_undefined_repr`] from default set of enabled lints
@Jarcho
Copy link
Contributor

Jarcho commented Feb 12, 2022

Hilariously it works if you remove #[repr(C)]. I can fix it later today.

bors added a commit that referenced this issue Feb 14, 2022
Fix `transmute_undefined_repr` with single field `#[repr(C)]` structs

Fixes: #8417

The description has also been made more precise.

changelog: Fix `transmute_undefined_repr` with single field `#[repr(C)]` structs
changelog: Move `transmute_undefined_repr` back to `correctness`
@bors bors closed this as completed in 4931cab Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants