Skip to content

Commit

Permalink
auto merge of #6385 : huonw/rust/rustc-dtor-struct-match, r=nikomatsakis
Browse files Browse the repository at this point in the history
**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.
  • Loading branch information
bors committed May 11, 2013
2 parents 96de2b0 + 912a352 commit e478ced
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 23 deletions.
54 changes: 38 additions & 16 deletions src/librustc/middle/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
6 changes: 0 additions & 6 deletions src/librustc/middle/typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}
18 changes: 18 additions & 0 deletions src/test/run-pass/issue-6341.rs
Original file line number Diff line number Diff line change
@@ -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() {}
22 changes: 22 additions & 0 deletions src/test/run-pass/issue-6344-let.rs
Original file line number Diff line number Diff line change
@@ -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)
}
24 changes: 24 additions & 0 deletions src/test/run-pass/issue-6344-match.rs
Original file line number Diff line number Diff line change
@@ -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)
}
}
}

0 comments on commit e478ced

Please sign in to comment.