From 19faaf1a9eff62606e777207205c6fea49aa1751 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 9 Oct 2014 14:58:45 +0300 Subject: [PATCH] Remove cat_discr it seems to be some kind of GC-related mess --- src/librustc/middle/borrowck/check_loans.rs | 1 - .../borrowck/gather_loans/gather_moves.rs | 3 +- .../middle/borrowck/gather_loans/lifetime.rs | 59 +------------- .../middle/borrowck/gather_loans/mod.rs | 4 +- .../borrowck/gather_loans/restrictions.rs | 77 +++++++------------ src/librustc/middle/borrowck/mod.rs | 3 +- src/librustc/middle/check_static.rs | 3 - src/librustc/middle/mem_categorization.rs | 11 +-- src/librustc/middle/typeck/check/regionck.rs | 7 +- 9 files changed, 35 insertions(+), 133 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 5f09cafb5e26e..f5e849f41967d 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -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; diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index 7c037cf50ff29..a1c02b85e8f51 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -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) } } diff --git a/src/librustc/middle/borrowck/gather_loans/lifetime.rs b/src/librustc/middle/borrowck/gather_loans/lifetime.rs index 5018fe0f96e07..41fe77d5197fd 100644 --- a/src/librustc/middle/borrowck/gather_loans/lifetime.rs +++ b/src/librustc/middle/borrowck/gather_loans/lifetime.rs @@ -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)) - } } } @@ -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) } } diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index d28baf48ddcdf..1a12828922cbf 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -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={}, \ diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 15a7437b6a87a..6a6fc1760f2bb 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -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 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 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 { @@ -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) - } } } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 804ed8830303e..8a4625861b3a6 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -379,8 +379,7 @@ pub fn opt_loan_path(cmt: &mc::cmt) -> Option> { }) } - mc::cat_downcast(ref cmt_base) | - mc::cat_discr(ref cmt_base, _) => { + mc::cat_downcast(ref cmt_base) => { opt_loan_path(cmt_base) } } diff --git a/src/librustc/middle/check_static.rs b/src/librustc/middle/check_static.rs index 42bc645200337..dd862d53c475b 100644 --- a/src/librustc/middle/check_static.rs +++ b/src/librustc/middle/check_static.rs @@ -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, @@ -307,7 +306,6 @@ impl euv::Delegate for GlobalChecker { } mc::cat_downcast(..) | - mc::cat_discr(..) | mc::cat_upvar(..) => unreachable!(), mc::cat_local(..) => { @@ -331,4 +329,3 @@ impl euv::Delegate for GlobalChecker { _cmt: mc::cmt, _mode: euv::ConsumeMode) {} } - diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index e43996559417b..57efb90ac8b19 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -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 } @@ -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) } @@ -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() @@ -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) } @@ -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) - } } } } diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index b595b9b84aed8..08f83ad7e38ec 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -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, _) => { @@ -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; @@ -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;