Skip to content

Commit

Permalink
Remove cat_discr
Browse files Browse the repository at this point in the history
it seems to be some kind of GC-related mess
  • Loading branch information
arielb1 committed Oct 27, 2014
1 parent c121cba commit 19faaf1
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 133 deletions.
1 change: 0 additions & 1 deletion src/librustc/middle/borrowck/check_loans.rs
Expand Up @@ -831,7 +831,6 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
return;
}

mc::cat_discr(b, _) |
mc::cat_deref(b, _, mc::OwnedPtr) => {
assert_eq!(cmt.mutbl, mc::McInherited);
cmt = b;
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/middle/borrowck/gather_loans/gather_moves.rs
Expand Up @@ -159,8 +159,7 @@ fn check_and_get_illegal_move_origin(bccx: &BorrowckCtxt,
}
}

mc::cat_deref(ref b, _, mc::OwnedPtr) |
mc::cat_discr(ref b, _) => {
mc::cat_deref(ref b, _, mc::OwnedPtr) => {
check_and_get_illegal_move_origin(bccx, b)
}
}
Expand Down
59 changes: 1 addition & 58 deletions src/librustc/middle/borrowck/gather_loans/lifetime.rs
Expand Up @@ -84,62 +84,6 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
mc::cat_interior(ref base, _) => { // L-Field
self.check(base, discr_scope)
}

mc::cat_discr(ref base, new_discr_scope) => {
// Subtle: in a match, we must ensure that each binding
// variable remains valid for the duration of the arm in
// which it appears, presuming that this arm is taken.
// But it is inconvenient in trans to root something just
// for one arm. Therefore, we insert a cat_discr(),
// basically a special kind of category that says "if this
// value must be dynamically rooted, root it for the scope
// `match_id`".
//
// As an example, consider this scenario:
//
// let mut x = @Some(3);
// match *x { Some(y) {...} None {...} }
//
// Technically, the value `x` need only be rooted
// in the `some` arm. However, we evaluate `x` in trans
// before we know what arm will be taken, so we just
// always root it for the duration of the match.
//
// As a second example, consider *this* scenario:
//
// let x = @@Some(3);
// match x { @@Some(y) {...} @@None {...} }
//
// Here again, `x` need only be rooted in the `some` arm.
// In this case, the value which needs to be rooted is
// found only when checking which pattern matches: but
// this check is done before entering the arm. Therefore,
// even in this case we just choose to keep the value
// rooted for the entire match. This means the value will be
// rooted even if the none arm is taken. Oh well.
//
// At first, I tried to optimize the second case to only
// root in one arm, but the result was suboptimal: first,
// it interfered with the construction of phi nodes in the
// arm, as we were adding code to root values before the
// phi nodes were added. This could have been addressed
// with a second basic block. However, the naive approach
// also yielded suboptimal results for patterns like:
//
// let x = @@...;
// match x { @@some_variant(y) | @@some_other_variant(y) =>
//
// The reason is that we would root the value once for
// each pattern and not once per arm. This is also easily
// fixed, but it's yet more code for what is really quite
// the corner case.
//
// Nonetheless, if you decide to optimize this case in the
// future, you need only adjust where the cat_discr()
// node appears to draw the line between what will be rooted
// in the *arm* vs the *match*.
self.check(base, Some(new_discr_scope))
}
}
}

Expand Down Expand Up @@ -182,8 +126,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
}
mc::cat_downcast(ref cmt) |
mc::cat_deref(ref cmt, _, mc::OwnedPtr) |
mc::cat_interior(ref cmt, _) |
mc::cat_discr(ref cmt, _) => {
mc::cat_interior(ref cmt, _) => {
self.scope(cmt)
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/librustc/middle/borrowck/gather_loans/mod.rs
Expand Up @@ -213,9 +213,7 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> {
/*!
* Guarantees that `addr_of(cmt)` will be valid for the duration of
* `static_scope_r`, or reports an error. This may entail taking
* out loans, which will be added to the `req_loan_map`. This can
* also entail "rooting" GC'd pointers, which means ensuring
* dynamically that they are not freed.
* out loans, which will be added to the `req_loan_map`.
*/

debug!("guarantee_valid(borrow_id={}, cmt={}, \
Expand Down
77 changes: 28 additions & 49 deletions src/librustc/middle/borrowck/gather_loans/restrictions.rs
Expand Up @@ -96,46 +96,27 @@ impl<'a, 'tcx> RestrictionsContext<'a, 'tcx> {
self.extend(result, cmt.mutbl, LpInterior(i))
}

mc::cat_deref(cmt_base, _, pk @ mc::OwnedPtr) => {
// R-Deref-Send-Pointer
//
// When we borrow the interior of an owned pointer, we
// cannot permit the base to be mutated, because that
// would cause the unique pointer to be freed.
//
// Eventually we should make these non-special and
// just rely on Deref<T> implementation.
let result = self.restrict(cmt_base);
self.extend(result, cmt.mutbl, LpDeref(pk))
}

mc::cat_static_item(..) => {
Safe
}

mc::cat_deref(cmt_base, _, mc::BorrowedPtr(ty::ImmBorrow, lt)) |
mc::cat_deref(cmt_base, _, mc::Implicit(ty::ImmBorrow, lt)) => {
// R-Deref-Imm-Borrowed
if !self.bccx.is_subregion_of(self.loan_region, lt) {
self.bccx.report(
BckError {
span: self.span,
cause: self.cause,
cmt: cmt_base,
code: err_borrowed_pointer_too_short(
self.loan_region, lt)});
return Safe;
}
Safe
}

mc::cat_deref(cmt_base, _, pk) => {
match pk {
mc::BorrowedPtr(ty::MutBorrow, lt) |
mc::BorrowedPtr(ty::UniqueImmBorrow, lt) |
mc::Implicit(ty::MutBorrow, lt) |
mc::Implicit(ty::UniqueImmBorrow, lt) => {
// R-Deref-Mut-Borrowed
mc::OwnedPtr => {
// R-Deref-Send-Pointer
//
// When we borrow the interior of an owned pointer, we
// cannot permit the base to be mutated, because that
// would cause the unique pointer to be freed.
//
// Eventually we should make these non-special and
// just rely on Deref<T> implementation.
let result = self.restrict(cmt_base);
self.extend(result, cmt.mutbl, LpDeref(pk))
}
mc::Implicit(bk, lt) | mc::BorrowedPtr(bk, lt) => {
// R-Deref-[Mut-]Borrowed
if !self.bccx.is_subregion_of(self.loan_region, lt) {
self.bccx.report(
BckError {
Expand All @@ -147,25 +128,23 @@ impl<'a, 'tcx> RestrictionsContext<'a, 'tcx> {
return Safe;
}

let result = self.restrict(cmt_base);
self.extend(result, cmt.mutbl, LpDeref(pk))
}
mc::UnsafePtr(..) => {
// We are very trusting when working with unsafe
// pointers.
Safe
}
_ => {
self.bccx.tcx.sess.span_bug(self.span,
"unhandled memcat in \
cat_deref")
match bk {
ty::ImmBorrow => Safe,
ty::MutBorrow | ty::UniqueImmBorrow => {
// R-Deref-Mut-Borrowed
//
// The referent can be aliased after the
// references lifetime ends (by a newly-unfrozen
// borrow).
let result = self.restrict(cmt_base);
self.extend(result, cmt.mutbl, LpDeref(pk))
}
}
}
// Borrowck is not relevant for unsafe pointers
mc::UnsafePtr(..) => Safe
}
}

mc::cat_discr(cmt_base, _) => {
self.restrict(cmt_base)
}
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/librustc/middle/borrowck/mod.rs
Expand Up @@ -379,8 +379,7 @@ pub fn opt_loan_path(cmt: &mc::cmt) -> Option<Rc<LoanPath>> {
})
}

mc::cat_downcast(ref cmt_base) |
mc::cat_discr(ref cmt_base, _) => {
mc::cat_downcast(ref cmt_base) => {
opt_loan_path(cmt_base)
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/middle/check_static.rs
Expand Up @@ -269,7 +269,6 @@ impl euv::Delegate for GlobalChecker {
break
}
mc::cat_deref(ref cmt, _, _) |
mc::cat_discr(ref cmt, _) |
mc::cat_downcast(ref cmt) |
mc::cat_interior(ref cmt, _) => cur = cmt,

Expand Down Expand Up @@ -307,7 +306,6 @@ impl euv::Delegate for GlobalChecker {
}

mc::cat_downcast(..) |
mc::cat_discr(..) |
mc::cat_upvar(..) => unreachable!(),

mc::cat_local(..) => {
Expand All @@ -331,4 +329,3 @@ impl euv::Delegate for GlobalChecker {
_cmt: mc::cmt,
_mode: euv::ConsumeMode) {}
}

11 changes: 1 addition & 10 deletions src/librustc/middle/mem_categorization.rs
Expand Up @@ -87,7 +87,6 @@ pub enum categorization {
cat_deref(cmt, uint, PointerKind), // deref of a ptr
cat_interior(cmt, InteriorKind), // something interior: field, tuple, etc
cat_downcast(cmt), // selects a particular enum variant (*1)
cat_discr(cmt, ast::NodeId), // match discriminant (see preserve())

// (*1) downcast is only required if the enum has more than one variant
}
Expand Down Expand Up @@ -1339,9 +1338,6 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
cat_upvar(ref var) => {
upvar_to_string(var, true)
}
cat_discr(ref cmt, _) => {
self.cmt_to_string(&**cmt)
}
cat_downcast(ref cmt) => {
self.cmt_to_string(&**cmt)
}
Expand Down Expand Up @@ -1379,7 +1375,6 @@ impl cmt_ {
Rc::new((*self).clone())
}
cat_downcast(ref b) |
cat_discr(ref b, _) |
cat_interior(ref b, _) |
cat_deref(ref b, _, OwnedPtr) => {
b.guarantor()
Expand All @@ -1404,8 +1399,7 @@ impl cmt_ {
cat_deref(ref b, _, Implicit(ty::UniqueImmBorrow, _)) |
cat_downcast(ref b) |
cat_deref(ref b, _, OwnedPtr) |
cat_interior(ref b, _) |
cat_discr(ref b, _) => {
cat_interior(ref b, _) => {
// Aliasability depends on base cmt
b.freely_aliasable(ctxt)
}
Expand Down Expand Up @@ -1490,9 +1484,6 @@ impl Repr for categorization {
cat_downcast(ref cmt) => {
format!("{}->(enum)", cmt.cat.repr(tcx))
}
cat_discr(ref cmt, _) => {
cmt.cat.repr(tcx)
}
}
}
}
Expand Down
7 changes: 2 additions & 5 deletions src/librustc/middle/typeck/check/regionck.rs
Expand Up @@ -1494,7 +1494,6 @@ fn link_region(rcx: &Rcx,
}
}

mc::cat_discr(cmt_base, _) |
mc::cat_downcast(cmt_base) |
mc::cat_deref(cmt_base, _, mc::OwnedPtr) |
mc::cat_interior(cmt_base, _) => {
Expand Down Expand Up @@ -1736,8 +1735,7 @@ fn adjust_upvar_borrow_kind_for_mut(rcx: &Rcx,
match cmt.cat.clone() {
mc::cat_deref(base, _, mc::OwnedPtr) |
mc::cat_interior(base, _) |
mc::cat_downcast(base) |
mc::cat_discr(base, _) => {
mc::cat_downcast(base) => {
// Interior or owned data is mutable if base is
// mutable, so iterate to the base.
cmt = base;
Expand Down Expand Up @@ -1788,8 +1786,7 @@ fn adjust_upvar_borrow_kind_for_unique(rcx: &Rcx, cmt: mc::cmt) {
match cmt.cat.clone() {
mc::cat_deref(base, _, mc::OwnedPtr) |
mc::cat_interior(base, _) |
mc::cat_downcast(base) |
mc::cat_discr(base, _) => {
mc::cat_downcast(base) => {
// Interior or owned data is unique if base is
// unique.
cmt = base;
Expand Down

5 comments on commit 19faaf1

@bors
Copy link
Contributor

@bors bors commented on 19faaf1 Oct 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from nikomatsakis
at arielb1@19faaf1

@bors
Copy link
Contributor

@bors bors commented on 19faaf1 Oct 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging arielb1/rust/remaining-garbage = 19faaf1 into auto

@bors
Copy link
Contributor

@bors bors commented on 19faaf1 Oct 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arielb1/rust/remaining-garbage = 19faaf1 merged ok, testing candidate = e05c3b7

@bors
Copy link
Contributor

@bors bors commented on 19faaf1 Oct 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 19faaf1 Oct 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = e05c3b7

Please sign in to comment.