Skip to content
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

Remove raw_pointer_deriving lint #14615

Closed
brson opened this issue Jun 3, 2014 · 7 comments
Closed

Remove raw_pointer_deriving lint #14615

brson opened this issue Jun 3, 2014 · 7 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@brson
Copy link
Contributor

brson commented Jun 3, 2014

After examination, it doesn't appear that deriving raw pointers presents any kind of safety problem (no derivings dereference unsafe pointers). Most people seem to believe that the original justification presented in #13032 isn't sufficient.

@brson brson added the E-easy label Jun 3, 2014
@emberian
Copy link
Member

emberian commented Jun 3, 2014

+1

@emberian
Copy link
Member

emberian commented Jun 3, 2014

I think that each derivation can emit a more specific warning when it handles a raw pointer in a possibly unexpected way (for example, Eq or Ord can say "ignoring raw pointer field"

@huonw
Copy link
Member

huonw commented Jun 3, 2014

I'm not in favour of removing it... how many instances of #[deriving] with a raw pointer give the desired implementation? In any case, I would think the lint should be hit rather rarely.

Also, this is a safety problem, e.g. a derived Clone can duplicate a pointer without duplicating the allocation, leading to use-after-free and/or double frees (the code example in #13032 demonstrates this). Most "interesting" smart pointers (i.e. not just simulating a shared & reference) will be incorrect with a derived Clone.

@frewsxcv
Copy link
Member

Worth pointing out that raw_pointer_deriving was recently renamed to raw_pointer_derive #20511

@kmcallister
Copy link
Contributor

I'm not in favour of removing it... how many instances of #[deriving] with a raw pointer give the desired implementation?

Most of them, if you understand raw pointers to be plain old data. For example, a struct which wraps an FFI object could #[derive(PartialEq, Eq, PartialOrd, Ord, Hash)] to be usable as a collection key. We treat the raw pointer as an opaque ID for the FFI object. Of course, Eq won't compare the structural fields behind that pointer (which are defined in foreign code anyway), and the ID is only unique over currently-existing objects, i.e. addresses can be reused. Anyone using raw pointers should already understand this. If someone is badly confused about what raw pointers are, and derives Eq expecting structural equality, then I don't think this lint will help them much anyway.

I would think the lint should be hit rather rarely.

People hit it all the time and get confused on IRC because it's warning them that they're getting exactly the behavior they expect. We have the lint disabled in 5 Servo modules, as well.

Eq or Ord can say "ignoring raw pointer field"

But they aren't ignored, they're just treated as plain old data, which (to me anyway) is the only thing you might possibly expect. People should know that raw pointers may be NULL, may point to structures whose layout/size is not known in Rust, and can't be dereferenced willy-nilly from safe code. If you're confused on that then you need to read the unsafe programming chapter, not a single lint message.

@tbu-
Copy link
Contributor

tbu- commented Nov 18, 2015

#[deriving(Clone)] on raw pointers is probably an error though. If there's an object coupled to it, this will likely lead to use after free.

@tbu-
Copy link
Contributor

tbu- commented Nov 18, 2015

Also, this can probably be closed, because #29882 already landed.

bvssvni added a commit to bvssvni/rust-sdl2_mixer that referenced this issue Dec 1, 2015
- Bumped to 0.10.0
- Removed `allow(raw_pointer_derive)`, see discussion
rust-lang/rust#14615
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

7 participants