Skip to content

Commit 1e79870

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 1e79870

File tree

9 files changed

+226
-74
lines changed

9 files changed

+226
-74
lines changed

src/librustc/middle/check_const.rs

Lines changed: 13 additions & 1 deletion
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

Lines changed: 145 additions & 28 deletions
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,165 @@ 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+
// It may seem a bit odd to return the cmt_derefd
928+
// unmodified here, but in fact I think it's the right
929+
// thing to do. Essentially the unsize transformation
930+
// isn't really relevant to the borrowing rules --
931+
// it's best thought of as a kind of side-modifier to
932+
// the autoref, adding additional data that is
933+
// attached to the pointer that is produced, but not
934+
// affecting the data being borrowed in any other
935+
// way. To see what I mean, consider this example:
936+
//
937+
// fn foo<'a>(&'a self) -> &'a Trait { self }
938+
//
939+
// This is valid because the underlying `self` value
940+
// lives for the lifetime 'a. If we were to treat the
941+
// "unsizing" as e.g. producing an rvalue, that would
942+
// only be valid for the temporary scope, which isn't
943+
// enough to justify the return value, which have the
944+
// lifetime 'a.
945+
//
946+
// Another option would be to add a variant for
947+
// categorization (like downcast) that wraps
948+
// cmt_derefd and represents the unsizing operation.
949+
// But I don't think there is any particular use for
950+
// this (yet). -nmatsakis
951+
return cmt_derefd.clone();
952+
}
953+
876954
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);
955+
// these are handled via special case above
956+
self.tcx().sess.span_bug(expr.span, "nexpected AutoUnsizeUniq");
882957
}
883-
ty::AutoUnsafe(..) => {
958+
959+
ty::AutoUnsafe(m, ref baseref) => {
960+
let cmt_base = self.walk_autoref_recursively(expr, cmt_derefd, baseref);
961+
962+
debug!("walk_autoref: expr.id={} cmt_base={}",
963+
expr.id,
964+
cmt_base.repr(self.tcx()));
965+
966+
// Converting from a &T to *T (or &mut T to *mut T) is
967+
// treated as borrowing it for the enclosing temporary
968+
// scope.
969+
let r = ty::ReScope(region::CodeExtent::from_node_id(expr.id));
970+
971+
self.delegate.borrow(expr.id,
972+
expr.span,
973+
cmt_base,
974+
r,
975+
ty::BorrowKind::from_mutbl(m),
976+
AutoUnsafe);
884977
}
885978
}
979+
980+
// Construct the categorization for the result of the autoref.
981+
// This is always an rvalue, since we are producing a new
982+
// (temporary) indirection.
983+
984+
let adj_ty =
985+
ty::adjust_ty_for_autoref(self.tcx(),
986+
expr.span,
987+
cmt_derefd.ty,
988+
opt_autoref);
989+
990+
self.mc.cat_rvalue_node(expr.id, expr.span, adj_ty)
886991
}
887992

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

src/librustc/middle/mem_categorization.rs

Lines changed: 14 additions & 10 deletions
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

Lines changed: 1 addition & 0 deletions
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

Lines changed: 2 additions & 0 deletions
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

Lines changed: 2 additions & 30 deletions
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

Lines changed: 9 additions & 5 deletions
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

Lines changed: 20 additions & 0 deletions
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)