Rollup merge of #24191 - nikomatsakis:issue-20791, r=pnkfelix
Modify the ExprUseVisitor to walk each part of an AutoRef, and in 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). Fixes #20791. r? @nrc
This commit is contained in:
commit
3b87140299
@ -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 {
|
||||
|
@ -99,6 +99,7 @@ pub enum LoanCause {
|
||||
ClosureCapture(Span),
|
||||
AddrOf,
|
||||
AutoRef,
|
||||
AutoUnsafe,
|
||||
RefBinding,
|
||||
OverloadedOperator,
|
||||
ClosureInvocation,
|
||||
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -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::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);
|
||||
|
||||
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::AutoUnsafe(..) => {
|
||||
|
||||
ty::AutoUnsizeUniq(_) => {
|
||||
// these are handled via special case above
|
||||
self.tcx().sess.span_bug(expr.span, "nexpected AutoUnsizeUniq");
|
||||
}
|
||||
|
||||
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
|
||||
|
@ -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,
|
||||
@ -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()));
|
||||
|
@ -542,6 +542,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
|
||||
euv::OverloadedOperator(..) |
|
||||
euv::AddrOf(..) |
|
||||
euv::AutoRef(..) |
|
||||
euv::AutoUnsafe(..) |
|
||||
euv::ClosureInvocation(..) |
|
||||
euv::ForLoop(..) |
|
||||
euv::RefBinding(..) |
|
||||
|
@ -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)
|
||||
@ -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"
|
||||
|
@ -20,7 +20,6 @@ use super::{
|
||||
PreferMutLvalue,
|
||||
structurally_resolved_type,
|
||||
};
|
||||
use middle::infer;
|
||||
use middle::traits;
|
||||
use middle::ty::{self, Ty};
|
||||
use syntax::ast;
|
||||
@ -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
|
||||
};
|
||||
|
@ -1119,8 +1119,8 @@ 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));
|
||||
@ -1128,11 +1128,15 @@ fn link_autoref(rcx: &Rcx,
|
||||
|
||||
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(..) | ty::AutoUnsizeUniq(_) | ty::AutoUnsize(_) => {}
|
||||
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::AutoUnsizeUniq(_) | ty::AutoUnsize(_) => {}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -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`
|
||||
}
|
20
src/test/compile-fail/borrowck-use-uninitialized-in-cast.rs
Normal file
20
src/test/compile-fail/borrowck-use-uninitialized-in-cast.rs
Normal file
@ -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.
|
||||
|
||||
// Check that we detect unused values that are cast to other things.
|
||||
// The problem was specified to casting to `*`, as creating unsafe
|
||||
// pointers was not being fully checked. Issue #20791.
|
||||
|
||||
// pretty-expanded FIXME #23616
|
||||
|
||||
fn main() {
|
||||
let x: &i32;
|
||||
let y = x as *const i32; //~ ERROR use of possibly uninitialized variable: `*x`
|
||||
}
|
Loading…
Reference in New Issue
Block a user