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

Drop finalizer on newtype structs does not work #6090

Closed
brson opened this issue Apr 27, 2013 · 4 comments
Closed

Drop finalizer on newtype structs does not work #6090

brson opened this issue Apr 27, 2013 · 4 comments
Labels
A-codegen Area: Code generation

Comments

@brson
Copy link
Contributor

brson commented Apr 27, 2013

This will leak the ptr. Valgrind will complain.

use core::libc::{c_void, malloc, free};

struct SmartPtr(*c_void);

impl Drop for SmartPtr {
    fn finalize(&self) {
        unsafe {
            error!("end ptr: %x", **self as uint);
            free(**self);
        }
    }
}

fn main() {
    unsafe {
        let junk = malloc(4);
        error!("start ptr: %x", junk as uint);
        let _junkbox = SmartPtr(junk);
    }
}
@brson
Copy link
Contributor Author

brson commented Apr 27, 2013

Nominating feature complete.

@luqmana
Copy link
Member

luqmana commented Apr 28, 2013

This seems to be because the drop flag isn't set for newtype structs meaning the branch which calls the actual destructor in trans::glue doesn't get executed. The function that sets the drop flag appropriately is middle::trans::adt::trans_start_init but it doesn't seem to be called.

@graydon
Copy link
Contributor

graydon commented May 2, 2013

accepted for production ready

@luqmana
Copy link
Member

luqmana commented May 2, 2013

Fixed per #6125.

@luqmana luqmana closed this as completed May 2, 2013
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 9, 2020
…ts, r=yaahc

lints: clarify rc_buffer and add caveats

This didn't display some types properly in the docs due the lack of code formatting.

Also, refs for the caveat:
rust-lang/rust-clippy#6044 (comment)
http-rs/surf#242

Fwiw I can't get `cargo test` to run, even on nightly. I get:
```
error[E0463]: can't find crate for `rustc_ast`
```

*Please keep the line below*
changelog: none, nightly
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 9, 2020
Downgrade rc_buffer to restriction

I think Arc\<Vec\<T\>\> and Arc\<String\> and similar are a totally reasonable data structure, as observed by others in the comments on [rust-lang#6044](rust-lang/rust-clippy#6044 (comment)) as well. Doing `Arc::make_mut(&mut self.vec).push(...)` or `Arc::make_mut(&mut self.string).push_str("...")` is a terrific and well performing copy-on-write pattern. Linting this with an enabled-by-default <kbd>performance</kbd> lint strikes me as an unacceptable false positive balance.

As of rust-lang#6090 the documentation of this lint now contains:

> **Known problems:** This pattern can be desirable ...

which should indicate that we shouldn't be linting against correct, reasonable, well-performing patterns with an enabled-by-default lint.

Mentioning rust-lang#6044, rust-lang#6090.
r? `@yaahc,` who reviewed the lint.

---

changelog: Remove rc_buffer from default set of enabled lints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation
Projects
None yet
Development

No branches or pull requests

3 participants