Skip to content

Commit 7458503

Browse files
committed
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).
1 parent ce97c19 commit 7458503

File tree

9 files changed

+232
-74
lines changed

9 files changed

+232
-74
lines changed

src/librustc/middle/check_const.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,19 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for CheckCrateVisitor<'a, 'tcx> {
662662
cmt: mc::cmt<'tcx>,
663663
_loan_region: ty::Region,
664664
bk: ty::BorrowKind,
665-
loan_cause: euv::LoanCause) {
665+
loan_cause: euv::LoanCause)
666+
{
667+
// Kind of hacky, but we allow Unsafe coercions in constants.
668+
// These occur when we convert a &T or *T to a *U, as well as
669+
// when making a thin pointer (e.g., `*T`) into a fat pointer
670+
// (e.g., `*Trait`).
671+
match loan_cause {
672+
euv::LoanCause::AutoUnsafe => {
673+
return;
674+
}
675+
_ => { }
676+
}
677+
666678
let mut cur = &cmt;
667679
let mut is_interior = false;
668680
loop {

src/librustc/middle/expr_use_visitor.rs

+151-28
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ pub enum LoanCause {
9999
ClosureCapture(Span),
100100
AddrOf,
101101
AutoRef,
102+
AutoUnsafe,
102103
RefBinding,
103104
OverloadedOperator,
104105
ClosureInvocation,
@@ -800,18 +801,8 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> {
800801
return_if_err!(self.mc.cat_expr_unadjusted(expr));
801802
self.delegate_consume(expr.id, expr.span, cmt_unadjusted);
802803
}
803-
ty::AdjustDerefRef(ty::AutoDerefRef {
804-
autoref: ref opt_autoref,
805-
autoderefs: n
806-
}) => {
807-
self.walk_autoderefs(expr, n);
808-
809-
match *opt_autoref {
810-
None => { }
811-
Some(ref r) => {
812-
self.walk_autoref(expr, r, n);
813-
}
814-
}
804+
ty::AdjustDerefRef(ref adj) => {
805+
self.walk_autoderefref(expr, adj);
815806
}
816807
}
817808
}
@@ -852,39 +843,171 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> {
852843
}
853844
}
854845

846+
fn walk_autoderefref(&mut self,
847+
expr: &ast::Expr,
848+
adj: &ty::AutoDerefRef<'tcx>) {
849+
debug!("walk_autoderefref expr={} adj={}",
850+
expr.repr(self.tcx()),
851+
adj.repr(self.tcx()));
852+
853+
self.walk_autoderefs(expr, adj.autoderefs);
854+
855+
// Weird hacky special case: AutoUnsizeUniq, which converts
856+
// from a ~T to a ~Trait etc, always comes in a stylized
857+
// fashion. In particular, we want to consume the ~ pointer
858+
// being dereferenced, not the dereferenced content (as the
859+
// content is, at least for upcasts, unsized).
860+
match adj.autoref {
861+
Some(ty::AutoUnsizeUniq(_)) => {
862+
assert!(adj.autoderefs == 1,
863+
format!("Expected exactly 1 deref with Uniq AutoRefs, found: {}",
864+
adj.autoderefs));
865+
let cmt_unadjusted =
866+
return_if_err!(self.mc.cat_expr_unadjusted(expr));
867+
self.delegate_consume(expr.id, expr.span, cmt_unadjusted);
868+
return;
869+
}
870+
_ => { }
871+
}
872+
873+
let autoref = adj.autoref.as_ref();
874+
let cmt_derefd = return_if_err!(
875+
self.mc.cat_expr_autoderefd(expr, adj.autoderefs));
876+
self.walk_autoref(expr, &cmt_derefd, autoref);
877+
}
878+
879+
/// Walks the autoref `opt_autoref` applied to the autoderef'd
880+
/// `expr`. `cmt_derefd` is the mem-categorized form of `expr`
881+
/// after all relevant autoderefs have occurred. Because AutoRefs
882+
/// can be recursive, this function is recursive: it first walks
883+
/// deeply all the way down the autoref chain, and then processes
884+
/// the autorefs on the way out. At each point, it returns the
885+
/// `cmt` for the rvalue that will be produced by introduced an
886+
/// autoref.
855887
fn walk_autoref(&mut self,
856888
expr: &ast::Expr,
857-
autoref: &ty::AutoRef,
858-
n: usize) {
859-
debug!("walk_autoref expr={}", expr.repr(self.tcx()));
889+
cmt_derefd: &mc::cmt<'tcx>,
890+
opt_autoref: Option<&ty::AutoRef<'tcx>>)
891+
-> mc::cmt<'tcx>
892+
{
893+
debug!("walk_autoref(expr.id={} cmt_derefd={} opt_autoref={:?})",
894+
expr.id,
895+
cmt_derefd.repr(self.tcx()),
896+
opt_autoref);
897+
898+
let autoref = match opt_autoref {
899+
Some(autoref) => autoref,
900+
None => {
901+
// No recursive step here, this is a base case.
902+
return cmt_derefd.clone();
903+
}
904+
};
860905

861906
match *autoref {
862-
ty::AutoPtr(r, m, _) => {
863-
let cmt_derefd = return_if_err!(
864-
self.mc.cat_expr_autoderefd(expr, n));
865-
debug!("walk_adjustment: cmt_derefd={}",
866-
cmt_derefd.repr(self.tcx()));
907+
ty::AutoPtr(r, m, ref baseref) => {
908+
let cmt_base = self.walk_autoref_recursively(expr, cmt_derefd, baseref);
909+
910+
debug!("walk_autoref: expr.id={} cmt_base={}",
911+
expr.id,
912+
cmt_base.repr(self.tcx()));
867913

868914
self.delegate.borrow(expr.id,
869915
expr.span,
870-
cmt_derefd,
916+
cmt_base,
871917
r,
872918
ty::BorrowKind::from_mutbl(m),
873919
AutoRef);
874920
}
875-
ty::AutoUnsize(_) |
921+
922+
ty::AutoUnsize(_) => {
923+
// Converting a `[T; N]` to `[T]` or `T` to `Trait`
924+
// isn't really a borrow, move, etc, in and of itself.
925+
// Also, no recursive step here, this is a base case.
926+
927+
// FIXME. It's a bit of a hack to just return `cmt_derefd` here,
928+
// because we are converting from a thin pointer to a fat pointer,
929+
// but we lack a correct "categorization" for this. Perhaps just adding
930+
// `cmt_unsize` is the right fix. I first tried `cmt_rvalue`, as that seemed
931+
// appropriate, but that led to errors in code like this:
932+
//
933+
// fn foo<'a>(&'a self) -> &'a Trait { self }
934+
//
935+
// The reason is simple: the (implicit) conversion from &T to &Self is
936+
// an autoref like:
937+
//
938+
// {autoderefs: 1, autoref: Some(Autoref(Autounsize(..)))}
939+
//
940+
// and if we categorized the autounsize as an rvalue,
941+
// then it would only be valid for the temporary
942+
// scope, which isn't enough to justify the return
943+
// value, which have the lifetime 'a.
944+
//
945+
// So I am now returning the unmodified cmt, which
946+
// while it doesn't feel complete, also doesn't seem
947+
// *wrong* -- after all, what is being borrowed *is*
948+
// `*self`, albeit with an extra vtable. Put another
949+
// way, having Autounsized nestled inside of Autoref
950+
// is somewhat misleading, in that it is not that the
951+
// autounsize products a new value in and of itself,
952+
// but rather that it "modifies" the enclosing
953+
// autoref. So perhaps it is the Adjustment that
954+
// should be changed? Unclear, so I am leaving this
955+
// as a lonely comment to be ignored for all
956+
// time. -nmatsakis
957+
return cmt_derefd.clone();
958+
}
959+
876960
ty::AutoUnsizeUniq(_) => {
877-
assert!(n == 1, format!("Expected exactly 1 deref with Uniq \
878-
AutoRefs, found: {}", n));
879-
let cmt_unadjusted =
880-
return_if_err!(self.mc.cat_expr_unadjusted(expr));
881-
self.delegate_consume(expr.id, expr.span, cmt_unadjusted);
961+
// these are handled via special case above
962+
self.tcx().sess.span_bug(expr.span, "nexpected AutoUnsizeUniq");
882963
}
883-
ty::AutoUnsafe(..) => {
964+
965+
ty::AutoUnsafe(m, ref baseref) => {
966+
let cmt_base = self.walk_autoref_recursively(expr, cmt_derefd, baseref);
967+
968+
debug!("walk_autoref: expr.id={} cmt_base={}",
969+
expr.id,
970+
cmt_base.repr(self.tcx()));
971+
972+
// Converting from a &T to *T (or &mut T to *mut T) is
973+
// treated as borrowing it for the enclosing temporary
974+
// scope.
975+
let r = ty::ReScope(region::CodeExtent::from_node_id(expr.id));
976+
977+
self.delegate.borrow(expr.id,
978+
expr.span,
979+
cmt_base,
980+
r,
981+
ty::BorrowKind::from_mutbl(m),
982+
AutoUnsafe);
884983
}
885984
}
985+
986+
// Construct the categorization for the result of the autoref.
987+
// This is always an rvalue, since we are producing a new
988+
// (temporary) indirection.
989+
990+
let adj_ty =
991+
ty::adjust_ty_for_autoref(self.tcx(),
992+
expr.span,
993+
cmt_derefd.ty,
994+
opt_autoref);
995+
996+
self.mc.cat_rvalue_node(expr.id, expr.span, adj_ty)
886997
}
887998

999+
fn walk_autoref_recursively(&mut self,
1000+
expr: &ast::Expr,
1001+
cmt_derefd: &mc::cmt<'tcx>,
1002+
autoref: &Option<Box<ty::AutoRef<'tcx>>>)
1003+
-> mc::cmt<'tcx>
1004+
{
1005+
// Shuffle from a ref to an optional box to an optional ref.
1006+
let autoref: Option<&ty::AutoRef<'tcx>> = autoref.as_ref().map(|b| &**b);
1007+
self.walk_autoref(expr, cmt_derefd, autoref)
1008+
}
1009+
1010+
8881011
// When this returns true, it means that the expression *is* a
8891012
// method-call (i.e. via the operator-overload). This true result
8901013
// also implies that walk_overloaded_operator already took care of

src/librustc/middle/mem_categorization.rs

+14-10
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,15 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
833833
ret
834834
}
835835

836+
/// Returns the lifetime of a temporary created by expr with id `id`.
837+
/// This could be `'static` if `id` is part of a constant expression.
838+
pub fn temporary_scope(&self, id: ast::NodeId) -> ty::Region {
839+
match self.typer.temporary_scope(id) {
840+
Some(scope) => ty::ReScope(scope),
841+
None => ty::ReStatic
842+
}
843+
}
844+
836845
pub fn cat_rvalue_node(&self,
837846
id: ast::NodeId,
838847
span: Span,
@@ -848,17 +857,12 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
848857
_ => check_const::NOT_CONST
849858
};
850859

860+
// Compute maximum lifetime of this rvalue. This is 'static if
861+
// we can promote to a constant, otherwise equal to enclosing temp
862+
// lifetime.
851863
let re = match qualif & check_const::NON_STATIC_BORROWS {
852-
check_const::PURE_CONST => {
853-
// Constant rvalues get promoted to 'static.
854-
ty::ReStatic
855-
}
856-
_ => {
857-
match self.typer.temporary_scope(id) {
858-
Some(scope) => ty::ReScope(scope),
859-
None => ty::ReStatic
860-
}
861-
}
864+
check_const::PURE_CONST => ty::ReStatic,
865+
_ => self.temporary_scope(id),
862866
};
863867
let ret = self.cat_rvalue(id, span, re, expr_ty);
864868
debug!("cat_rvalue_node ret {}", ret.repr(self.tcx()));

src/librustc_borrowck/borrowck/check_loans.rs

+1
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
542542
euv::OverloadedOperator(..) |
543543
euv::AddrOf(..) |
544544
euv::AutoRef(..) |
545+
euv::AutoUnsafe(..) |
545546
euv::ClosureInvocation(..) |
546547
euv::ForLoop(..) |
547548
euv::RefBinding(..) |

src/librustc_borrowck/borrowck/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
775775
euv::AddrOf |
776776
euv::RefBinding |
777777
euv::AutoRef |
778+
euv::AutoUnsafe |
778779
euv::ForLoop |
779780
euv::MatchDiscriminant => {
780781
format!("cannot borrow {} as mutable", descr)
@@ -822,6 +823,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
822823
BorrowViolation(euv::OverloadedOperator) |
823824
BorrowViolation(euv::AddrOf) |
824825
BorrowViolation(euv::AutoRef) |
826+
BorrowViolation(euv::AutoUnsafe) |
825827
BorrowViolation(euv::RefBinding) |
826828
BorrowViolation(euv::MatchDiscriminant) => {
827829
"cannot borrow data mutably"

src/librustc_typeck/check/op.rs

+2-30
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ use super::{
2020
PreferMutLvalue,
2121
structurally_resolved_type,
2222
};
23-
use middle::infer;
2423
use middle::traits;
2524
use middle::ty::{self, Ty};
2625
use syntax::ast;
@@ -314,36 +313,9 @@ fn lookup_op_method<'a, 'tcx>(fcx: &'a FnCtxt<'a, 'tcx>,
314313

315314
let method = match trait_did {
316315
Some(trait_did) => {
317-
// We do eager coercions to make using operators
318-
// more ergonomic:
319-
//
320-
// - If the input is of type &'a T (resp. &'a mut T),
321-
// then reborrow it to &'b T (resp. &'b mut T) where
322-
// 'b <= 'a. This makes things like `x == y`, where
323-
// `x` and `y` are both region pointers, work. We
324-
// could also solve this with variance or different
325-
// traits that don't force left and right to have same
326-
// type.
327-
let (adj_ty, adjustment) = match lhs_ty.sty {
328-
ty::ty_rptr(r_in, mt) => {
329-
let r_adj = fcx.infcx().next_region_var(infer::Autoref(lhs_expr.span));
330-
fcx.mk_subr(infer::Reborrow(lhs_expr.span), r_adj, *r_in);
331-
let adjusted_ty = ty::mk_rptr(fcx.tcx(), fcx.tcx().mk_region(r_adj), mt);
332-
let autoptr = ty::AutoPtr(r_adj, mt.mutbl, None);
333-
let adjustment = ty::AutoDerefRef { autoderefs: 1, autoref: Some(autoptr) };
334-
(adjusted_ty, adjustment)
335-
}
336-
_ => {
337-
(lhs_ty, ty::AutoDerefRef { autoderefs: 0, autoref: None })
338-
}
339-
};
340-
341-
debug!("adjusted_ty={} adjustment={:?}",
342-
adj_ty.repr(fcx.tcx()),
343-
adjustment);
344-
316+
let noop = ty::AutoDerefRef { autoderefs: 0, autoref: None };
345317
method::lookup_in_trait_adjusted(fcx, expr.span, Some(lhs_expr), opname,
346-
trait_did, adjustment, adj_ty, Some(other_tys))
318+
trait_did, noop, lhs_ty, Some(other_tys))
347319
}
348320
None => None
349321
};

src/librustc_typeck/check/regionck.rs

+9-5
Original file line numberDiff line numberDiff line change
@@ -1119,20 +1119,24 @@ fn link_pattern<'a, 'tcx>(rcx: &Rcx<'a, 'tcx>,
11191119
fn link_autoref(rcx: &Rcx,
11201120
expr: &ast::Expr,
11211121
autoderefs: usize,
1122-
autoref: &ty::AutoRef) {
1123-
1122+
autoref: &ty::AutoRef)
1123+
{
11241124
debug!("link_autoref(autoref={:?})", autoref);
11251125
let mc = mc::MemCategorizationContext::new(rcx.fcx);
11261126
let expr_cmt = ignore_err!(mc.cat_expr_autoderefd(expr, autoderefs));
11271127
debug!("expr_cmt={}", expr_cmt.repr(rcx.tcx()));
11281128

11291129
match *autoref {
11301130
ty::AutoPtr(r, m, _) => {
1131-
link_region(rcx, expr.span, r,
1132-
ty::BorrowKind::from_mutbl(m), expr_cmt);
1131+
link_region(rcx, expr.span, r, ty::BorrowKind::from_mutbl(m), expr_cmt);
1132+
}
1133+
1134+
ty::AutoUnsafe(m, _) => {
1135+
let r = ty::ReScope(CodeExtent::from_node_id(expr.id));
1136+
link_region(rcx, expr.span, r, ty::BorrowKind::from_mutbl(m), expr_cmt);
11331137
}
11341138

1135-
ty::AutoUnsafe(..) | ty::AutoUnsizeUniq(_) | ty::AutoUnsize(_) => {}
1139+
ty::AutoUnsizeUniq(_) | ty::AutoUnsize(_) => {}
11361140
}
11371141
}
11381142

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Variation on `borrowck-use-uninitialized-in-cast` in which we do a
12+
// trait cast from an uninitialized source. Issue #20791.
13+
14+
trait Foo { fn dummy(&self) { } }
15+
impl Foo for i32 { }
16+
17+
fn main() {
18+
let x: &i32;
19+
let y = x as *const Foo; //~ ERROR use of possibly uninitialized variable: `*x`
20+
}

0 commit comments

Comments
 (0)