c_vec: Modernize #10736
c_vec: Modernize #10736
Conversation
| * | ||
| * Fails if `ofs` is greater or equal to the length of the vector | ||
| */ | ||
| pub unsafe fn get(&self, ofs: uint) -> T { |
There was a problem hiding this comment.
This could probably be:
pub unsafe fn get<'a>(&'a self, idx: uint) -> &'a T { ... }and avoid the compulsory .clone?
There was a problem hiding this comment.
Hmm, true. Will change, thanks!
|
Fixed, ready for next round of reviews. |
|
I'm personally not a fan of moving the unsafe-ness to the setters/getters instead of the constructor. This class is supposed to be a safe interface around dealing with C vectors. The only unsafe operation is constructing a class saying that "this unsafe memory is actually valid". Once you have made that statement, you should no longer have to make the statement that you're willing to accept unsafe code. |
|
Thanks for your remarks everyone. Rewritten parts of it, rebased and squashed. See commit message for overview of the current version :) |
Generally use more modern constructs (such as using `CVec::new()` as constructor and move to more method usage). Potentially controversial changes: * change `get()` to return a reference instead of cloning * remove `set()`, add `get_mut()` instead * add an `unwrap()` method that destroys the CVec without running any associated destructor
|
Pushed a new version |
Controversial change: make the constructors safe, but instead mark the setters and getters as unsafe. This follows the tradition that construction of *T is safe, but the access is unsafe instead.
initial clippy::ref_pattern implementation This implements a new lint that discourages the use of the `ref` keyword as outlined in rust-lang#9010. I think there are still some things to improve about this lint, but I need some feedback before I can implement those. - [x] ~~Maybe it's desirable that a quick fix is listed too, instead of a generic `avoid using the ref keyword` lint.~~ - [x] `let ref x = y` already has a lint (`clippy::toplevel_ref_arg`). This implementation will report this too, so you get two lints for the same issue, which is not great. I don't really know a way around this though. - [X] The dogfood test is currently failing locally, though I ran `cargo clippy -- -D clippy::all -D clippy::pedantic` and got no output, so I'm not sure why this is. Any help with these would be greatly appreciated. fixes rust-lang#9010 changelog: [`ref_pattern`]: newly added lint
Controversial change: make the constructors safe, but instead mark the
setters and getters as unsafe. This follows the tradition that construction
of *T is safe, but the access is unsafe instead.