Add auto traits and clone trait migrations for RFC2229 #84730

roxelo · 2021-04-30T00:23:31Z

This PR

renames the existent RFC2229 migration disjoint_capture_drop_reorder to disjoint_capture_migration
add additional migrations for auto traits and clone trait

Closes rust-lang/project-rfc-2229#29
Closes rust-lang/project-rfc-2229#28

r? @nikomatsakis

arora-aman

mostly nits

compiler/rustc_typeck/src/check/upvar.rs

arora-aman · 2021-05-04T18:52:00Z

compiler/rustc_typeck/src/check/upvar.rs

+
+        let cause = ObligationCause::misc(self.tcx.hir().span(var_hir_id), self.body_id);
+
+        let clone_obligation_should_hold = tcx


nit: maybe its better to do this with a helper like (just to reduce some degree of duplication):

Check need_2229_migrations_for_trait(var_hir_id, trait)

Checks for the specific var_hir meets trait bound

Loops over all captures and see if that trait bound is met

and then this function can just do

if need_2229_migrations..(var_hir_id, tcx.lang_items().clone_trait()) { auto_trait_reasons.insert("`Clone`"); }

A helper function does seem like it would be good here.

compiler/rustc_typeck/src/check/upvar.rs

arora-aman · 2021-05-04T18:56:03Z

compiler/rustc_typeck/src/check/upvar.rs

+                drop_reorder_reason = true;
+            }
+
+            if need_some_migrations {


this can just be need need_auto_trait_migrations || need_drop_migrations

I still think it's simple to just use need_migrations. Since we do need to handle drop and auto_trait migration differently

src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.rs

nikomatsakis

Do we have a test that causes some variable to be captured for multiple reasons -- e.g., because of Send but also because of destructors?

compiler/rustc_typeck/src/check/upvar.rs

Combine all 2229 migrations under one flag name

roxelo · 2021-05-06T19:01:35Z

Yes, there are a few tests that check that a variable is being captured for multiple reasons. For instance here and here

nikomatsakis

This looks about ready to land! One nit.

nikomatsakis · 2021-05-10T08:54:22Z

compiler/rustc_typeck/src/check/upvar.rs

+
+        let cause = ObligationCause::misc(self.tcx.hir().span(var_hir_id), self.body_id);
+
+        let clone_obligation_should_hold = tcx


A helper function does seem like it would be good here.

nikomatsakis · 2021-05-12T08:33:13Z

@bors r+

bors · 2021-05-12T08:33:15Z

📌 Commit 564b4de has been approved by nikomatsakis

bors · 2021-05-12T13:33:40Z

⌛ Testing commit 564b4de with merge 28e2b29...

bors · 2021-05-12T16:14:25Z

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

pnkfelix · 2021-05-18T15:32:14Z

Performance triage indicates that this PR introduced a 1.4% regression when fully compiling coercions-debug.

I don't think any performance hit was expected. However, it also seems like this may be just noise.

petrochenkov · 2021-06-24T17:03:09Z

compiler/rustc_hir/src/lang_items.rs

@@ -343,4 +343,7 @@ language_item_table! {
    Range,                   sym::Range,               range_struct,               Target::Struct;
    RangeToInclusive,        sym::RangeToInclusive,    range_to_inclusive_struct,  Target::Struct;
    RangeTo,                 sym::RangeTo,             range_to_struct,            Target::Struct;
+    Send,                    sym::send,                send_trait,                 Target::Trait;
+    UnwindSafe,              sym::unwind_safe,         unwind_safe_trait,          Target::Trait;
+    RefUnwindSafe,           sym::ref_unwind_safe,     ref_unwind_safe_trait,      Target::Trait;


I've just seen in #86603 that Send is made a lang item again.

I don't see why this PR needs to add new lang items, all of these could be diagnostic items instead.

That's true, I forgot that diagnostic items were a thing.

@petrochenkov Thanks for pointing this out, I'll try to have a fix done in the next few days.

@petrochenkov The fix is up for review #86726

…-rfc2229-migration, r=nikomatsakis Use diagnostic items instead of lang items for rfc2229 migrations This PR removes the `Send`, `UnwindSafe` and `RefUnwindSafe` lang items introduced in rust-lang#84730, and uses diagnostic items instead to check for `Send`, `UnwindSafe` and `RefUnwindSafe` traits for RFC2229 migrations. r? `@nikomatsakis`

…-rfc2229-migration, r=nikomatsakis Use diagnostic items instead of lang items for rfc2229 migrations This PR removes the `Send`, `UnwindSafe` and `RefUnwindSafe` lang items introduced in rust-lang#84730, and uses diagnostic items instead to check for `Send`, `UnwindSafe` and `RefUnwindSafe` traits for RFC2229 migrations. r? ``@nikomatsakis``

…-rfc2229-migration, r=nikomatsakis Use diagnostic items instead of lang items for rfc2229 migrations This PR removes the `Send`, `UnwindSafe` and `RefUnwindSafe` lang items introduced in rust-lang#84730, and uses diagnostic items instead to check for `Send`, `UnwindSafe` and `RefUnwindSafe` traits for RFC2229 migrations. r? ```@nikomatsakis```

rust-highfive assigned nikomatsakis Apr 30, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2021

This comment has been minimized.

Sign in to view

arora-aman reviewed May 4, 2021

View reviewed changes

nikomatsakis requested changes May 4, 2021

View reviewed changes

compiler/rustc_typeck/src/check/upvar.rs Outdated Show resolved Hide resolved

roxelo added 2 commits May 6, 2021 14:17

Add additional migrations to handle auto-traits and clone traits

9afea61

Combine all 2229 migrations under one flag name

Fix some nits

ad5e18d

roxelo force-pushed the rox-auto-trait branch from 25f3d74 to ad5e18d Compare May 6, 2021 18:58

Add docstring

acfc708

This comment has been minimized.

Sign in to view

nikomatsakis requested changes May 10, 2021

View reviewed changes

use the correct attributes and add helper function

564b4de

roxelo force-pushed the rox-auto-trait branch from a895081 to 564b4de Compare May 11, 2021 18:02

nikomatsakis approved these changes May 12, 2021

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label May 12, 2021

bors merged commit 28e2b29 into rust-lang:master May 12, 2021

rustbot added this to the 1.54.0 milestone May 12, 2021

This was referenced May 12, 2021

Insignificant destructors rfc 2229 #84152

Merged

Move UnwindSafe, RefUnwindSafe, AssertUnwindSafe to core #84662

Merged

implement Default for all arrays #84838

Closed

petrochenkov reviewed Jun 24, 2021

View reviewed changes

roxelo mentioned this pull request Jun 29, 2021

Use diagnostic items instead of lang items for rfc2229 migrations #86726

Merged

m-ou-se mentioned this pull request Sep 3, 2021

🎉 Rust 2021 celebration and thanks #88623

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add auto traits and clone trait migrations for RFC2229 #84730

Add auto traits and clone trait migrations for RFC2229 #84730

roxelo commented Apr 30, 2021

This comment has been minimized.

arora-aman left a comment

arora-aman May 4, 2021

nikomatsakis May 10, 2021

arora-aman May 4, 2021

roxelo May 6, 2021

nikomatsakis left a comment

roxelo commented May 6, 2021

This comment has been minimized.

This comment has been minimized.

nikomatsakis left a comment

nikomatsakis May 10, 2021

nikomatsakis commented May 12, 2021

bors commented May 12, 2021

bors commented May 12, 2021

bors commented May 12, 2021

pnkfelix commented May 18, 2021

petrochenkov Jun 24, 2021

nikomatsakis Jun 24, 2021

roxelo Jun 24, 2021

roxelo Jun 29, 2021


		let cause = ObligationCause::misc(self.tcx.hir().span(var_hir_id), self.body_id);

		let clone_obligation_should_hold = tcx

Add auto traits and clone trait migrations for RFC2229 #84730

Add auto traits and clone trait migrations for RFC2229 #84730

Conversation

roxelo commented Apr 30, 2021

This comment has been minimized.

arora-aman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis left a comment

Choose a reason for hiding this comment

roxelo commented May 6, 2021

This comment has been minimized.

This comment has been minimized.

nikomatsakis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented May 12, 2021

bors commented May 12, 2021

bors commented May 12, 2021

bors commented May 12, 2021

pnkfelix commented May 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment