4

Github Add auto traits and clone trait migrations for RFC2229 by roxelo · Pull R...

 2 years ago
source link: https://github.com/rust-lang/rust/pull/84730
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

Copy link

Member

arora-aman left a comment

mostly nits

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

let clone_obligation_should_hold = tcx

arora-aman on May 4

Member

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`");
}

nikomatsakis 29 days ago

Contributor

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

drop_reorder_reason = true;

}

if need_some_migrations {

arora-aman on May 4

Member

this can just be need need_auto_trait_migrations || need_drop_migrations

roxelo on May 6

Author

Contributor

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


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK