From beda82ddf1f482f286a8d9af3402626dc56d6fea Mon Sep 17 00:00:00 2001
From: Marijn Haverbeke <marijnh@gmail.com>
Date: Tue, 7 Jun 2011 11:20:51 +0200
Subject: [PATCH] A revised, improved alias-checker

The old system tried to ensure that the location an alias pointed at
would retain its type. That turned out to not be strong enough in the
face of aliases to the inside of tags.

The new system instead proves that values pointed to by aliases are
not replaced (or invalidated in some other way) at all. It knows of
two sufficient conditions for this, and tries to prove at least of
them:

A) The alias is 'immutably rooted' in a local, and this local is not
   reassigned for the lifetime of the alias. Immutably rooted means
   the alias refers to the local itself, or to something reachable
   from the local through immutable dereferencing.

B) No value whose type might include the type of the 'inner mutable
   element' of the thing the alias refers to (for example, the box in
   rec(mutable x = @mutable int)) is from the outer scope is accessed
   for the lifetime of the alias. This means for functions, no other
   argument types may include the alias's inner mutable type. For alt,
   for each, and for, it means the body does not refer to any locals
   originating from outside their scope that include this type.

The lifetime of an alias in an alt, for each, or for body is defined
as the range from its definition to its last use, not to the point
where it goes out of scope. This makes working around these
restrictions somewhat less annoying. For example, you can assign to
your alt-ed value you don't refer to any bindings afterwards.
---
 src/comp/front/ast.rs                   |   4 +
 src/comp/middle/alias.rs                | 557 +++++++++++++++++-------
 src/comp/middle/resolve.rs              |   2 +-
 src/test/compile-fail/unsafe-alias-2.rs |  15 +
 src/test/compile-fail/unsafe-alias.rs   |  10 +-
 src/test/compile-fail/unsafe-alt.rs     |   3 +-
 6 files changed, 430 insertions(+), 161 deletions(-)
 create mode 100644 src/test/compile-fail/unsafe-alias-2.rs

diff --git a/src/comp/front/ast.rs b/src/comp/front/ast.rs
index 4f83ee7fa8d7..fce15a556c6f 100644
--- a/src/comp/front/ast.rs
+++ b/src/comp/front/ast.rs
@@ -11,6 +11,10 @@ type ident = str;
 type path_ = rec(vec[ident] idents, vec[@ty] types);
 type path = spanned[path_];
 
+fn path_name(&path p) -> str {
+    ret str::connect(p.node.idents, "::");
+}
+
 type crate_num = int;
 const crate_num local_crate = 0;
 type def_num = int;
diff --git a/src/comp/middle/alias.rs b/src/comp/middle/alias.rs
index b433799ac368..fa978ef53c03 100644
--- a/src/comp/middle/alias.rs
+++ b/src/comp/middle/alias.rs
@@ -1,209 +1,388 @@
 import front::ast;
 import front::ast::ident;
-import front::ast::def_id;
+import front::ast::def_num;
+import util::common::span;
+import visit::vt;
 import std::vec;
 import std::str;
 import std::option;
 import std::option::some;
 import std::option::none;
+import std::option::is_none;
 
-tag deref_t {
-    field(ident);
-    index;
-    unbox;
+tag valid {
+    valid;
+    overwritten(span, ast::path);
+    val_taken(span, ast::path);
 }
-type deref = rec(bool mut, deref_t t);
 
-type ctx = @rec(@ty::ctxt tcx,
-                resolve::def_map dm,
-                // The current blacklisted (non-assignable) locals
-                mutable vec[vec[def_id]] bl,
-                // A stack of blacklists for outer function scopes
-                mutable vec[vec[vec[def_id]]] blstack);
+type restrict = @rec(vec[def_num] root_vars,
+                     def_num block_defnum,
+                     vec[def_num] bindings,
+                     vec[ty::t] tys,
+                     mutable valid ok);
+
+type scope = vec[restrict];
+type ctx = rec(@ty::ctxt tcx,
+               resolve::def_map dm);
 
 fn check_crate(@ty::ctxt tcx, resolve::def_map dm, &@ast::crate crate) {
-    auto cx = @rec(tcx = tcx,
-                   dm = dm,
-                   mutable bl = vec::empty[vec[def_id]](),
-                   mutable blstack = vec::empty[vec[vec[def_id]]]());
-    auto v = rec(visit_item_pre = bind enter_item(cx, _),
-                 visit_item_post = bind leave_item(cx, _),
-                 visit_method_pre = bind enter_method(cx, _),
-                 visit_method_post = bind leave_method(cx, _),
-                 visit_expr_pre = bind check_expr(cx, _),
-                 visit_expr_post = bind leave_expr(cx, _)
-                 with walk::default_visitor());
-    walk::walk_crate(v, *crate);
+    auto cx = @rec(tcx = tcx, dm = dm);
+    auto v = @rec(visit_fn = visit_fn,
+                  visit_expr = bind visit_expr(cx, _, _, _)
+                  with *visit::default_visitor[scope]());
+    visit::visit_crate(*crate, [], visit::vtor(v));
 }
 
-fn enter_item(ctx cx, &@ast::item it) {
-    alt (it.node) {
-        case (ast::item_fn(_, _, _, _, _)) {
-            vec::push(cx.blstack, cx.bl);
-            cx.bl = [];
-        }
-        case (_) {}
-    }
-}
-fn leave_item(ctx cx, &@ast::item it) {
-    alt (it.node) {
-        case (ast::item_fn(_, _, _, _, _)) {
-            cx.bl = vec::pop(cx.blstack);
-        }
-        case (_) {}
-    }
+fn visit_fn(&ast::_fn f, &span sp, &ident name, &ast::def_id d_id,
+            &ast::ann a, &scope sc, &vt[scope] v) {
+    visit::visit_fn_decl(f.decl, sc, v);
+    vt(v).visit_block(f.body, [], v);
 }
 
-fn enter_method(ctx cx, &@ast::method mt) {
-    vec::push(cx.blstack, cx.bl);
-    cx.bl = [];
-}
-fn leave_method(ctx cx, &@ast::method mt) {
-    cx.bl = vec::pop(cx.blstack);
-}
-
-fn check_expr(ctx cx, &@ast::expr ex) {
+fn visit_expr(&@ctx cx, &@ast::expr ex, &scope sc, &vt[scope] v) {
+    auto handled = false;
     alt (ex.node) {
         case (ast::expr_call(?f, ?args, _)) {
-            auto fty = ty::expr_ty(*cx.tcx, f);
-            auto argtys = alt (ty::struct(*cx.tcx, fty)) {
-                case (ty::ty_fn(_, ?args, _, _)) { args }
-                case (ty::ty_native_fn(_, ?args, _)) { args }
-            };
-            auto i = 0u;
-            let vec[def_id] listed = [];
-            for (ty::arg argty in argtys) {
-                if (argty.mode != ty::mo_val) {
-                    alt (check_rooted(cx, args.(i), false)) {
-                        case (some(?did)) {
-                            vec::push(listed, did);
-                        }
-                        case (_) {}
-                    }
-                }
-                i += 1u;
-            }
-            // FIXME when mutable aliases can be distinguished, go over the
-            // args again and ensure that we're not passing a blacklisted
-            // variable by mutable alias (using 'listed' and the context
-            // blacklist).
+            check_call(*cx, f, args, sc);
+        }
+        case (ast::expr_alt(?input, ?arms, _)) {
+            check_alt(*cx, input, arms, sc, v);
+            handled = true;
         }
         case (ast::expr_put(?val, _)) {
             alt (val) {
-                case (some(?ex)) { check_rooted(cx, ex, false); }
+                case (some(?ex)) {
+                    auto root = expr_root(*cx, ex, false);
+                    if (!is_none(root.inner_mut)) {
+                        cx.tcx.sess.span_err
+                            (ex.span,
+                             "result of put must be immutably rooted");
+                    }
+                    visit_expr(cx, ex, sc, v);
+                }
+                case (_) {}
+            }
+            handled = true;
+        }
+        case (ast::expr_for_each(?decl, ?call, ?block, _)) {
+            check_for_each(*cx, decl, call, block, sc, v);
+            handled = true;
+        }
+        case (ast::expr_for(?decl, ?seq, ?block, _)) {
+            check_for(*cx, decl, seq, block, sc, v);
+            handled = true;
+        }
+
+        case (ast::expr_path(?pt, ?ann)) {
+            check_var(*cx, ex, pt, ann, false, sc);
+        }
+        case (ast::expr_move(?dest, ?src, _)) {
+            check_assign(cx, dest, src, sc, v);
+            handled = true;
+        }
+        case (ast::expr_assign(?dest, ?src, _)) {
+            check_assign(cx, dest, src, sc, v);
+            handled = true;
+        }
+        case (ast::expr_assign_op(_, ?dest, ?src, _)) {
+            check_assign(cx, dest, src, sc, v);
+            handled = true;
+        }
+
+        case (_) {}
+    }
+    if (!handled) { visit::visit_expr(ex, sc, v); }
+}
+
+fn check_call(&ctx cx, &@ast::expr f, &vec[@ast::expr] args, &scope sc)
+    -> rec(vec[def_num] root_vars, vec[ty::t] unsafe_ts) {
+    auto fty = ty::expr_ty(*cx.tcx, f);
+    auto arg_ts = alt (ty::struct(*cx.tcx, fty)) {
+        case (ty::ty_fn(_, ?args, _, _)) { args }
+        case (ty::ty_native_fn(_, ?args, _)) { args }
+    };
+
+    auto i = 0u;
+    let vec[def_num] roots = [];
+    let vec[ty::t] unsafe_ts = [];
+    let vec[uint] unsafe_t_offsets = [];
+    for (ty::arg arg_t in arg_ts) {
+        if (arg_t.mode != ty::mo_val) {
+            auto root = expr_root(cx, args.(i), false);
+            alt (path_def_id(cx, root.ex)) {
+                case (some(?did)) { vec::push(roots, did._1); }
+                case (_) {}
+            }
+            alt (root.inner_mut) {
+                case (some(?t)) {
+                    vec::push(unsafe_ts, t);
+                    vec::push(unsafe_t_offsets, i);
+                }
                 case (_) {}
             }
         }
-        case (ast::expr_alt(?input, _, _)) {
-            vec::push(cx.bl, alt (check_rooted(cx, input, true)) {
-                case (some(?did)) { [did] }
-                case (_) { vec::empty[def_id]() }
-            });
-        }
-
-        case (ast::expr_move(?dest, _, _)) { check_assign(cx, dest); }
-        case (ast::expr_assign(?dest, _, _)) { check_assign(cx, dest); }
-        case (ast::expr_assign_op(_, ?dest, _, _)) { check_assign(cx, dest); }
-        case (_) {}
+        i += 1u;
     }
-}
 
-fn leave_expr(ctx cx, &@ast::expr ex) {
-    alt (ex.node) {
-        case (ast::expr_alt(_, _, _)) { vec::pop(cx.bl); }
-        case (_) {}
-    }
-}
-
-fn check_assign(&ctx cx, &@ast::expr ex) {
-    alt (ex.node) {
-        case (ast::expr_path(?pt, ?ann)) {
-            auto did = ast::def_id_of_def(cx.dm.get(ann.id));
-            for (vec[def_id] layer in cx.bl) {
-                for (def_id black in layer) {
-                    if (did == black) {
-                        cx.tcx.sess.span_err
-                            (ex.span, str::connect(pt.node.idents, "::") +
-                             " is being aliased and may not be assigned to");
-                    }
+    if (vec::len(unsafe_ts) > 0u) {
+        alt (f.node) {
+            case (ast::expr_path(_, ?ann)) {
+                if (def_is_local(cx.dm.get(ann.id))) {
+                    cx.tcx.sess.span_err
+                        (f.span, #fmt("function may alias with argument \
+                         %u, which is not immutably rooted",
+                         unsafe_t_offsets.(0)));
                 }
             }
+            case (_) {}
         }
-        case (_) {}
+    }
+    auto j = 0u;
+    for (ty::t unsafe in unsafe_ts) {
+        auto offset = unsafe_t_offsets.(j);
+        j += 1u;
+        auto i = 0u;
+        for (ty::arg arg_t in arg_ts) {
+            if (i != offset &&
+                // FIXME false should be replace with mutability of alias
+                ty_can_unsafely_include(cx, unsafe, arg_t.ty, false)) {
+                cx.tcx.sess.span_err
+                    (args.(i).span, #fmt("argument %u may alias with \
+                     argument %u, which is not immutably rooted", i, offset));
+            }
+            i += 1u;
+        }
+    }
+    // FIXME when mutable aliases can be distinguished, go over the args again
+    // and ensure that we're not passing a root variable by mutable alias
+    // (using roots and the scope root vars).
+
+    ret rec(root_vars = roots, unsafe_ts = unsafe_ts);
+}
+
+fn check_alt(&ctx cx, &@ast::expr input, &vec[ast::arm] arms,
+             &scope sc, &vt[scope] v) {
+    visit::visit_expr(input, sc, v);
+    auto root = expr_root(cx, input, true);
+    auto roots = alt (path_def_id(cx, root.ex)) {
+        case (some(?did)) { [did._1] }
+        case (_) { [] }
+    };
+    let vec[ty::t] forbidden_tp = alt (root.inner_mut) {
+        case (some(?t)) { [t] }
+        case (_) { [] }
+    };
+
+    for (ast::arm a in arms) {
+        auto dnums = arm_defnums(a);
+        auto new_sc = sc;
+        if (vec::len(dnums) > 0u) {
+            vec::push(new_sc, @rec(root_vars=roots,
+                                   block_defnum=dnums.(0),
+                                   bindings=dnums,
+                                   tys=forbidden_tp,
+                                   mutable ok=valid));
+        }
+        visit::visit_arm(a, new_sc, v);
     }
 }
 
-fn check_rooted(&ctx cx, &@ast::expr ex, bool autoderef)
-    -> option::t[def_id] {
-    auto root = expr_root(cx, ex, autoderef);
-    if (has_unsafe_box(root.ds)) {
-        cx.tcx.sess.span_err
-            (ex.span, "can not create alias to improperly anchored value");
+fn arm_defnums(&ast::arm arm) -> vec[def_num] {
+    auto dnums = [];
+    fn walk_pat(&mutable vec[def_num] found, &@ast::pat p) {
+        alt (p.node) {
+            case (ast::pat_bind(_, ?did, _)) {
+                vec::push(found, did._1);
+            }
+            case (ast::pat_tag(_, ?children, _)) {
+                for (@ast::pat child in children) {
+                    walk_pat(found, child);
+                }
+            }
+            case (_) {}
+        }
     }
-    alt (root.ex.node) {
-        case (ast::expr_path(_, ?ann)) {
-            ret some(ast::def_id_of_def(cx.dm.get(ann.id)));
+    walk_pat(dnums, arm.pat);
+    ret dnums;
+}
+
+fn check_for_each(&ctx cx, &@ast::decl decl, &@ast::expr call,
+                  &ast::block block, &scope sc, &vt[scope] v) {
+    visit::visit_expr(call, sc, v);
+    alt (call.node) {
+        case (ast::expr_call(?f, ?args, _)) {
+            auto data = check_call(cx, f, args, sc);
+            auto defnum = alt (decl.node) {
+                case (ast::decl_local(?l)) { l.id._1 }
+            };
+            
+            auto new_sc = @rec(root_vars=data.root_vars,
+                               block_defnum=defnum,
+                               bindings=[defnum],
+                               tys=data.unsafe_ts,
+                               mutable ok=valid);
+            visit::visit_block(block, sc + [new_sc], v);
+        }
+    }
+}
+
+fn check_for(&ctx cx, &@ast::decl decl, &@ast::expr seq,
+             &ast::block block, &scope sc, &vt[scope] v) {
+    visit::visit_expr(seq, sc, v);
+    auto defnum = alt (decl.node) {
+        case (ast::decl_local(?l)) { l.id._1 }
+    };
+
+    auto root = expr_root(cx, seq, false);
+    auto root_def = alt (path_def_id(cx, root.ex)) {
+        case (some(?did)) { [did._1] }
+        case (_) { [] }
+    };
+    auto unsafe = alt (root.inner_mut) {
+        case (some(?t)) { [t] }
+        case (_) { [] }
+    };
+    // If this is a mutable vector, don't allow it to be touched.
+    auto seq_t = ty::expr_ty(*cx.tcx, seq);
+    alt (ty::struct(*cx.tcx, seq_t)) {
+        case (ty::ty_vec(?mt)) {
+            if (mt.mut != ast::imm) { unsafe = [seq_t]; }
+        }
+        case (ty::ty_str) {}
+    }
+
+    auto new_sc = @rec(root_vars=root_def,
+                       block_defnum=defnum,
+                       bindings=[defnum],
+                       tys=unsafe,
+                       mutable ok=valid);
+    visit::visit_block(block, sc + [new_sc], v);
+}
+
+fn check_var(&ctx cx, &@ast::expr ex, &ast::path p, ast::ann ann, bool assign,
+             &scope sc) {
+    auto def = cx.dm.get(ann.id);
+    if (!def_is_local(def)) { ret; }
+    auto my_defnum = ast::def_id_of_def(def)._1;
+    auto var_t = ty::expr_ty(*cx.tcx, ex);
+    for (restrict r in sc) {
+        // excludes variables introduced since the alias was made
+        if (my_defnum < r.block_defnum) {
+            for (ty::t t in r.tys) {
+                if (ty_can_unsafely_include(cx, t, var_t, assign)) {
+                    r.ok = val_taken(ex.span, p);
+                }
+            }
+        } else if (r.ok != valid && vec::member(my_defnum, r.bindings)) {
+            fail_alias(cx, r.ok, p);
+        }
+    }
+}
+
+fn fail_alias(&ctx cx, valid issue, &ast::path pt) {
+    auto base = " will invalidate alias " + ast::path_name(pt) +
+        ", which is still used";
+    alt (issue) {
+        case (overwritten(?sp, ?wpt)) {
+            cx.tcx.sess.span_err
+                (sp, "overwriting " + ast::path_name(wpt) + base);
+        }
+        case (val_taken(?sp, ?vpt)) {
+            cx.tcx.sess.span_err
+                (sp, "taking the value of " + ast::path_name(vpt) +
+                 base);
+        }
+    }
+}
+
+fn check_assign(&@ctx cx, &@ast::expr dest, &@ast::expr src,
+                &scope sc, &vt[scope] v) {
+    visit_expr(cx, src, sc, v);
+    alt (dest.node) {
+        case (ast::expr_path(?p, ?ann)) {
+            auto dnum = ast::def_id_of_def(cx.dm.get(ann.id))._1;
+            auto var_t = ty::expr_ty(*cx.tcx, dest);
+            for (restrict r in sc) {
+                if (vec::member(dnum, r.root_vars)) {
+                    r.ok = overwritten(dest.span, p);
+                }
+            }
+            check_var(*cx, dest, p, ann, true, sc);
         }
         case (_) {
-            ret none[def_id];
+            visit_expr(cx, dest, sc, v);
         }
     }
 }
 
 fn expr_root(&ctx cx, @ast::expr ex, bool autoderef)
-    -> rec(@ast::expr ex, vec[deref] ds) {
-    let vec[deref] ds = [];
-    if (autoderef) {
-        auto auto_unbox = maybe_auto_unbox(cx, ex);
-        if (auto_unbox.done) {
-            vec::push(ds, rec(mut=auto_unbox.mut, t=unbox));
-        }
-    }
+    -> rec(@ast::expr ex, option::t[ty::t] inner_mut, bool mut_in_box) {
+    let option::t[ty::t] mut = none;
+    // This is not currently used but would make it possible to be more
+    // liberal -- only stuff in a mutable box needs full type-inclusion
+    // checking, things that aren't in a box need only be checked against
+    // locally live aliases and their root.
+    auto mut_in_box = false;
     while (true) {
         alt ({ex.node}) {
             case (ast::expr_field(?base, ?ident, _)) {
-                auto auto_unbox = maybe_auto_unbox(cx, base);
-                alt (auto_unbox.t) {
+                auto base_t = ty::expr_ty(*cx.tcx, base);
+                auto auto_unbox = maybe_auto_unbox(cx, base_t);
+                alt (ty::struct(*cx.tcx, auto_unbox.t)) {
                     case (ty::ty_tup(?fields)) {
                         auto fnm = ty::field_num(cx.tcx.sess, ex.span, ident);
-                        auto mt = fields.(fnm).mut != ast::imm;
-                        vec::push(ds, rec(mut=mt, t=field(ident)));
+                        if (fields.(fnm).mut != ast::imm && is_none(mut)) {
+                            mut = some(auto_unbox.t);
+                        }
                     }
                     case (ty::ty_rec(?fields)) {
                         for (ty::field fld in fields) {
                             if (str::eq(ident, fld.ident)) {
-                                auto mt = fld.mt.mut != ast::imm;
-                                vec::push(ds, rec(mut=mt, t=field(ident)));
+                                if (fld.mt.mut != ast::imm && is_none(mut)) {
+                                    mut = some(auto_unbox.t);
+                                }
                                 break;
                             }
                         }
                     }
-                    case (ty::ty_obj(_)) {
-                        vec::push(ds, rec(mut=false, t=field(ident)));
-                    }
+                    case (ty::ty_obj(_)) {}
                 }
                 if (auto_unbox.done) {
-                    vec::push(ds, rec(mut=auto_unbox.mut, t=unbox));
+                    if (!is_none(mut)) { mut_in_box = true; }
+                    else if (auto_unbox.mut) { mut = some(base_t); }
                 }
                 ex = base;
             }
             case (ast::expr_index(?base, _, _)) {
-                auto auto_unbox = maybe_auto_unbox(cx, base);
-                alt (auto_unbox.t) {
+                auto base_t = ty::expr_ty(*cx.tcx, base);
+                auto auto_unbox = maybe_auto_unbox(cx, base_t);
+                alt (ty::struct(*cx.tcx, auto_unbox.t)) {
                     case (ty::ty_vec(?mt)) {
-                        vec::push(ds, rec(mut=mt.mut != ast::imm, t=index));
+                        if (mt.mut != ast::imm && is_none(mut)) {
+                            mut = some(auto_unbox.t);
+                        }
                     }
                 }
                 if (auto_unbox.done) {
-                    vec::push(ds, rec(mut=auto_unbox.mut, t=unbox));
+                    if (!is_none(mut)) { mut_in_box = true; }
+                    else if (auto_unbox.mut) { mut = some(base_t); }
+                }
+                if (auto_unbox.done && !is_none(mut)) {
                 }
                 ex = base;
             }
             case (ast::expr_unary(?op, ?base, _)) {
                 if (op == ast::deref) {
-                    alt (ty::struct(*cx.tcx, ty::expr_ty(*cx.tcx, base))) {
+                    auto base_t = ty::expr_ty(*cx.tcx, base);
+                    alt (ty::struct(*cx.tcx, base_t)) {
                         case (ty::ty_box(?mt)) {
-                            vec::push(ds, rec(mut=mt.mut!=ast::imm, t=unbox));
+                            if (mt.mut != ast::imm && is_none(mut)) {
+                                mut = some(base_t);
+                            }
+                            if (!is_none(mut)) {
+                                mut_in_box = true;
+                            }
                         }
                     }
                     ex = base;
@@ -214,34 +393,100 @@ fn expr_root(&ctx cx, @ast::expr ex, bool autoderef)
             case (_) { break; }
         }
     }
-    vec::reverse(ds);
-    ret rec(ex = ex, ds = ds);
+    if (autoderef) {
+        auto ex_t = ty::expr_ty(*cx.tcx, ex);
+        auto auto_unbox = maybe_auto_unbox(cx, ex_t);
+        if (auto_unbox.done) {
+            if (!is_none(mut)) { mut_in_box = true; }
+            else if (auto_unbox.mut) { mut = some(ex_t); }
+        }
+    }
+    ret rec(ex = ex, inner_mut = mut, mut_in_box = mut_in_box);
 }
 
-fn maybe_auto_unbox(&ctx cx, &@ast::expr ex)
-    -> rec(ty::sty t, bool done, bool mut) {
-    auto tp = ty::struct(*cx.tcx, ty::expr_ty(*cx.tcx, ex));
-    alt (tp) {
+fn maybe_auto_unbox(&ctx cx, &ty::t t)
+    -> rec(ty::t t, bool done, bool mut) {
+    alt (ty::struct(*cx.tcx, t)) {
         case (ty::ty_box(?mt)) {
-            ret rec(t=ty::struct(*cx.tcx, mt.ty),
-                    done=true, mut=mt.mut != ast::imm);
+            ret rec(t=mt.ty, done=true, mut=mt.mut != ast::imm);
+        }
+        case (_) {
+            ret rec(t=t, done=false, mut=false);
         }
-        case (_) { ret rec(t=tp, done=false, mut=false); }
     }
 }
 
-fn has_unsafe_box(&vec[deref] ds) -> bool {
-    auto saw_mut = false;
-    for (deref d in ds) {
-        if (d.mut) { saw_mut = true; }
-        if (d.t == unbox) {
-            // Directly aliasing the content of a mutable box is never okay,
-            // and any box living under mutable connection may be severed from
-            // its root and freed.
-            if (saw_mut) { ret true; }
+fn path_def_id(&ctx cx, &@ast::expr ex) -> option::t[ast::def_id] {
+    alt (ex.node) {
+        case (ast::expr_path(_, ?ann)) {
+            ret some(ast::def_id_of_def(cx.dm.get(ann.id)));
+        }
+        case (_) {
+            ret none;
         }
     }
-    ret false;
+}
+
+fn ty_can_unsafely_include(&ctx cx, ty::t needle, ty::t haystack, bool mut)
+    -> bool {
+    fn get_mut(bool cur, &ty::mt mt) -> bool {
+        ret cur || mt.mut != ast::imm;
+    }
+    fn helper(&ty::ctxt tcx, ty::t needle, ty::t haystack, bool mut) -> bool {
+        if (needle == haystack) { ret true; }
+        alt (ty::struct(tcx, haystack)) {
+            case (ty::ty_tag(_, ?ts)) {
+                for (ty::t t in ts) {
+                    if (helper(tcx, needle, t, mut)) { ret true; }
+                }
+                ret false;
+            }
+            case (ty::ty_box(?mt)) {
+                ret helper(tcx, needle, mt.ty, get_mut(mut, mt));
+            }
+            case (ty::ty_vec(?mt)) {
+                ret helper(tcx, needle, mt.ty, get_mut(mut, mt));
+            }
+            case (ty::ty_ptr(?mt)) {
+                ret helper(tcx, needle, mt.ty, get_mut(mut, mt));
+            }
+            case (ty::ty_tup(?mts)) {
+                for (ty::mt mt in mts) {
+                    if (helper(tcx, needle, mt.ty, get_mut(mut, mt))) {
+                        ret true;
+                    }
+                }
+                ret false;
+            }
+            case (ty::ty_rec(?fields)) {
+                for (ty::field f in fields) {
+                    if (helper(tcx, needle, f.mt.ty, get_mut(mut, f.mt))) {
+                        ret true;
+                    }
+                }
+                ret false;
+            }
+            // These may contain anything.
+            case (ty::ty_fn(_, _, _, _)) { ret true; }
+            case (ty::ty_obj(_)) { ret true; }
+            // A type param may include everything, but can only be treated as
+            // opaque downstream, and is thus safe unless we saw mutable
+            // fields, in which case the whole thing can be overwritten.
+            case (ty::ty_param(_)) { ret mut; }
+            case (_) { ret false; }
+        }
+    }
+    ret helper(*cx.tcx, needle, haystack, mut);
+}
+
+fn def_is_local(&ast::def d) -> bool {
+    ret alt (d) {
+        case (ast::def_local(_)) { true }
+        case (ast::def_arg(_)) { true }
+        case (ast::def_obj_field(_)) { true }
+        case (ast::def_binding(_)) { true }
+        case (_) { false }
+    };
 }
 
 // Local Variables:
diff --git a/src/comp/middle/resolve.rs b/src/comp/middle/resolve.rs
index 8ec4dfe6297e..1bd7307a5abf 100644
--- a/src/comp/middle/resolve.rs
+++ b/src/comp/middle/resolve.rs
@@ -307,7 +307,7 @@ fn resolve_names(&@env e, &ast::crate c) {
                     }
                     case (_) {
                         e.sess.span_err(p.span, "not a tag variant: " +
-                                        str::connect(p.node.idents, "::"));
+                                        ast::path_name(p));
                     }
                 }
                 for (@ast::pat child in children) {
diff --git a/src/test/compile-fail/unsafe-alias-2.rs b/src/test/compile-fail/unsafe-alias-2.rs
new file mode 100644
index 000000000000..4a9551a9130f
--- /dev/null
+++ b/src/test/compile-fail/unsafe-alias-2.rs
@@ -0,0 +1,15 @@
+// error-pattern:invalidate alias x
+
+fn whoknows(@mutable int x) {
+    *x = 10;
+}
+
+fn main() {
+    auto box = @mutable 1;
+    alt (*box) {
+        case (?x) {
+            whoknows(box);
+            log_err x;
+        }
+    }
+}
diff --git a/src/test/compile-fail/unsafe-alias.rs b/src/test/compile-fail/unsafe-alias.rs
index e4ff5f86da2c..a81b490266d8 100644
--- a/src/test/compile-fail/unsafe-alias.rs
+++ b/src/test/compile-fail/unsafe-alias.rs
@@ -1,10 +1,14 @@
-// error-pattern:can not create alias
+// error-pattern:may alias with argument
 
-fn foo(&int x) {
+fn foo(&int x, fn() f) {
     log x;
 }
 
+fn whoknows(@mutable int x) {
+    *x = 10;
+}
+
 fn main() {
     auto box = @mutable 1;
-    foo(*box);
+    foo(*box, bind whoknows(box));
 }
diff --git a/src/test/compile-fail/unsafe-alt.rs b/src/test/compile-fail/unsafe-alt.rs
index a0342d87c867..4144e3751db0 100644
--- a/src/test/compile-fail/unsafe-alt.rs
+++ b/src/test/compile-fail/unsafe-alt.rs
@@ -1,4 +1,4 @@
-// error-pattern:x is being aliased
+// error-pattern:invalidate alias i
 
 tag foo {
     left(int);
@@ -10,6 +10,7 @@ fn main() {
     alt (x) {
         case (left(?i)) {
             x = right(false);
+            log i;
         }
         case (_) {}
     }