]> git.lizzy.rs Git - rust.git/commitdiff
Modify the ExprUseVisitor to walk each part of an AutoRef, and in
authorNiko Matsakis <niko@alum.mit.edu>
Wed, 8 Apr 2015 08:31:51 +0000 (04:31 -0400)
committerNiko Matsakis <niko@alum.mit.edu>
Wed, 8 Apr 2015 13:49:41 +0000 (09:49 -0400)
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).

src/librustc/middle/check_const.rs
src/librustc/middle/expr_use_visitor.rs
src/librustc/middle/mem_categorization.rs
src/librustc_borrowck/borrowck/check_loans.rs
src/librustc_borrowck/borrowck/mod.rs
src/librustc_typeck/check/op.rs
src/librustc_typeck/check/regionck.rs
src/test/compile-fail/borrowck-use-uninitialized-in-cast-trait.rs [new file with mode: 0644]
src/test/compile-fail/borrowck-use-uninitialized-in-cast.rs [new file with mode: 0644]

index ce011f2561b79f661744aa78f613fff58e023a23..ee9d1e015539e337a18a19ca066d6ddb197b2610 100644 (file)
@@ -662,7 +662,19 @@ fn borrow(&mut self,
               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 {
index 2fa9c7c8fbebb0a7e8ceb794b5926ace6f818259..18e634a2dd6308ecb23e3e35cb4c8578bf075fd9 100644 (file)
@@ -99,6 +99,7 @@ pub enum LoanCause {
     ClosureCapture(Span),
     AddrOf,
     AutoRef,
+    AutoUnsafe,
     RefBinding,
     OverloadedOperator,
     ClosureInvocation,
@@ -800,18 +801,8 @@ fn walk_adjustment(&mut self, expr: &ast::Expr) {
                             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 @@ fn walk_autoderefs(&mut self,
         }
     }
 
+    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
index 85255d04df4329adc0033d8451fa3940f62379cb..6c7dc61109fb5109cd5f4b5cf3ae524b672404c6 100644 (file)
@@ -833,6 +833,15 @@ fn env_deref(&self,
         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 @@ pub fn cat_rvalue_node(&self,
             _ => 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()));
index ce7b492c51af158de887e36a92efb09feacb8a6e..9776538de3fedc748f03f584706ac35b43312b0b 100644 (file)
@@ -542,6 +542,7 @@ pub fn report_error_if_loan_conflicts_with_restriction(&self,
                 euv::OverloadedOperator(..) |
                 euv::AddrOf(..) |
                 euv::AutoRef(..) |
+                euv::AutoUnsafe(..) |
                 euv::ClosureInvocation(..) |
                 euv::ForLoop(..) |
                 euv::RefBinding(..) |
index f8da075e4bdc239929212106049be19c7bf0beca..c57cbcb929fbc8999d32287a047e78bad3b078b7 100644 (file)
@@ -775,6 +775,7 @@ pub fn bckerr_to_string(&self, err: &BckError<'tcx>) -> String {
                     euv::AddrOf |
                     euv::RefBinding |
                     euv::AutoRef |
+                    euv::AutoUnsafe |
                     euv::ForLoop |
                     euv::MatchDiscriminant => {
                         format!("cannot borrow {} as mutable", descr)
@@ -822,6 +823,7 @@ pub fn report_aliasability_violation(&self,
             BorrowViolation(euv::OverloadedOperator) |
             BorrowViolation(euv::AddrOf) |
             BorrowViolation(euv::AutoRef) |
+            BorrowViolation(euv::AutoUnsafe) |
             BorrowViolation(euv::RefBinding) |
             BorrowViolation(euv::MatchDiscriminant) => {
                 "cannot borrow data mutably"
index 49e88dc1483eb1ad52af26bdde92ff79ba2c6721..7c1fea4e60f6ecffa0281f906bbdb92da26f254a 100644 (file)
@@ -20,7 +20,6 @@
     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
     };
index 9171367468026a8f9226b4537b22500f4ff51394..9554e6ad8aad3881f1a8be980429b32ddf3e9604 100644 (file)
@@ -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(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(_) => {}
     }
 }
 
diff --git a/src/test/compile-fail/borrowck-use-uninitialized-in-cast-trait.rs b/src/test/compile-fail/borrowck-use-uninitialized-in-cast-trait.rs
new file mode 100644 (file)
index 0000000..796b455
--- /dev/null
@@ -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`
+}
diff --git a/src/test/compile-fail/borrowck-use-uninitialized-in-cast.rs b/src/test/compile-fail/borrowck-use-uninitialized-in-cast.rs
new file mode 100644 (file)
index 0000000..a3d5af8
--- /dev/null
@@ -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`
+}