From f6743fea70e99efc33f6890770da8c0f91723311 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 10 May 2013 16:12:19 +1000 Subject: [PATCH 1/3] 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. --- src/librustc/middle/check_match.rs | 54 +++++++++++++------ src/librustc/middle/typeck/check/_match.rs | 6 --- ...-deconstructing-destructing-struct-let.rs} | 3 +- ...deconstructing-destructing-struct-match.rs | 28 ++++++++++ src/test/run-pass/issue-6341.rs | 18 +++++++ 5 files changed, 86 insertions(+), 23 deletions(-) rename src/test/compile-fail/{disallowed-deconstructing-destructing-struct.rs => disallowed-deconstructing-destructing-struct-let.rs} (89%) create mode 100644 src/test/compile-fail/disallowed-deconstructing-destructing-struct-match.rs create mode 100644 src/test/run-pass/issue-6341.rs 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::() + .. *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 similarity index 89% rename from src/test/compile-fail/disallowed-deconstructing-destructing-struct.rs rename to src/test/compile-fail/disallowed-deconstructing-destructing-struct-let.rs index fa34c056794..ed68defdc26 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 // 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 or the MIT license +// , 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 or the MIT license +// , 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 From 81e06a5259f5e477f552a35ac58c4b69f58b1451 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 10 May 2013 19:30:42 +1000 Subject: [PATCH 2/3] Issue number on xfailed test --- .../disallowed-deconstructing-destructing-struct-let.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/compile-fail/disallowed-deconstructing-destructing-struct-let.rs b/src/test/compile-fail/disallowed-deconstructing-destructing-struct-let.rs index ed68defdc26..c363f172d2f 100644 --- a/src/test/compile-fail/disallowed-deconstructing-destructing-struct-let.rs +++ b/src/test/compile-fail/disallowed-deconstructing-destructing-struct-let.rs @@ -1,4 +1,4 @@ -// xfail-test +// 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. From 912a352712b1b97009a11e3c3f7c4ba7360d9eaf Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 10 May 2013 19:42:24 +1000 Subject: [PATCH 3/3] Add some positive tests for dtor struct destructuring --- src/test/run-pass/issue-6344-let.rs | 22 ++++++++++++++++++++++ src/test/run-pass/issue-6344-match.rs | 24 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 src/test/run-pass/issue-6344-let.rs create mode 100644 src/test/run-pass/issue-6344-match.rs 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 or the MIT license +// , 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 or the MIT license +// , 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) + } + } +}