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.

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

Insignificant destructors rfc 2229 #84152

Merged
merged 1 commit into from
May 15, 2021
Merged

Insignificant destructors rfc 2229 #84152

merged 1 commit into from
May 15, 2021

Conversation

null-sleep
Copy link
Contributor

@null-sleep null-sleep commented Apr 13, 2021 •
edited
Loading

  • Adds new attribute rustc_insignificant_dtor to annotate the drop method.
  • Adds a query to check if a type has a significant drop.
  • Updates closure analysis to check for significant drops rather than just drop.

A type marked with the attribute rustc_insignificant_dtor is considered to not be significant. A drop is significant if it is implemented by the user or does anything that will have any observable behavior (other than freeing up memory).

rust-lang/project-rfc-2229#35

@rust-highfive
Copy link
Collaborator

r? @estebank

(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 Apr 13, 2021
@null-sleep
Copy link
Contributor Author

r? @ghost

@null-sleep
Copy link
Contributor Author

Testing with CI. Local build kept ommitting version = 3 from Cargo.lock

@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 13, 2021
@crlf0710 crlf0710 added S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2021
@crlf0710
Copy link
Member

crlf0710 commented May 1, 2021

Triage: switching to S-experimental since reviewer=ghost.

@null-sleep null-sleep force-pushed the insignificant_dtor branch from 532ebb9 to eb794ce Compare May 4, 2021 03:05
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@null-sleep null-sleep force-pushed the insignificant_dtor branch 2 times, most recently from 5a39524 to f37ed61 Compare May 4, 2021 05:59
arora-aman
arora-aman reviewed May 4, 2021
View reviewed changes
Copy link
Member

@arora-aman arora-aman left a comment

Choose a reason for hiding this comment

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

nits, LGTM otherwise

compiler/rustc_middle/src/query/mod.rs Outdated Show resolved Hide resolved
src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.fixed Outdated Show resolved Hide resolved
src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_no_migrations.rs Outdated Show resolved Hide resolved
src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.rs Show resolved Hide resolved
@null-sleep null-sleep changed the title add new attribute rustc_insignificant_dtor and a query to check if a … Insignificant destructors rfc 2229 May 4, 2021
@null-sleep null-sleep force-pushed the insignificant_dtor branch from f37ed61 to 2d12033 Compare May 4, 2021 22:56
@null-sleep
Copy link
Contributor Author

r? @nikomatsakis

nikomatsakis
nikomatsakis requested changes May 5, 2021
View reviewed changes
compiler/rustc_middle/src/query/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/query/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/util.rs Outdated Show resolved Hide resolved
compiler/rustc_ty_utils/src/needs_drop.rs Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented May 6, 2021

☔ The latest upstream changes (presumably #84982) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 6, 2021
@null-sleep null-sleep force-pushed the insignificant_dtor branch from 2d12033 to 9a0a8aa Compare May 9, 2021 21:54
nikomatsakis
nikomatsakis requested changes May 10, 2021
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Looks great! A few more nits.

compiler/rustc_ty_utils/src/needs_drop.rs Outdated Show resolved Hide resolved
compiler/rustc_ty_utils/src/needs_drop.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented May 12, 2021

☔ The latest upstream changes (presumably #84730) made this pull request unmergeable. Please resolve the merge conflicts.

@null-sleep null-sleep force-pushed the insignificant_dtor branch from 9a0a8aa to a7e1cec Compare May 15, 2021 02:57
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 15, 2021

📌 Commit a7e1cec has been approved by nikomatsakis

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 15, 2021
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 15, 2021
@bors
Copy link
Contributor

bors commented May 15, 2021

⌛ Testing commit a7e1cec with merge b439be0...

@bors
Copy link
Contributor

bors commented May 15, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing b439be0 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 15, 2021
@bors bors merged commit b439be0 into rust-lang:master May 15, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 15, 2021
@bors bors mentioned this pull request May 15, 2021
Merged
@crlf0710 crlf0710 removed the S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. label May 23, 2021
@m-ou-se m-ou-se mentioned this pull request Sep 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arora-aman arora-aman arora-aman left review comments

@nikomatsakis nikomatsakis nikomatsakis requested changes

Assignees

@nikomatsakis nikomatsakis

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
10 participants
@null-sleep @rust-highfive @crlf0710 @rust-log-analyzer @bors @nikomatsakis @arora-aman @estebank @jyn514 @rustbot