auto merge of #6385 : huonw/rust/rustc-dtor-struct-match, r=nikomatsakis

**Caveat**: With the current commit, this check only works for `match`s, the checks (incorrectly) do not run for patterns in `let`s, and invalid/unsafe code compiles.

I don't know how to fix this, I experimented with some things to try to make let patterns and match patterns run on the same code (since this would presumably fix many of the other unsoundness issues of let-patterns, e.g. #6225), but I don't understand enough of the code. (I think I heard someone talking about a fix for `let` being in progress?)

Fixes #6344 and #6341.

7 files changed, 132 insertions, 23 deletions

diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs
index 09232b5bba8..893940d8ac6 100644
--- a/src/librustc/middle/check_match.rs
+++ b/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);
     }
 }
diff --git a/src/librustc/middle/typeck/check/_match.rs b/src/librustc/middle/typeck/check/_match.rs
index 40c5df7b768..a1a098bc576 100644
--- a/src/librustc/middle/typeck/check/_match.rs
+++ b/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);
 }
diff --git a/src/test/compile-fail/disallowed-deconstructing-destructing-struct.rs b/src/test/compile-fail/disallowed-deconstructing-destructing-struct-let.rs
index fa34c056794..c363f172d2f 100644
--- a/src/test/compile-fail/disallowed-deconstructing-destructing-struct.rs
+++ b/src/test/compile-fail/disallowed-deconstructing-destructing-struct-let.rs
@@ -1,3 +1,4 @@
+// xfail-test #3024
 // 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
 }
 
diff --git a/src/test/compile-fail/disallowed-deconstructing-destructing-struct-match.rs b/src/test/compile-fail/disallowed-deconstructing-destructing-struct-match.rs
new file mode 100644
index 00000000000..40305ba8b95
--- /dev/null
+++ b/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
+    }
+}
diff --git a/src/test/run-pass/issue-6341.rs b/src/test/run-pass/issue-6341.rs
new file mode 100644
index 00000000000..394345556fc
--- /dev/null
+++ b/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() {}
\ No newline at end of file
diff --git a/src/test/run-pass/issue-6344-let.rs b/src/test/run-pass/issue-6344-let.rs
new file mode 100644
index 00000000000..916131b6b71
--- /dev/null
+++ b/src/test/run-pass/issue-6344-let.rs
@@ -0,0 +1,22 @@
+// xfail-test #3874
+// 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 A { x: uint }
+
+impl Drop for A {
+    fn finalize(&self) {}
+}
+
+fn main() {
+    let a = A { x: 0 };
+
+    let A { x: ref x } = a;
+    debug!("%?", x)
+}
diff --git a/src/test/run-pass/issue-6344-match.rs b/src/test/run-pass/issue-6344-match.rs
new file mode 100644
index 00000000000..5bf57aa7116
--- /dev/null
+++ b/src/test/run-pass/issue-6344-match.rs
@@ -0,0 +1,24 @@
+// 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 A { x: uint }
+
+impl Drop for A {
+    fn finalize(&self) {}
+}
+
+fn main() {
+    let a = A { x: 0 };
+
+    match a {
+        A { x : ref x } => {
+            debug!("%?", x)
+        }
+    }
+}