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

Make Box drop through Drop trait #93105

Closed

Conversation

beepster4096
Copy link
Contributor

@beepster4096 beepster4096 commented Jan 20, 2022

This PR makes Box drop through its implementation of the Drop trait, instead of by being special-cased in the compiler.

Blocked on #93028

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 20, 2022
@rust-highfive
Copy link
Collaborator

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 20, 2022
@beepster4096 beepster4096 marked this pull request as ready for review January 20, 2022 05:46
@rust-log-analyzer

This comment has been minimized.

@beepster4096 beepster4096 force-pushed the delangifying_box_free branch from 763eba2 to 6059879 Compare January 20, 2022 06:03
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking alloc v0.0.0 (/checkout/library/alloc)
    Checking cfg-if v0.1.10
    Checking adler v0.2.3
    Checking rustc-demangle v0.1.21
error[E0367]: `Drop` impl requires `A: core::alloc::Allocator` but the struct it is implemented for does not
     |
     |
1176 |     A: ~const Allocator,
     |
note: the implementor must specify the same requirement
    --> library/alloc/src/boxed.rs:175:1
     |
     |
175  | / pub struct Box<
176  | |     T: ?Sized,
177  | |     #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,
178  | | >(Unique<T>, A);

For more information about this error, try `rustc --explain E0367`.
error: could not compile `alloc` due to previous error
Build completed unsuccessfully in 0:01:21

@beepster4096
Copy link
Contributor Author

Alright good, now that CI is failing in the way it should, all we have to do is wait for #93028 to merge.

@nagisa
Copy link
Member

nagisa commented Jan 22, 2022

I suspect this will run into problems with ensuring that the memory is indeed freed in case of incomplete initialization as part of the box syntax or in case of panics during drop.

Would be a good idea to verify there are tests for these behaviours and if not – to implement some.

@beepster4096
Copy link
Contributor Author

Actually, this is still going to need some compiler magic for dropping T because of some of Box's other magic.

@beepster4096
Copy link
Contributor Author

Honestly, don't think this is that useful because of that. Not gonna bother finishing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants