Skip to content

remove intrinsics::drop_in_place #140151

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

remove intrinsics::drop_in_place #140151

wants to merge 1 commit into from

Conversation

RalfJung
Copy link
Member

This was only ever accidentally stable, and has been marked as deprecated since Rust 1.52, released almost 4 years ago. We've removed the old serialization derives, maybe we can remove this one as well?

As suggested by @jhpratt, let's see what crater says for this one.

@rustbot
Copy link
Collaborator

rustbot commented Apr 22, 2025

r? @thomcc

rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 22, 2025

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

@RalfJung RalfJung force-pushed the drop_in_place-is-not-an-intrinsic branch from b6e062c to f8b0768 Compare April 22, 2025 07:41
@RalfJung
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 22, 2025
@bors
Auto merge of rust-lang#140151 - RalfJung:drop_in_place-is-not-an-int…
5c9519e 
…rinsic, r=<try>

remove intrinsics::drop_in_place

This was only ever accidentally stable, and has been marked as deprecated since Rust 1.52, released almost 4 years ago. We've removed the old serialization `derive`s, maybe we can remove this one as well?

As suggested by `@jhpratt,` let's see what crater says for this one.
@bors
Copy link
Collaborator

bors commented Apr 22, 2025

⌛ Trying commit f8b0768 with merge 5c9519e...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@RalfJung RalfJung force-pushed the drop_in_place-is-not-an-intrinsic branch from f8b0768 to dd39c2f Compare April 22, 2025 09:08
@RalfJung
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 22, 2025
@bors
Auto merge of rust-lang#140151 - RalfJung:drop_in_place-is-not-an-int…
1bd50d4 
…rinsic, r=<try>

remove intrinsics::drop_in_place

This was only ever accidentally stable, and has been marked as deprecated since Rust 1.52, released almost 4 years ago. We've removed the old serialization `derive`s, maybe we can remove this one as well?

As suggested by `@jhpratt,` let's see what crater says for this one.
@bors
Copy link
Collaborator

bors commented Apr 22, 2025

⌛ Trying commit dd39c2f with merge 1bd50d4...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@RalfJung
Copy link
Member Author

More tests need blessing if we want to land this, but that doesn't block a crater run.

@bors
Copy link
Collaborator

bors commented Apr 22, 2025

☀️ Try build successful - checks-actions
Build commit: 1bd50d4 (1bd50d4315eb15970d395b6be5823f9384707693)

@RalfJung
Copy link
Member Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-140151 created and queued.
🤖 Automatically detected try build 1bd50d4
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2025
@RalfJung RalfJung force-pushed the drop_in_place-is-not-an-intrinsic branch from dd39c2f to e7efab9 Compare April 22, 2025 12:06
@craterbot
Copy link
Collaborator

🚧 Experiment pr-140151 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-140151 is completed!
📊 4 regressed and 5 fixed (619040 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Apr 24, 2025
@lcnr
Copy link
Contributor

lcnr commented Apr 24, 2025

all regressions look unrelated

@RalfJung
Copy link
Member Author

Yeah, indeed.

So this becomes a question for @rust-lang/libs-api then I guess -- do we want to remove this accidentally-stable function that has been deprecated for years?

@RalfJung RalfJung added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Apr 24, 2025
@Amanieu
Copy link
Member

Amanieu commented Apr 29, 2025

We discussed this in the @rust-lang/libs-api meeting and there was no opposition to removing it.

@rfcbot merge

@rfcbot

This comment was marked as outdated.

Sign in to view
@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 29, 2025
@Amanieu Amanieu added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 29, 2025
@Amanieu

This comment was marked as outdated.

Sign in to view
@rfcbot

This comment was marked as outdated.

Sign in to view
@Amanieu
Copy link
Member

Amanieu commented Apr 29, 2025

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 29, 2025 •
edited by joshtriplett
Loading

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 29, 2025
@RalfJung RalfJung added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 29, 2025
@rustbot rustbot mentioned this pull request Apr 29, 2025
Open
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Apr 29, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 29, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thomcc thomcc

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@RalfJung @rustbot @bors @rust-log-analyzer @craterbot @lcnr @Amanieu @rfcbot @thomcc