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 headers from ~@T and ~[@T] allocations #8035

Closed
wants to merge 4 commits into from

Conversation

alexcrichton
Copy link
Member

I don't believe that these are needed in cleanup after all. Because ~T has the same semantics in the compiler regardless of what T is, then there are two cases for the destruction of ~T. One is before the program exits (which is normal), the other is when the program exits during annihilation. It's possible for ~T to be located inside an @ cycle.

Even though there were headers on ~[@t], the value itself was ignored because each of the elements were linked together themselves. Whatever box contains the ~ allocation will have its destructor run when annihilation comes around, subsequently freeing the ~ allocation.

This builds just fine on OSX, and I'm having other unrelated valgrind issues on linux to ensure that there's no leaks, but I'm sure bors will complain at me if there are any.

By the way, thanks to @thestinger's awesome work removing headers previously, this was a super-easy change!

@thestinger
Copy link
Contributor

I think managed-unique does still need to be managed by the local heap, for the garbage collector to land. The headers can definitely be removed since they're unused, but it might be tricky to keep them using local_malloc and local_free without having the same representation as managed boxes.

cc @graydon

@alexcrichton
Copy link
Member Author

@graydon agrees with @thestinger, the headers will be needed for the pending gc and its future iterations, so there's no real gains removing them now only to add them back later.

@thestinger
Copy link
Contributor

@alexcrichton: I think it's more that local_malloc is needed rather than the headers themselves, but I'm not entirely clear on that.

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 13, 2022
`README`: document that Clippy may change codegen

Currently, Clippy does not guarantee the same codegen will be produced.
Therefore, it should not be used as an universal replacement for `rustc`.

See rust-lang/rust-clippy#8035.

Signed-off-by: Miguel Ojeda <[email protected]>

fixes rust-lang#8035
changelog: document that Clippy may change codegen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants