Skip to content

Extend dropping_{references,copy_types} lints to drop_in_place - #160229

Open
Urgau wants to merge 3 commits into
rust-lang:mainfrom
Urgau:extend-drop_useless-drop-in-place
Open

Extend dropping_{references,copy_types} lints to drop_in_place#160229
Urgau wants to merge 3 commits into
rust-lang:mainfrom
Urgau:extend-drop_useless-drop-in-place

Conversation

@Urgau

@Urgau Urgau commented Jul 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

This PR extends the dropping_{references,copy_types} lints to also check for calls to std::ptr::drop_in_place and <*mut _>::drop_in_place.

It also extends the undropped_manually_drops lint.

Fixes #160127
cc @theemathas

@rustbot rustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 30, 2026
@rustbot

rustbot commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

r? @mu001999

rustbot has assigned @mu001999.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 16 candidates

@Urgau Urgau added the I-lang-nominated Nominated for discussion during a lang team meeting. label Jul 30, 2026
@rustbot

rustbot commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

@Urgau Urgau mentioned this pull request Jul 30, 2026
Closed
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Urgau
Urgau force-pushed the extend-drop_useless-drop-in-place branch from 1380cb0 to 27c8128 Compare July 30, 2026 18:49
@mu001999 mu001999 added S-waiting-on-t-lang Status: Awaiting decision from T-lang and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2026
@traviscross traviscross added I-lang-radar Items that are on lang's radar and will need eventual work or consideration. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang T-lang Relevant to the language team labels Aug 5, 2026
@traviscross

traviscross commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Makes sense to me. Thanks @Urgau.

@rfcbot fcp merge lang

@rust-rfcbot

rust-rfcbot commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@traviscross 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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-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 Aug 5, 2026
oli-obk
oli-obk approved these changes Aug 5, 2026
View reviewed changes

@oli-obk oli-obk left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

miri test changes lgtm

View changes since this review

mu001999
mu001999 reviewed Aug 5, 2026
View reviewed changes
Comment thread compiler/rustc_lint/src/lints.rs Outdated
mu001999
mu001999 reviewed Aug 5, 2026
View reviewed changes
Comment thread compiler/rustc_lint/src/drop_forget_useless.rs Outdated
mu001999
mu001999 reviewed Aug 5, 2026
View reviewed changes
Comment thread compiler/rustc_lint/src/drop_forget_useless.rs Outdated
mu001999
mu001999 reviewed Aug 5, 2026
View reviewed changes
Comment thread compiler/rustc_lint/src/drop_forget_useless.rs Outdated
scottmcm
scottmcm reviewed Aug 5, 2026
View reviewed changes
Comment thread compiler/rustc_lint/src/drop_forget_useless.rs Outdated
/// The `dropping_references` lint checks for calls to `std::mem::drop` with a reference
/// instead of an owned value.
/// The `dropping_references` lint checks for calls to `std::mem::drop`
/// and `std::ptr::drop_in_place` with a reference instead of an owned value.

@scottmcm scottmcm Aug 5, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: might want to rephrase this because passing a reference to drop_in_place is an ordinary and expected thing to do. Maybe "where the dropped type is a reference" or something?

View changes since the review

@Urgau Urgau Aug 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Went with your proposed wording.

@scottmcm

scottmcm commented Aug 5, 2026

Copy link
Copy Markdown
Member

Personally I think this doesn't need FCP. The intent of this lint is to catch trying to drop things where that doesn't do anything, and how that drop is spelled doesn't matter to me.

@rfcbot reviewed

@Urgau
Urgau force-pushed the extend-drop_useless-drop-in-place branch from 27c8128 to 5d882cb Compare August 5, 2026 17:23
@rustbot

rustbot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@scottmcm

scottmcm commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Pondering: thinking about the intent I described above made me think of unions which have a "things that aren't dropped" rule for what can go in them. So should we be linting on drop_in_place::<ManuallyDrop<_>> too, for example?

@Urgau

Urgau commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

So should we be linting on drop_in_place::<ManuallyDrop<_>> too, for example?

We could, we even already have the lint for it undropped_manually_drops.

@Urgau

Urgau commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

As suggested I've also extended the undropped_manually_drops lint to also take into account drop_in_place::<ManuallyDrop<_>>.

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

Reviewers

@scottmcm scottmcm scottmcm left review comments
@mu001999 mu001999 mu001999 left review comments
@oli-obk oli-obk oli-obk approved these changes

Assignees

@mu001999 mu001999

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. I-lang-nominated Nominated for discussion during a lang team meeting. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-t-lang Status: Awaiting decision from T-lang T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team T-libs Relevant to the library 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.

Feature request: Lint on drop_in_place on types with no destructor

8 participants

@Urgau @rustbot @rust-log-analyzer @traviscross @rust-rfcbot @scottmcm @oli-obk @mu001999