Skip to content

Commit

Permalink
Modify the ExprUseVisitor to walk each part of an AutoRef, and in
Browse files Browse the repository at this point in the history
particular to treat an AutoUnsize as as kind of "instantaneous" borrow
of the value being unsized. This prevents us from feeding uninitialized
data.

This caused a problem for the eager reborrow of comparison traits,
because that wound up introducing a "double AutoRef", which was not
being thoroughly checked before but turned out not to type check.
Fortunately, we can just remove that "eager reborrow" as it is no longer
needed now that `PartialEq` doesn't force both LHS and RHS to have the
same type (and even if we did have this problem, the better way would be
to lean on introducing a common supertype).
  • Loading branch information
nikomatsakis committed Apr 8, 2015
1 parent ce97c19 commit 1e79870
Show file tree
Hide file tree
Showing 9 changed files with 226 additions and 74 deletions.
14 changes: 13 additions & 1 deletion src/librustc/middle/check_const.rs
Expand Up @@ -662,7 +662,19 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for CheckCrateVisitor<'a, 'tcx> {
cmt: mc::cmt<'tcx>,
_loan_region: ty::Region,
bk: ty::BorrowKind,
loan_cause: euv::LoanCause) {
loan_cause: euv::LoanCause)
{
// Kind of hacky, but we allow Unsafe coercions in constants.
// These occur when we convert a &T or *T to a *U, as well as
// when making a thin pointer (e.g., `*T`) into a fat pointer
// (e.g., `*Trait`).
match loan_cause {
euv::LoanCause::AutoUnsafe => {
return;
}
_ => { }
}

let mut cur = &cmt;
let mut is_interior = false;
loop {
Expand Down
173 changes: 145 additions & 28 deletions src/librustc/middle/expr_use_visitor.rs
Expand Up @@ -99,6 +99,7 @@ pub enum LoanCause {
ClosureCapture(Span),
AddrOf,
AutoRef,
AutoUnsafe,
RefBinding,
OverloadedOperator,
ClosureInvocation,
Expand Down Expand Up @@ -800,18 +801,8 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> {
return_if_err!(self.mc.cat_expr_unadjusted(expr));
self.delegate_consume(expr.id, expr.span, cmt_unadjusted);
}
ty::AdjustDerefRef(ty::AutoDerefRef {
autoref: ref opt_autoref,
autoderefs: n
}) => {
self.walk_autoderefs(expr, n);

match *opt_autoref {
None => { }
Some(ref r) => {
self.walk_autoref(expr, r, n);
}
}
ty::AdjustDerefRef(ref adj) => {
self.walk_autoderefref(expr, adj);
}
}
}
Expand Down Expand Up @@ -852,39 +843,165 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> {
}
}

fn walk_autoderefref(&mut self,
expr: &ast::Expr,
adj: &ty::AutoDerefRef<'tcx>) {
debug!("walk_autoderefref expr={} adj={}",
expr.repr(self.tcx()),
adj.repr(self.tcx()));

self.walk_autoderefs(expr, adj.autoderefs);

// Weird hacky special case: AutoUnsizeUniq, which converts
// from a ~T to a ~Trait etc, always comes in a stylized
// fashion. In particular, we want to consume the ~ pointer
// being dereferenced, not the dereferenced content (as the
// content is, at least for upcasts, unsized).
match adj.autoref {
Some(ty::AutoUnsizeUniq(_)) => {
assert!(adj.autoderefs == 1,
format!("Expected exactly 1 deref with Uniq AutoRefs, found: {}",
adj.autoderefs));
let cmt_unadjusted =
return_if_err!(self.mc.cat_expr_unadjusted(expr));
self.delegate_consume(expr.id, expr.span, cmt_unadjusted);
return;
}
_ => { }
}

let autoref = adj.autoref.as_ref();
let cmt_derefd = return_if_err!(
self.mc.cat_expr_autoderefd(expr, adj.autoderefs));
self.walk_autoref(expr, &cmt_derefd, autoref);
}

/// Walks the autoref `opt_autoref` applied to the autoderef'd
/// `expr`. `cmt_derefd` is the mem-categorized form of `expr`
/// after all relevant autoderefs have occurred. Because AutoRefs
/// can be recursive, this function is recursive: it first walks
/// deeply all the way down the autoref chain, and then processes
/// the autorefs on the way out. At each point, it returns the
/// `cmt` for the rvalue that will be produced by introduced an
/// autoref.
fn walk_autoref(&mut self,
expr: &ast::Expr,
autoref: &ty::AutoRef,
n: usize) {
debug!("walk_autoref expr={}", expr.repr(self.tcx()));
cmt_derefd: &mc::cmt<'tcx>,
opt_autoref: Option<&ty::AutoRef<'tcx>>)
-> mc::cmt<'tcx>
{
debug!("walk_autoref(expr.id={} cmt_derefd={} opt_autoref={:?})",
expr.id,
cmt_derefd.repr(self.tcx()),
opt_autoref);

let autoref = match opt_autoref {
Some(autoref) => autoref,
None => {
// No recursive step here, this is a base case.
return cmt_derefd.clone();
}
};

match *autoref {
ty::AutoPtr(r, m, _) => {
let cmt_derefd = return_if_err!(
self.mc.cat_expr_autoderefd(expr, n));
debug!("walk_adjustment: cmt_derefd={}",
cmt_derefd.repr(self.tcx()));
ty::AutoPtr(r, m, ref baseref) => {
let cmt_base = self.walk_autoref_recursively(expr, cmt_derefd, baseref);

debug!("walk_autoref: expr.id={} cmt_base={}",
expr.id,
cmt_base.repr(self.tcx()));

self.delegate.borrow(expr.id,
expr.span,
cmt_derefd,
cmt_base,
r,
ty::BorrowKind::from_mutbl(m),
AutoRef);
}
ty::AutoUnsize(_) |

ty::AutoUnsize(_) => {
// Converting a `[T; N]` to `[T]` or `T` to `Trait`
// isn't really a borrow, move, etc, in and of itself.
// Also, no recursive step here, this is a base case.

// It may seem a bit odd to return the cmt_derefd
// unmodified here, but in fact I think it's the right
// thing to do. Essentially the unsize transformation
// isn't really relevant to the borrowing rules --
// it's best thought of as a kind of side-modifier to
// the autoref, adding additional data that is
// attached to the pointer that is produced, but not
// affecting the data being borrowed in any other
// way. To see what I mean, consider this example:
//
// fn foo<'a>(&'a self) -> &'a Trait { self }
//
// This is valid because the underlying `self` value
// lives for the lifetime 'a. If we were to treat the
// "unsizing" as e.g. producing an rvalue, that would
// only be valid for the temporary scope, which isn't
// enough to justify the return value, which have the
// lifetime 'a.
//
// Another option would be to add a variant for
// categorization (like downcast) that wraps
// cmt_derefd and represents the unsizing operation.
// But I don't think there is any particular use for
// this (yet). -nmatsakis
return cmt_derefd.clone();
}

ty::AutoUnsizeUniq(_) => {
assert!(n == 1, format!("Expected exactly 1 deref with Uniq \
AutoRefs, found: {}", n));
let cmt_unadjusted =
return_if_err!(self.mc.cat_expr_unadjusted(expr));
self.delegate_consume(expr.id, expr.span, cmt_unadjusted);
// these are handled via special case above
self.tcx().sess.span_bug(expr.span, "nexpected AutoUnsizeUniq");
}
ty::AutoUnsafe(..) => {

ty::AutoUnsafe(m, ref baseref) => {
let cmt_base = self.walk_autoref_recursively(expr, cmt_derefd, baseref);

debug!("walk_autoref: expr.id={} cmt_base={}",
expr.id,
cmt_base.repr(self.tcx()));

// Converting from a &T to *T (or &mut T to *mut T) is
// treated as borrowing it for the enclosing temporary
// scope.
let r = ty::ReScope(region::CodeExtent::from_node_id(expr.id));

self.delegate.borrow(expr.id,
expr.span,
cmt_base,
r,
ty::BorrowKind::from_mutbl(m),
AutoUnsafe);
}
}

// Construct the categorization for the result of the autoref.
// This is always an rvalue, since we are producing a new
// (temporary) indirection.

let adj_ty =
ty::adjust_ty_for_autoref(self.tcx(),
expr.span,
cmt_derefd.ty,
opt_autoref);

self.mc.cat_rvalue_node(expr.id, expr.span, adj_ty)
}

fn walk_autoref_recursively(&mut self,
expr: &ast::Expr,
cmt_derefd: &mc::cmt<'tcx>,
autoref: &Option<Box<ty::AutoRef<'tcx>>>)
-> mc::cmt<'tcx>
{
// Shuffle from a ref to an optional box to an optional ref.
let autoref: Option<&ty::AutoRef<'tcx>> = autoref.as_ref().map(|b| &**b);
self.walk_autoref(expr, cmt_derefd, autoref)
}


// When this returns true, it means that the expression *is* a
// method-call (i.e. via the operator-overload). This true result
// also implies that walk_overloaded_operator already took care of
Expand Down
24 changes: 14 additions & 10 deletions src/librustc/middle/mem_categorization.rs
Expand Up @@ -833,6 +833,15 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
ret
}

/// Returns the lifetime of a temporary created by expr with id `id`.
/// This could be `'static` if `id` is part of a constant expression.
pub fn temporary_scope(&self, id: ast::NodeId) -> ty::Region {
match self.typer.temporary_scope(id) {
Some(scope) => ty::ReScope(scope),
None => ty::ReStatic
}
}

pub fn cat_rvalue_node(&self,
id: ast::NodeId,
span: Span,
Expand All @@ -848,17 +857,12 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
_ => check_const::NOT_CONST
};

// Compute maximum lifetime of this rvalue. This is 'static if
// we can promote to a constant, otherwise equal to enclosing temp
// lifetime.
let re = match qualif & check_const::NON_STATIC_BORROWS {
check_const::PURE_CONST => {
// Constant rvalues get promoted to 'static.
ty::ReStatic
}
_ => {
match self.typer.temporary_scope(id) {
Some(scope) => ty::ReScope(scope),
None => ty::ReStatic
}
}
check_const::PURE_CONST => ty::ReStatic,
_ => self.temporary_scope(id),
};
let ret = self.cat_rvalue(id, span, re, expr_ty);
debug!("cat_rvalue_node ret {}", ret.repr(self.tcx()));
Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/borrowck/check_loans.rs
Expand Up @@ -542,6 +542,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
euv::OverloadedOperator(..) |
euv::AddrOf(..) |
euv::AutoRef(..) |
euv::AutoUnsafe(..) |
euv::ClosureInvocation(..) |
euv::ForLoop(..) |
euv::RefBinding(..) |
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_borrowck/borrowck/mod.rs
Expand Up @@ -775,6 +775,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
euv::AddrOf |
euv::RefBinding |
euv::AutoRef |
euv::AutoUnsafe |
euv::ForLoop |
euv::MatchDiscriminant => {
format!("cannot borrow {} as mutable", descr)
Expand Down Expand Up @@ -822,6 +823,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
BorrowViolation(euv::OverloadedOperator) |
BorrowViolation(euv::AddrOf) |
BorrowViolation(euv::AutoRef) |
BorrowViolation(euv::AutoUnsafe) |
BorrowViolation(euv::RefBinding) |
BorrowViolation(euv::MatchDiscriminant) => {
"cannot borrow data mutably"
Expand Down
32 changes: 2 additions & 30 deletions src/librustc_typeck/check/op.rs
Expand Up @@ -20,7 +20,6 @@ use super::{
PreferMutLvalue,
structurally_resolved_type,
};
use middle::infer;
use middle::traits;
use middle::ty::{self, Ty};
use syntax::ast;
Expand Down Expand Up @@ -314,36 +313,9 @@ fn lookup_op_method<'a, 'tcx>(fcx: &'a FnCtxt<'a, 'tcx>,

let method = match trait_did {
Some(trait_did) => {
// We do eager coercions to make using operators
// more ergonomic:
//
// - If the input is of type &'a T (resp. &'a mut T),
// then reborrow it to &'b T (resp. &'b mut T) where
// 'b <= 'a. This makes things like `x == y`, where
// `x` and `y` are both region pointers, work. We
// could also solve this with variance or different
// traits that don't force left and right to have same
// type.
let (adj_ty, adjustment) = match lhs_ty.sty {
ty::ty_rptr(r_in, mt) => {
let r_adj = fcx.infcx().next_region_var(infer::Autoref(lhs_expr.span));
fcx.mk_subr(infer::Reborrow(lhs_expr.span), r_adj, *r_in);
let adjusted_ty = ty::mk_rptr(fcx.tcx(), fcx.tcx().mk_region(r_adj), mt);
let autoptr = ty::AutoPtr(r_adj, mt.mutbl, None);
let adjustment = ty::AutoDerefRef { autoderefs: 1, autoref: Some(autoptr) };
(adjusted_ty, adjustment)
}
_ => {
(lhs_ty, ty::AutoDerefRef { autoderefs: 0, autoref: None })
}
};

debug!("adjusted_ty={} adjustment={:?}",
adj_ty.repr(fcx.tcx()),
adjustment);

let noop = ty::AutoDerefRef { autoderefs: 0, autoref: None };
method::lookup_in_trait_adjusted(fcx, expr.span, Some(lhs_expr), opname,
trait_did, adjustment, adj_ty, Some(other_tys))
trait_did, noop, lhs_ty, Some(other_tys))
}
None => None
};
Expand Down
14 changes: 9 additions & 5 deletions src/librustc_typeck/check/regionck.rs
Expand Up @@ -1119,20 +1119,24 @@ fn link_pattern<'a, 'tcx>(rcx: &Rcx<'a, 'tcx>,
fn link_autoref(rcx: &Rcx,
expr: &ast::Expr,
autoderefs: usize,
autoref: &ty::AutoRef) {

autoref: &ty::AutoRef)
{
debug!("link_autoref(autoref={:?})", autoref);
let mc = mc::MemCategorizationContext::new(rcx.fcx);
let expr_cmt = ignore_err!(mc.cat_expr_autoderefd(expr, autoderefs));
debug!("expr_cmt={}", expr_cmt.repr(rcx.tcx()));

match *autoref {
ty::AutoPtr(r, m, _) => {
link_region(rcx, expr.span, r,
ty::BorrowKind::from_mutbl(m), expr_cmt);
link_region(rcx, expr.span, r, ty::BorrowKind::from_mutbl(m), expr_cmt);
}

ty::AutoUnsafe(m, _) => {
let r = ty::ReScope(CodeExtent::from_node_id(expr.id));
link_region(rcx, expr.span, r, ty::BorrowKind::from_mutbl(m), expr_cmt);
}

ty::AutoUnsafe(..) | ty::AutoUnsizeUniq(_) | ty::AutoUnsize(_) => {}
ty::AutoUnsizeUniq(_) | ty::AutoUnsize(_) => {}
}
}

Expand Down
20 changes: 20 additions & 0 deletions src/test/compile-fail/borrowck-use-uninitialized-in-cast-trait.rs
@@ -0,0 +1,20 @@
// Copyright 2015 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.

// Variation on `borrowck-use-uninitialized-in-cast` in which we do a
// trait cast from an uninitialized source. Issue #20791.

trait Foo { fn dummy(&self) { } }
impl Foo for i32 { }

fn main() {
let x: &i32;
let y = x as *const Foo; //~ ERROR use of possibly uninitialized variable: `*x`
}

0 comments on commit 1e79870

Please sign in to comment.