librustc: allow destructuring of structs with destructors if the pattern has no moves.

This check only works for `match`s, the checks (incorrectly) do not run for patterns in
`let`s.

Author: huonw

src/librustc/middle/check_match.rs

@@ -822,43 +822,65 @@ pub fn check_legality_of_move_bindings(cx: @MatchCheckCtxt,
             }
         }
 
-        // Now check to ensure that any move binding is not behind an @ or &.
-        // This is always illegal.
+        // Now check to ensure that any move binding is not behind an
+        // @ or &, or within a struct with a destructor.  This is
+        // always illegal.
         let vt = visit::mk_vt(@visit::Visitor {
-            visit_pat: |pat, behind_bad_pointer: bool, v| {
+            visit_pat: |pat, (behind_bad_pointer, behind_dtor_struct): (bool, bool), v| {
                 match pat.node {
                     pat_ident(_, _, sub) => {
                         debug!("(check legality of move) checking pat \
-                                ident with behind_bad_pointer %?",
-                                behind_bad_pointer);
+                                ident with behind_bad_pointer %? and behind_dtor_struct %?",
+                               behind_bad_pointer, behind_dtor_struct);
 
-                        if behind_bad_pointer &&
+                        if behind_bad_pointer || behind_dtor_struct &&
                             cx.moves_map.contains(&pat.id)
                         {
-                            cx.tcx.sess.span_err(
-                                pat.span,
-                                "by-move pattern \
-                                 bindings may not occur \
-                                 behind @ or & bindings");
+                            let msg = if behind_bad_pointer {
+                                "by-move pattern bindings may not occur behind @ or & bindings"
+                            } else {
+                                "cannot bind by-move within struct (it has a destructor)"
+                            };
+                            cx.tcx.sess.span_err(pat.span, msg);
                         }
 
                         match sub {
                             None => {}
                             Some(subpat) => {
-                                (v.visit_pat)(subpat, behind_bad_pointer, v);
+                                (v.visit_pat)(subpat,
+                                              (behind_bad_pointer, behind_dtor_struct),
+                                              v);
                             }
                         }
                     }
 
                     pat_box(subpat) | pat_region(subpat) => {
-                        (v.visit_pat)(subpat, true, v);
+                        (v.visit_pat)(subpat, (true, behind_dtor_struct), v);
                     }
 
-                    _ => visit::visit_pat(pat, behind_bad_pointer, v)
+                    pat_struct(_, ref fields, _) => {
+                        let behind_dtor_struct = behind_dtor_struct ||
+                            (match cx.tcx.def_map.find(&pat.id) {
+                                Some(&def_struct(id)) => {
+                                    ty::has_dtor(cx.tcx, id)
+                                }
+                                _ => false
+                            });
+                        debug!("(check legality of move) checking pat \
+                                struct with behind_bad_pointer %? and behind_dtor_struct %?",
+                               behind_bad_pointer, behind_dtor_struct);
+
+                        for fields.each |fld| {
+                            (v.visit_pat)(fld.pat, (behind_bad_pointer,
+                                                    behind_dtor_struct), v)
+                        }
+                    }
+
+                    _ => visit::visit_pat(pat, (behind_bad_pointer, behind_dtor_struct), v)
                 }
             },
-            .. *visit::default_visitor::<bool>()
+            .. *visit::default_visitor::<(bool, bool)>()
         });
-        (vt.visit_pat)(*pat, false, vt);
+        (vt.visit_pat)(*pat, (false, false), vt);
     }
 }

src/librustc/middle/typeck/check/_match.rs

@@ -340,12 +340,6 @@ pub fn check_struct_pat(pcx: &pat_ctxt, pat_id: ast::node_id, span: span,
         }
     }
 
-    // Forbid pattern-matching structs with destructors.
-    if ty::has_dtor(tcx, class_id) {
-        tcx.sess.span_err(span, "deconstructing struct not allowed in pattern \
-                                 (it has a destructor)");
-    }
-
     check_struct_pat_fields(pcx, span, path, fields, class_fields, class_id,
                             substitutions, etc);
 }

src/test/compile-fail/disallowed-deconstructing-destructing-struct.rs renamed to src/test/compile-fail/disallowed-deconstructing-destructing-struct-let.rs

@@ -1,3 +1,4 @@
+// xfail-test
 // Copyright 2012 The Rust Project Developers. See the COPYRIGHT
 // file at the top-level directory of this distribution and at
 // http://rust-lang.org/COPYRIGHT.
@@ -19,7 +20,7 @@ impl Drop for X {
 }
 
 fn unwrap(x: X) -> ~str {
-    let X { x: y } = x; //~ ERROR deconstructing struct not allowed in pattern
+    let X { x: y } = x; //~ ERROR cannot bind by-move within struct
     y
 }
 

src/test/compile-fail/disallowed-deconstructing-destructing-struct-match.rs

@@ -0,0 +1,28 @@
+// Copyright 2013 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.
+
+struct X {
+    x: ~str,
+}
+
+impl Drop for X {
+    fn finalize(&self) {
+        error!("value: %s", self.x);
+    }
+}
+
+fn main() {
+    let x = X { x: ~"hello" };
+
+    match x {
+        X { x: y } => error!("contents: %s", y)
+        //~^ ERROR cannot bind by-move within struct
+    }
+}

src/test/run-pass/issue-6341.rs

@@ -0,0 +1,18 @@
+// Copyright 2013 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.
+
+#[deriving(Eq)]
+struct A { x: uint }
+
+impl Drop for A {
+    fn finalize(&self) {}
+}
+
+fn main() {}