This is a similar issue to #2946.
Recently it was discovered that a change recommended by the trivially_copy_pass_by_ref lint had started causing undefined behavior in Rust-SDL2: Rust-SDL2/rust-sdl2#1020.
This is the problematic change:
impl Point {
// ...
- pub fn raw(&self) -> *const sys::SDL_Point {
+ pub fn raw(self) -> *const sys::SDL_Point {
self.raw
}
}
Point is a simple wrapper around an owned sys::SDL_Point, which itself is just a simple FFI-compatible data structure. Since Point implements Copy, the trivially_copy_pass_by_ref lint was recommended on Point::raw - even though making that change causes Point::raw to return a pointer to an immediately-dropped copy of one of its own fields.
Here's a minimal reproduction of the issue. The Point::raw method correctly produces a consistent pointer to the internal RawPoint, whereas the Point::raw_linted method produces a different pointer on each call.
#![forbid(clippy::pedantic)]
#[derive(Clone, Copy)]
struct RawPoint {
pub x: u8,
}
#[derive(Clone, Copy)]
struct Point {
pub raw: RawPoint,
}
impl Point {
pub fn raw(&self) -> *const RawPoint {
&self.raw
}
pub fn raw_linted(self) -> *const RawPoint {
&self.raw
}
}
fn main() {
let p = Point { raw: RawPoint { x: 10 } };
// This passes
assert_eq!(p.raw(), p.raw());
// This fails
assert_eq!(p.raw_linted(), p.raw_linted());
}
This would be a bit more challenging to decide than the solution in #2951. Pointers could be present as arbitrarily nested fields of a returned struct, which would not itself be a pointer. Further, pointers could be manipulated by long-lasting side effects (e.g. sent over a channel) during a function without actually ever being returned. Arguably it is the responsibility of the author of the unsafe dereference site to guarantee that the pointer remains valid, but it's still unreasonable to recommend this change in these cases.
Meta
cargo clippy -V: clippy 0.0.212 (346aec9 2020-07-11)
rustc -Vv:
rustc 1.46.0-nightly (346aec9b0 2020-07-11)
binary: rustc
commit-hash: 346aec9b02f3c74f3fce97fd6bda24709d220e49
commit-date: 2020-07-11
host: x86_64-unknown-linux-gnu
release: 1.46.0-nightly
LLVM version: 10.0
This is a similar issue to #2946.
Recently it was discovered that a change recommended by the
trivially_copy_pass_by_reflint had started causing undefined behavior in Rust-SDL2: Rust-SDL2/rust-sdl2#1020.This is the problematic change:
impl Point { // ... - pub fn raw(&self) -> *const sys::SDL_Point { + pub fn raw(self) -> *const sys::SDL_Point { self.raw } }Pointis a simple wrapper around an ownedsys::SDL_Point, which itself is just a simple FFI-compatible data structure. SincePointimplementsCopy, thetrivially_copy_pass_by_reflint was recommended onPoint::raw- even though making that change causesPoint::rawto return a pointer to an immediately-dropped copy of one of its own fields.Here's a minimal reproduction of the issue. The
Point::rawmethod correctly produces a consistent pointer to the internalRawPoint, whereas thePoint::raw_lintedmethod produces a different pointer on each call.This would be a bit more challenging to decide than the solution in #2951. Pointers could be present as arbitrarily nested fields of a returned struct, which would not itself be a pointer. Further, pointers could be manipulated by long-lasting side effects (e.g. sent over a channel) during a function without actually ever being returned. Arguably it is the responsibility of the author of the
unsafedereference site to guarantee that the pointer remains valid, but it's still unreasonable to recommend this change in these cases.Meta
cargo clippy -V: clippy 0.0.212 (346aec9 2020-07-11)rustc -Vv: