Auto merge of #44551 - scalexm:copy-clone-closures, r=arielb1

bors · bors · commit e2504cfc7615 · 2017-09-21T00:35:33.000Z
Implement `Copy`/`Clone` for closures Implement RFC [#2132](rust-lang/rfcs#2132) (tracking issue: #44490). NB: I'm not totally sure about the whole feature gates thing, that's my first PR of this kind...
diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs
@@ -570,6 +570,8 @@ define_dep_nodes!( <'tcx>
     [] MissingExternCrateItem(CrateNum),
     [] UsedCrateSource(CrateNum),
     [] PostorderCnums,
+    [] HasCloneClosures(CrateNum),
+    [] HasCopyClosures(CrateNum),
 
     [] Freevars(DefId),
     [] MaybeUnusedTraitImport(DefId),
diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs
@@ -1340,7 +1340,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             self.assemble_candidates_from_impls(obligation, &mut candidates)?;
 
             // For other types, we'll use the builtin rules.
-            let copy_conditions = self.copy_conditions(obligation);
+            let copy_conditions = self.copy_clone_conditions(obligation);
             self.assemble_builtin_bound_candidates(copy_conditions, &mut candidates)?;
         } else if lang_items.sized_trait() == Some(def_id) {
             // Sized is never implementable by end-users, it is
@@ -1355,7 +1355,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                  // Same builtin conditions as `Copy`, i.e. every type which has builtin support
                  // for `Copy` also has builtin support for `Clone`, + tuples and arrays of `Clone`
                  // types have builtin support for `Clone`.
-                 let clone_conditions = self.copy_conditions(obligation);
+                 let clone_conditions = self.copy_clone_conditions(obligation);
                  self.assemble_builtin_bound_candidates(clone_conditions, &mut candidates)?;
              }
 
@@ -2050,7 +2050,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         }
     }
 
-    fn copy_conditions(&mut self, obligation: &TraitObligation<'tcx>)
+    fn copy_clone_conditions(&mut self, obligation: &TraitObligation<'tcx>)
                      -> BuiltinImplConditions<'tcx>
     {
         // NOTE: binder moved to (*)
@@ -2068,8 +2068,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                 Where(ty::Binder(Vec::new()))
             }
 
-            ty::TyDynamic(..) | ty::TyStr | ty::TySlice(..) |
-            ty::TyClosure(..) | ty::TyGenerator(..) |
+            ty::TyDynamic(..) | ty::TyStr | ty::TySlice(..) | ty::TyGenerator(..) |
             ty::TyRef(_, ty::TypeAndMut { ty: _, mutbl: hir::MutMutable }) => {
                 Never
             }
@@ -2084,6 +2083,22 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                 Where(ty::Binder(tys.to_vec()))
             }
 
+            ty::TyClosure(def_id, substs) => {
+                let trait_id = obligation.predicate.def_id();
+                let copy_closures =
+                    Some(trait_id) == self.tcx().lang_items().copy_trait() &&
+                    self.tcx().has_copy_closures(def_id.krate);
+                let clone_closures =
+                    Some(trait_id) == self.tcx().lang_items().clone_trait() &&
+                    self.tcx().has_clone_closures(def_id.krate);
+
+                if copy_closures || clone_closures {
+                    Where(ty::Binder(substs.upvar_tys(def_id, self.tcx()).collect()))
+                } else {
+                    Never
+                }
+            }
+
             ty::TyAdt(..) | ty::TyProjection(..) | ty::TyParam(..) | ty::TyAnon(..) => {
                 // Fallback to whatever user-defined impls exist in this case.
                 None
@@ -2370,10 +2385,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                     self.sized_conditions(obligation)
                 }
                 _ if Some(trait_def) == lang_items.copy_trait() => {
-                    self.copy_conditions(obligation)
+                    self.copy_clone_conditions(obligation)
                 }
                 _ if Some(trait_def) == lang_items.clone_trait() => {
-                    self.copy_conditions(obligation)
+                    self.copy_clone_conditions(obligation)
                 }
                 _ => bug!("unexpected builtin trait {:?}", trait_def)
             };
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
@@ -2247,4 +2247,12 @@ pub fn provide(providers: &mut ty::maps::Providers) {
         assert_eq!(cnum, LOCAL_CRATE);
         tcx.output_filenames.clone()
     };
+    providers.has_copy_closures = |tcx, cnum| {
+        assert_eq!(cnum, LOCAL_CRATE);
+        tcx.sess.features.borrow().copy_closures
+    };
+    providers.has_clone_closures = |tcx, cnum| {
+        assert_eq!(cnum, LOCAL_CRATE);
+        tcx.sess.features.borrow().clone_closures
+    };
 }
diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs
@@ -38,7 +38,7 @@ pub enum InstanceDef<'tcx> {
     /// drop_in_place::<T>; None for empty drop glue.
     DropGlue(DefId, Option<Ty<'tcx>>),
 
-    /// Builtin method implementation, e.g. `Clone::clone`.
+    ///`<T as Clone>::clone` shim.
     CloneShim(DefId, Ty<'tcx>),
 }
 
diff --git a/src/librustc/ty/maps/config.rs b/src/librustc/ty/maps/config.rs
@@ -490,3 +490,15 @@ impl<'tcx> QueryDescription for queries::output_filenames<'tcx> {
         format!("output_filenames")
     }
 }
+
+impl<'tcx> QueryDescription for queries::has_clone_closures<'tcx> {
+    fn describe(_tcx: TyCtxt, _: CrateNum) -> String {
+        format!("seeing if the crate has enabled `Clone` closures")
+    }
+}
+
+impl<'tcx> QueryDescription for queries::has_copy_closures<'tcx> {
+    fn describe(_tcx: TyCtxt, _: CrateNum) -> String {
+        format!("seeing if the crate has enabled `Copy` closures")
+    }
+}
diff --git a/src/librustc/ty/maps/mod.rs b/src/librustc/ty/maps/mod.rs
@@ -326,6 +326,9 @@ define_maps! { <'tcx>
     [] fn compile_codegen_unit: CompileCodegenUnit(InternedString) -> Stats,
     [] fn output_filenames: output_filenames_node(CrateNum)
         -> Arc<OutputFilenames>,
+
+    [] fn has_copy_closures: HasCopyClosures(CrateNum) -> bool,
+    [] fn has_clone_closures: HasCloneClosures(CrateNum) -> bool,
 }
 
 //////////////////////////////////////////////////////////////////////
diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs
@@ -218,6 +218,16 @@ impl CrateMetadata {
         attr::contains_name(&attrs, "no_builtins")
     }
 
+     pub fn has_copy_closures(&self) -> bool {
+        let attrs = self.get_item_attrs(CRATE_DEF_INDEX);
+        attr::contains_feature_attr(&attrs, "copy_closures")
+    }
+
+    pub fn has_clone_closures(&self) -> bool {
+        let attrs = self.get_item_attrs(CRATE_DEF_INDEX);
+        attr::contains_feature_attr(&attrs, "clone_closures")
+    }
+
     pub fn panic_strategy(&self) -> PanicStrategy {
         self.root.panic_strategy.clone()
     }
diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs
@@ -231,6 +231,9 @@ provide! { <'tcx> tcx, def_id, other, cdata,
     }
 
     used_crate_source => { Rc::new(cdata.source.clone()) }
+
+    has_copy_closures => { cdata.has_copy_closures() }
+    has_clone_closures => { cdata.has_clone_closures() }
 }
 
 pub fn provide_local<'tcx>(providers: &mut Providers<'tcx>) {
diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs
@@ -296,9 +296,15 @@ fn build_clone_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             let len = len.val.to_const_int().unwrap().to_u64().unwrap();
             builder.array_shim(ty, len)
         }
-        ty::TyTuple(tys, _) => builder.tuple_shim(tys),
+        ty::TyClosure(def_id, substs) => {
+            builder.tuple_like_shim(
+                &substs.upvar_tys(def_id, tcx).collect::<Vec<_>>(),
+                AggregateKind::Closure(def_id, substs)
+            )
+        }
+        ty::TyTuple(tys, _) => builder.tuple_like_shim(&**tys, AggregateKind::Tuple),
         _ => {
-            bug!("clone shim for `{:?}` which is not `Copy` and is not an aggregate", self_ty);
+            bug!("clone shim for `{:?}` which is not `Copy` and is not an aggregate", self_ty)
         }
     };
 
@@ -613,7 +619,12 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> {
         self.block(vec![], TerminatorKind::Resume, true);
     }
 
-    fn tuple_shim(&mut self, tys: &ty::Slice<Ty<'tcx>>) {
+    fn tuple_like_shim(&mut self, tys: &[ty::Ty<'tcx>], kind: AggregateKind<'tcx>) {
+        match kind {
+            AggregateKind::Tuple | AggregateKind::Closure(..) => (),
+            _ => bug!("only tuples and closures are accepted"),
+        };
+
         let rcvr = Lvalue::Local(Local::new(1+0)).deref();
 
         let mut returns = Vec::new();
@@ -646,17 +657,17 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> {
             }
         }
 
-        // `return (returns[0], returns[1], ..., returns[tys.len() - 1]);`
+        // `return kind(returns[0], returns[1], ..., returns[tys.len() - 1]);`
         let ret_statement = self.make_statement(
             StatementKind::Assign(
                 Lvalue::Local(RETURN_POINTER),
                 Rvalue::Aggregate(
-                    box AggregateKind::Tuple,
+                    box kind,
                     returns.into_iter().map(Operand::Consume).collect()
                 )
             )
         );
-       self.block(vec![ret_statement], TerminatorKind::Return, false);
+        self.block(vec![ret_statement], TerminatorKind::Return, false);
     }
 }
 
diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs
@@ -500,6 +500,20 @@ pub fn first_attr_value_str_by_name(attrs: &[Attribute], name: &str) -> Option<S
         .and_then(|at| at.value_str())
 }
 
+/// Check if `attrs` contains an attribute like `#![feature(feature_name)]`.
+/// This will not perform any "sanity checks" on the form of the attributes.
+pub fn contains_feature_attr(attrs: &[Attribute], feature_name: &str) -> bool {
+    attrs.iter().any(|item| {
+        item.check_name("feature") &&
+        item.meta_item_list().map(|list| {
+            list.iter().any(|mi| {
+                mi.word().map(|w| w.name() == feature_name)
+                         .unwrap_or(false)
+            })
+        }).unwrap_or(false)
+    })
+}
+
 /* Higher-level applications */
 
 pub fn find_crate_name(attrs: &[Attribute]) -> Option<Symbol> {
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
@@ -385,6 +385,10 @@ declare_features! (
 
     // allow '|' at beginning of match arms (RFC 1925)
     (active, match_beginning_vert, "1.21.0", Some(44101)),
+
+    // Copy/Clone closures (RFC 2132)
+    (active, clone_closures, "1.22.0", Some(44490)),
+    (active, copy_closures, "1.22.0", Some(44490)),
 );
 
 declare_features! (
@@ -1573,7 +1577,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
 pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute]) -> Features {
     let mut features = Features::new();
 
-    let mut feature_checker = MutexFeatureChecker::default();
+    let mut feature_checker = FeatureChecker::default();
 
     for attr in krate_attrs {
         if !attr.check_name("feature") {
@@ -1622,14 +1626,16 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute]) -> F
     features
 }
 
-// A collector for mutually-exclusive features and their flag spans
+/// A collector for mutually exclusive and interdependent features and their flag spans.
 #[derive(Default)]
-struct MutexFeatureChecker {
+struct FeatureChecker {
     proc_macro: Option<Span>,
     custom_attribute: Option<Span>,
+    copy_closures: Option<Span>,
+    clone_closures: Option<Span>,
 }
 
-impl MutexFeatureChecker {
+impl FeatureChecker {
     // If this method turns out to be a hotspot due to branching,
     // the branching can be eliminated by modifying `set!()` to set these spans
     // only for the features that need to be checked for mutual exclusion.
@@ -1642,6 +1648,14 @@ impl MutexFeatureChecker {
         if features.custom_attribute {
             self.custom_attribute = self.custom_attribute.or(Some(span));
         }
+
+        if features.copy_closures {
+            self.copy_closures = self.copy_closures.or(Some(span));
+        }
+
+        if features.clone_closures {
+            self.clone_closures = self.clone_closures.or(Some(span));
+        }
     }
 
     fn check(self, handler: &Handler) {
@@ -1653,6 +1667,15 @@ impl MutexFeatureChecker {
 
             panic!(FatalError);
         }
+
+        if let (Some(span), None) = (self.copy_closures, self.clone_closures) {
+            handler.struct_span_err(span, "`#![feature(copy_closures)]` can only be used with \
+                                           `#![feature(clone_closures)]`")
+                  .span_note(span, "`#![feature(copy_closures)]` declared here")
+                  .emit();
+
+            panic!(FatalError);
+        }
     }
 }
 
diff --git a/src/test/compile-fail/feature-gate-clone-closures.rs b/src/test/compile-fail/feature-gate-clone-closures.rs
@@ -0,0 +1,21 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[derive(Clone)]
+struct S(i32);
+
+fn main() {
+    let a = S(5);
+    let hello = move || {
+        println!("Hello {}", a.0);
+    };
+
+    let hello = hello.clone(); //~ ERROR no method named `clone` found for type
+}
diff --git a/src/test/compile-fail/feature-gate-copy-closures.rs b/src/test/compile-fail/feature-gate-copy-closures.rs
@@ -0,0 +1,19 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    let a = 5;
+    let hello = || {
+        println!("Hello {}", a);
+    };
+
+    let b = hello;
+    let c = hello; //~ ERROR use of moved value: `hello` [E0382]
+}
diff --git a/src/test/compile-fail/not-clone-closure.rs b/src/test/compile-fail/not-clone-closure.rs
@@ -0,0 +1,24 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Check that closures do not implement `Clone` if their environment is not `Clone`.
+
+#![feature(clone_closures)]
+
+struct S(i32);
+
+fn main() {
+    let a = S(5);
+    let hello = move || {
+        println!("Hello {}", a.0);
+    };
+
+    let hello = hello.clone(); //~ ERROR the trait bound `S: std::clone::Clone` is not satisfied
+}
diff --git a/src/test/compile-fail/not-copy-closure.rs b/src/test/compile-fail/not-copy-closure.rs
@@ -0,0 +1,24 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Check that closures do not implement `Copy` if their environment is not `Copy`.
+
+#![feature(copy_closures)]
+#![feature(clone_closures)]
+
+fn main() {
+    let mut a = 5;
+    let hello = || {
+        a += 1;
+    };
+
+    let b = hello;
+    let c = hello; //~ ERROR use of moved value: `hello` [E0382]
+}
diff --git a/src/test/run-pass/clone-closure.rs b/src/test/run-pass/clone-closure.rs
diff --git a/src/test/run-pass/copy-closure.rs b/src/test/run-pass/copy-closure.rs

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ pub enum InstanceDef<'tcx> {`
`38`	`38`	`/// drop_in_place::<T>; None for empty drop glue.`
`39`	`39`	`DropGlue(DefId, Option<Ty<'tcx>>),`
`40`	`40`
`41`		- /// Builtin method implementation, e.g. `Clone::clone`.
	`41`	+ ///`<T as Clone>::clone` shim.
`42`	`42`	`CloneShim(DefId, Ty<'tcx>),`
`43`	`43`	`}`
`44`	`44`
Original file line number	Diff line number	Diff line change
`@@ -326,6 +326,9 @@ define_maps! { <'tcx>`
`326`	`326`	`[] fn compile_codegen_unit: CompileCodegenUnit(InternedString) -> Stats,`
`327`	`327`	`[] fn output_filenames: output_filenames_node(CrateNum)`
`328`	`328`	`-> Arc<OutputFilenames>,`
	`329`	`+`
	`330`	`+ [] fn has_copy_closures: HasCopyClosures(CrateNum) -> bool,`
	`331`	`+ [] fn has_clone_closures: HasCloneClosures(CrateNum) -> bool,`
`329`	`332`	`}`
`330`	`333`
`331`	`334`	`//////////////////////////////////////////////////////////////////////`
Original file line number	Diff line number	Diff line change
`@@ -231,6 +231,9 @@ provide! { <'tcx> tcx, def_id, other, cdata,`
`231`	`231`	`}`
`232`	`232`
`233`	`233`	`used_crate_source => { Rc::new(cdata.source.clone()) }`
	`234`	`+`
	`235`	`+ has_copy_closures => { cdata.has_copy_closures() }`
	`236`	`+ has_clone_closures => { cdata.has_clone_closures() }`
`234`	`237`	`}`
`235`	`238`
`236`	`239`	`pub fn provide_local<'tcx>(providers: &mut Providers<'tcx>) {`
Original file line number	Diff line number	Diff line change
`@@ -296,9 +296,15 @@ fn build_clone_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,`
`296`	`296`	`let len = len.val.to_const_int().unwrap().to_u64().unwrap();`
`297`	`297`	`builder.array_shim(ty, len)`
`298`	`298`	`}`
`299`		`- ty::TyTuple(tys, _) => builder.tuple_shim(tys),`
	`299`	`+ ty::TyClosure(def_id, substs) => {`
	`300`	`+ builder.tuple_like_shim(`
	`301`	`+ &substs.upvar_tys(def_id, tcx).collect::<Vec<_>>(),`
	`302`	`+ AggregateKind::Closure(def_id, substs)`
	`303`	`+ )`
	`304`	`+ }`
	`305`	`+ ty::TyTuple(tys, _) => builder.tuple_like_shim(&**tys, AggregateKind::Tuple),`
`300`	`306`	`_ => {`
`301`		- bug!("clone shim for `{:?}` which is not `Copy` and is not an aggregate", self_ty);
	`307`	+ bug!("clone shim for `{:?}` which is not `Copy` and is not an aggregate", self_ty)
`302`	`308`	`}`
`303`	`309`	`};`
`304`	`310`
`@@ -613,7 +619,12 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> {`
`613`	`619`	`self.block(vec![], TerminatorKind::Resume, true);`
`614`	`620`	`}`
`615`	`621`
`616`		`- fn tuple_shim(&mut self, tys: &ty::Slice<Ty<'tcx>>) {`
	`622`	`+ fn tuple_like_shim(&mut self, tys: &[ty::Ty<'tcx>], kind: AggregateKind<'tcx>) {`
	`623`	`+ match kind {`
	`624`	`+ AggregateKind::Tuple \| AggregateKind::Closure(..) => (),`
	`625`	`+ _ => bug!("only tuples and closures are accepted"),`
	`626`	`+ };`
	`627`	`+`
`617`	`628`	`let rcvr = Lvalue::Local(Local::new(1+0)).deref();`
`618`	`629`
`619`	`630`	`let mut returns = Vec::new();`
`@@ -646,17 +657,17 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> {`
`646`	`657`	`}`
`647`	`658`	`}`
`648`	`659`
`649`		- // `return (returns[0], returns[1], ..., returns[tys.len() - 1]);`
	`660`	+ // `return kind(returns[0], returns[1], ..., returns[tys.len() - 1]);`
`650`	`661`	`let ret_statement = self.make_statement(`
`651`	`662`	`StatementKind::Assign(`
`652`	`663`	`Lvalue::Local(RETURN_POINTER),`
`653`	`664`	`Rvalue::Aggregate(`
`654`		`- box AggregateKind::Tuple,`
	`665`	`+ box kind,`
`655`	`666`	`returns.into_iter().map(Operand::Consume).collect()`
`656`	`667`	`)`
`657`	`668`	`)`
`658`	`669`	`);`
`659`		`- self.block(vec![ret_statement], TerminatorKind::Return, false);`
	`670`	`+ self.block(vec![ret_statement], TerminatorKind::Return, false);`
`660`	`671`	`}`
`661`	`672`	`}`
`662`	`673`