Reviewer comments

Author: Nick Cameron

src/librustc/middle/expr_use_visitor.rs

@@ -848,26 +848,16 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> {
 
         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).
-        if let Some(ty) = adj.unsize {
-            if let ty::ty_uniq(_) = ty.sty {
-                assert!(adj.autoderefs == 0,
-                        format!("Expected no derefs with unsize AutoRefs, found: {}",
-                                 adj.repr(self.tcx())));
-                let cmt_unadjusted =
-                    return_if_err!(self.mc.cat_expr_unadjusted(expr));
-                self.delegate_consume(expr.id, expr.span, cmt_unadjusted);
-                return;
-            }
-        }
+        let cmt_derefd =
+            return_if_err!(self.mc.cat_expr_autoderefd(expr, adj.autoderefs));
 
-        let cmt_derefd = return_if_err!(
-            self.mc.cat_expr_autoderefd(expr, adj.autoderefs));
-        self.walk_autoref(expr, cmt_derefd, adj.autoref);
+        let cmt_refd =
+            self.walk_autoref(expr, cmt_derefd, adj.autoref);
+
+        if adj.unsize.is_some() {
+            // Unsizing consumes the thin pointer and produces a fat one.
+            self.delegate_consume(expr.id, expr.span, cmt_refd);
+        }
     }
 
 

src/librustc/middle/ty.rs

@@ -283,58 +283,84 @@ pub enum Variance {
 
 #[derive(Copy, Clone, Debug)]
 pub enum AutoAdjustment<'tcx> {
-    AdjustReifyFnPointer, // go from a fn-item type to a fn-pointer type
-    AdjustUnsafeFnPointer, // go from a safe fn pointer to an unsafe fn pointer
+    AdjustReifyFnPointer,   // go from a fn-item type to a fn-pointer type
+    AdjustUnsafeFnPointer,  // go from a safe fn pointer to an unsafe fn pointer
     AdjustDerefRef(AutoDerefRef<'tcx>),
 }
 
 /// Represents coercing a pointer to a different kind of pointer - where 'kind'
 /// here means either or both of raw vs borrowed vs unique and fat vs thin.
-/// The simplest cases are where the pointer is not adjusted fat vs thin. Here
-/// the pointer will be dereferenced N times (where a dereference can happen to
-/// to raw or borrowed pointers or any smart pointer which implements Deref,
-/// including Box<_>). The number of dereferences is given by `autoderefs`.
-/// It can then be auto-referenced zero or one times, indicated by `autoref`, to
-/// either a raw or borrowed pointer. In these cases unsize is None.
 ///
-/// A DST coercon involves unsizing the underlying data. We start with a thin
-/// pointer, deref a number of times, unsize the underlying data, then autoref.
-/// The 'unsize' phase may change a fixed length array to a dynamically sized one,
-/// a concrete object to a trait object, or statically sized struct to a dyncamically
-/// sized one.
-/// E.g., &[i32; 4] -> &[i32] is represented by:
+/// We transform pointers by following the following steps in order:
+/// 1. Deref the pointer `self.autoderefs` times (may be 0).
+/// 2. If `autoref` is `Some(_)`, then take the address and produce either a
+///    `&` or `*` pointer.
+/// 3. If `unsize` is `Some(_)`, then apply the unsize transformation,
+///    which will do things like convert thin pointers to fat
+///    pointers, or convert structs containing thin pointers to
+///    structs containing fat pointers, or convert between fat
+///    pointers.  We don't store the details of how the transform is
+///    done (in fact, we don't know that, because it might depend on
+///    the precise type parameters). We just store the target
+///    type. Trans figures out what has to be done at monomorphization
+///    time based on the precise source/target type at hand.
+///
+/// To make that more concrete, here are some common scenarios:
+///
+/// 1. The simplest cases are where the pointer is not adjusted fat vs thin.
+/// Here the pointer will be dereferenced N times (where a dereference can
+/// happen to to raw or borrowed pointers or any smart pointer which implements
+/// Deref, including Box<_>). The number of dereferences is given by
+/// `autoderefs`.  It can then be auto-referenced zero or one times, indicated
+/// by `autoref`, to either a raw or borrowed pointer. In these cases unsize is
+/// None.
+///
+/// 2. A thin-to-fat coercon involves unsizing the underlying data. We start
+/// with a thin pointer, deref a number of times, unsize the underlying data,
+/// then autoref. The 'unsize' phase may change a fixed length array to a
+/// dynamically sized one, a concrete object to a trait object, or statically
+/// sized struct to a dyncamically sized one. E.g., &[i32; 4] -> &[i32] is
+/// represented by:
+///
+/// ```
 /// AutoDerefRef {
 ///     autoderefs: 1,          // &[i32; 4] -> [i32; 4]
-///     unsize: Some([i32]),    // [i32; 4] -> [i32]
 ///     autoref: Some(AutoPtr), // [i32] -> &[i32]
+///     unsize: Some([i32]),    // [i32; 4] -> [i32]
 /// }
+/// ```
+///
 /// Note that for a struct, the 'deep' unsizing of the struct is not recorded.
 /// E.g., `struct Foo<T> { x: T }` we can coerce &Foo<[i32; 4]> to &Foo<[i32]>
 /// The autoderef and -ref are the same as in the above example, but the type
 /// stored in `unsize` is `Foo<[i32]>`, we don't store any further detail about
 /// the underlying conversions from `[i32; 4]` to `[i32]`.
 ///
-/// Box pointers are treated somewhat differently, the last deref is not counted,
-/// nor is the 'ref' to a `Box<_>`. Imagine them more like structs.
-/// E.g., Box<[i32; 4]> -> Box<[i32]> is represented by:
+/// 3. Coercing a `Box<T>` to `Box<Trait>` is an interesting special case.  In
+/// that case, we have the pointer we need coming in, so there are no
+/// autoderefs, and no autoref. Instead we just do the `Unsize` transformation.
+/// At some point, of course, `Box` should move out of the compiler, in which
+/// case this is analogous to transformating a struct. E.g., Box<[i32; 4]> ->
+/// Box<[i32]> is represented by:
+///
+/// ```
 /// AutoDerefRef {
 ///     autoderefs: 0,
-///     unsize: Some(Box<[i32]>),
 ///     autoref: None,
+///     unsize: Some(Box<[i32]>),
 /// }
+/// ```
 #[derive(Copy, Clone, Debug)]
 pub struct AutoDerefRef<'tcx> {
-    // FIXME with more powerful date structures we could have a better design
-    // here.
-
-    /// Apply a number of dereferences, producing an lvalue.
+    /// Step 1. Apply a number of dereferences, producing an lvalue.
     pub autoderefs: usize,
 
-    /// Produce a pointer/reference from the value.
+    /// Step 2. Optionally produce a pointer/reference from the value.
     pub autoref: Option<AutoRef<'tcx>>,
 
-    /// Unsize a pointer/reference value, e.g. &[T; n] to &[T].
-    /// The stored type is the target pointer type.
+    /// Step 3. Unsize a pointer/reference value, e.g. `&[T; n]` to
+    /// `&[T]`. The stored type is the target pointer type. Note that
+    /// the source could be a thin or fat pointer.
     pub unsize: Option<Ty<'tcx>>,
 }
 

src/librustc_typeck/check/method/confirm.rs

@@ -143,7 +143,10 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
                 ty::adjust_ty_for_autoref(self.tcx(), target, Some(autoref))
             }))
         } else {
-            // No unsizing should be performed without autoref.
+            // No unsizing should be performed without autoref (at
+            // least during method dispach). This is because we
+            // currently only unsize `[T;N]` to `[T]`, and naturally
+            // that must occur being a reference.
             assert!(pick.unsize.is_none());
             (None, None)
         };

src/test/compile-fail/issue-21950.rs

@@ -14,7 +14,7 @@ use std::ops::Add;
 
 fn main() {
     let x = &10 as
-            //~^ ERROR the value of the associated type `Output` (from the trait `core::ops::Add`) must be specified
             &Add;
             //~^ ERROR the type parameter `RHS` must be explicitly specified in an object type because its default value `Self` references the type `Self`
+            //~^^ ERROR the value of the associated type `Output` (from the trait `core::ops::Add`) must be specified
 }