catch transmuting references which could be pointer casts

[oli-obk]

pub unsafe fn from_utf8_unchecked(v: &[u8]) -> &str {
    std::mem::transmute(v)
}

is an example from libcore that isn't caught by clippy. It has been fixed in libcore, but it would be nice if we caught handwritten impls of similar things

[Ryan1729]

I had a look at what the rules for pointer casts actually are. This section of the first-edition rust book seems to be the most up to date prose reference.

Here's the list of rules from the above link:

e as U is a valid pointer cast in any of the following cases:

e has type *T, U has type *U_0, and either U_0: Sized or unsize_kind(T) == unsize_kind(U_0); a ptr-ptr-cast

e has type *T and U is a numeric type, while T: Sized; ptr-addr-cast

e is an integer and U is *U_0, while U_0: Sized; addr-ptr-cast

e has type &[T; n] and U is *const T; array-ptr-cast

e is a function pointer type and U has type *T, while T: Sized; fptr-ptr-cast

e is a function pointer type and U is an integer; fptr-addr-cast

The rustc code that actually does the check seems to be this do_check method, which is not currently exposed. So, unless someone has a better idea, in order to implement this lint it seems like the checks from the list above would need to be recreated in clippy.

[oli-obk]

We can expose that function in rustc so that clippy can access it.

[Ryan1729]

That function takes a FnCtxt as a parameter, so exposing it as is, would also require exposing FnCtxt. I recently happened to see this comment complaining that FnCtxt was not exposed as well.

[oli-obk]

I'm not sure how well we can use FnCtxt outside rustc (and this late in the compliation process), but I think it's worth an experiment. This feels like a PR that could be done on rustc and immediately touch the src/tools/clippy dir instead of bouncing back and forth between clippy and rustc.

[Ryan1729]

I figured I'd give experimenting with making the relevant parts of rustc_typeck public a go. So I cloned https://github.com/rust-lang/rust, fetched the submodules, and got the version of clippy in src/tools/clippy to build. But I can't figure out how to get clippy to use the repo's version of the rustc internal libs. I'm guessing that I need to build clippy with the edited rustc, but after running rustup toolchain link myrust <folder>, cargo +myrust build in the src/tools/clippy folder gives me an error saying that it cannot find the rustc_ast crate. At this point I'm stumped.

Is trying out a change like this inside the rust repo something that has been done before? Is there some documentation on this particular workflow anywhere?

[oli-obk]

So, we've abandoned submodules for clippy, meaning that, when working on the rustc repo, you can basically treat clippy as part of rustc. You'd be making a single PR to rustc without touching the clippy repo. If that PR is accepted, then we sync the changes to the clippy repo.

To run clippy with the custom built rustc, you run ./x.py test src/tools/clippy from within the rustc repo and it will set up everything to work correctly so you don't have to worry about it. The only bummer is that any change to rustc will take around 10-20 minutes depending on your setup until you get to a testing clippy.

[Ryan1729]

Aha. I understood that I'd be making a PR to the rust repo that would be synced later, but I didn't know that I should use x.py to run clippy's tests. Thanks for clearing that up.

[Ryan1729]

I've got a branch of the rust repo where I've made some parts of rustc_typeck public, and written a new lint called transmutes_expressible_as_ptr_casts that suggests replacing, (for example) transmute::<*const [i32], *const [u16]>(p) with p as *const [u16]. While writing and testing that lint, I realized that it could, (and in some sense should,) just be part of the useless_transmute lint, but that lint has been placed back into the nursery due to a false positive. And that false positive appears to require using MIR borrowck to address properly.

So maybe it would make sense to keep the transmutes_expressible_as_ptr_casts as a separate lint for now? I wasn't sure so I figured it might be best to bring up the issue here before proceeding further.

In case it interests anyone, the parts I needed to make public are mostly shown in this commit, and the single other change was in this one.

[oli-obk]

I wasn't sure so I figured it might be best to bring up the issue here before proceeding further.

yea, keep it as a separate lint for now

[Ryan1729]

All right. The PR's here.

[rail-rain]

Since all the work on the PR (merging and synching back to Clippy) has done, I don't see any reasons to keep this issue opened. Can we close this issue?