Auto merge of #61619 - Centril:rollup-b8hmiw7, r=Centril

Rollup of 4 pull requests

Successful merges:

 - #61332 (Remove asterisk suggestion for move errors in borrowck)
 - #61532 ([const-prop] Support Rvalue::{Ref,Len} and Deref)
 - #61586 (ci: Disable LLVM/debug assertions for asmjs builder)
 - #61599 (libcore/pin: Minor grammar corrections for module documentation)

Failed merges:

r? @ghost

src/ci/docker/asmjs/Dockerfile

@@ -37,3 +37,11 @@ ENV SCRIPT python2.7 ../x.py test --target $TARGETS \
   src/libstd \
   src/liballoc \
   src/libcore
+
+# Debug assertions in rustc are largely covered by other builders, and LLVM
+# assertions cause this builder to slow down by quite a large amount and don't
+# buy us a huge amount over other builders (not sure if we've ever seen an
+# asmjs-specific backend assertion trip), so disable assertions for these
+# tests.
+ENV NO_LLVM_ASSERTIONS=1
+ENV NO_DEBUG_ASSERTIONS=1

src/libcore/pin.rs

@@ -1,10 +1,10 @@
 //! Types that pin data to its location in memory.
 //!
-//! It is sometimes useful to have objects that are guaranteed to not move,
+//! It is sometimes useful to have objects that are guaranteed not to move,
 //! in the sense that their placement in memory does not change, and can thus be relied upon.
 //! A prime example of such a scenario would be building self-referential structs,
-//! since moving an object with pointers to itself will invalidate them,
-//! which could cause undefined behavior.
+//! as moving an object with pointers to itself will invalidate them, which could cause undefined
+//! behavior.
 //!
 //! A [`Pin<P>`] ensures that the pointee of any pointer type `P` has a stable location in memory,
 //! meaning it cannot be moved elsewhere and its memory cannot be deallocated
@@ -15,9 +15,10 @@
 //! moving the values they contain: you can move out of a `Box<T>`, or you can use [`mem::swap`].
 //! [`Pin<P>`] wraps a pointer type `P`, so `Pin<Box<T>>` functions much like a regular `Box<T>`:
 //! when a `Pin<Box<T>>` gets dropped, so do its contents, and the memory gets deallocated.
-//! Similarily, `Pin<&mut T>` is a lot like `&mut T`. However, [`Pin<P>`] does not let clients
+//! Similarly, `Pin<&mut T>` is a lot like `&mut T`. However, [`Pin<P>`] does not let clients
 //! actually obtain a `Box<T>` or `&mut T` to pinned data, which implies that you cannot use
 //! operations such as [`mem::swap`]:
+//!
 //! ```
 //! use std::pin::Pin;
 //! fn swap_pins<T>(x: Pin<&mut T>, y: Pin<&mut T>) {
@@ -39,19 +40,19 @@
 //! as a "`P`-style pointer" to a pinned `P::Target` -- so, a `Pin<Box<T>>` is
 //! an owned pointer to a pinned `T`, and a `Pin<Rc<T>>` is a reference-counted
 //! pointer to a pinned `T`.
-//! For correctness, [`Pin<P>`] relies on the [`Deref`] and [`DerefMut`] implementations
-//! to not move out of their `self` parameter, and to only ever return a pointer
-//! to pinned data when they are called on a pinned pointer.
+//! For correctness, [`Pin<P>`] relies on the implementations of [`Deref`] and
+//! [`DerefMut`] not to move out of their `self` parameter, and only ever to
+//! return a pointer to pinned data when they are called on a pinned pointer.
 //!
 //! # `Unpin`
 //!
-//! However, these restrictions are usually not necessary. Many types are always freely
-//! movable, even when pinned, because they do not rely on having a stable address.
-//! This includes all the basic types (like `bool`, `i32`, references)
-//! as well as types consisting solely of these types.
-//! Types that do not care about pinning implement the [`Unpin`] auto-trait, which
-//! cancels the effect of [`Pin<P>`]. For `T: Unpin`, `Pin<Box<T>>` and `Box<T>` function
-//! identically, as do `Pin<&mut T>` and `&mut T`.
+//! Many types are always freely movable, even when pinned, because they do not
+//! rely on having a stable address. This includes all the basic types (like
+//! `bool`, `i32`, and references) as well as types consisting solely of these
+//! types. Types that do not care about pinning implement the [`Unpin`]
+//! auto-trait, which cancels the effect of [`Pin<P>`]. For `T: Unpin`,
+//! `Pin<Box<T>>` and `Box<T>` function identically, as do `Pin<&mut T>` and
+//! `&mut T`.
 //!
 //! Note that pinning and `Unpin` only affect the pointed-to type `P::Target`, not the pointer
 //! type `P` itself that got wrapped in `Pin<P>`. For example, whether or not `Box<T>` is
@@ -65,11 +66,11 @@
 //! use std::marker::PhantomPinned;
 //! use std::ptr::NonNull;
 //!
-//! // This is a self-referential struct since the slice field points to the data field.
+//! // This is a self-referential struct because the slice field points to the data field.
 //! // We cannot inform the compiler about that with a normal reference,
-//! // since this pattern cannot be described with the usual borrowing rules.
-//! // Instead we use a raw pointer, though one which is known to not be null,
-//! // since we know it's pointing at the string.
+//! // as this pattern cannot be described with the usual borrowing rules.
+//! // Instead we use a raw pointer, though one which is known not to be null,
+//! // as we know it's pointing at the string.
 //! struct Unmovable {
 //!     data: String,
 //!     slice: NonNull<String>,
@@ -146,7 +147,7 @@
 //! section needs to function correctly.
 //!
 //! Notice that this guarantee does *not* mean that memory does not leak! It is still
-//! completely okay not to ever call `drop` on a pinned element (e.g., you can still
+//! completely okay not ever to call `drop` on a pinned element (e.g., you can still
 //! call [`mem::forget`] on a `Pin<Box<T>>`). In the example of the doubly-linked
 //! list, that element would just stay in the list. However you may not free or reuse the storage
 //! *without calling `drop`*.
@@ -192,7 +193,7 @@
 //!     `Unpin`. This is the default, but `Unpin` is a safe trait, so as the author of
 //!     the wrapper it is your responsibility *not* to add something like
 //!     `impl<T> Unpin for Wrapper<T>`. (Notice that adding a projection operation
-//!     requires unsafe code, so the fact that `Unpin` is a safe trait  does not break
+//!     requires unsafe code, so the fact that `Unpin` is a safe trait does not break
 //!     the principle that you only have to worry about any of this if you use `unsafe`.)
 //! 2.  The destructor of the wrapper must not move structural fields out of its argument. This
 //!     is the exact point that was raised in the [previous section][drop-impl]: `drop` takes

src/librustc_mir/borrow_check/move_errors.rs

@@ -503,32 +503,12 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                 move_from,
                 ..
             } => {
-                let try_remove_deref = match move_from {
-                    Place::Projection(box Projection {
-                        elem: ProjectionElem::Deref,
-                        ..
-                    }) => true,
-                    _ => false,
-                };
-                if try_remove_deref && snippet.starts_with('*') {
-                    // The snippet doesn't start with `*` in (e.g.) index
-                    // expressions `a[b]`, which roughly desugar to
-                    // `*Index::index(&a, b)` or
-                    // `*IndexMut::index_mut(&mut a, b)`.
-                    err.span_suggestion(
-                        span,
-                        "consider removing the `*`",
-                        snippet[1..].to_owned(),
-                        Applicability::Unspecified,
-                    );
-                } else {
-                    err.span_suggestion(
-                        span,
-                        "consider borrowing here",
-                        format!("&{}", snippet),
-                        Applicability::Unspecified,
-                    );
-                }
+                err.span_suggestion(
+                    span,
+                    "consider borrowing here",
+                    format!("&{}", snippet),
+                    Applicability::Unspecified,
+                );
 
                 if binds_to.is_empty() {
                     let place_ty = move_from.ty(self.mir, self.infcx.tcx).ty;

src/librustc_mir/interpret/place.rs

@@ -663,6 +663,23 @@ where
         Ok(())
     }
 
+    /// Write an `Immediate` to memory.
+    #[inline(always)]
+    pub fn write_immediate_to_mplace(
+        &mut self,
+        src: Immediate<M::PointerTag>,
+        dest: MPlaceTy<'tcx, M::PointerTag>,
+    ) -> EvalResult<'tcx> {
+        self.write_immediate_to_mplace_no_validate(src, dest)?;
+
+        if M::enforce_validity(self) {
+            // Data got changed, better make sure it matches the type!
+            self.validate_operand(dest.into(), vec![], None, /*const_mode*/ false)?;
+        }
+
+        Ok(())
+    }
+
     /// Write an immediate to a place.
     /// If you use this you are responsible for validating that things got copied at the
     /// right type.

src/librustc_mir/transform/const_prop.rs

@@ -17,8 +17,7 @@ use syntax_pos::{Span, DUMMY_SP};
 use rustc::ty::subst::InternalSubsts;
 use rustc_data_structures::indexed_vec::IndexVec;
 use rustc::ty::layout::{
-    LayoutOf, TyLayout, LayoutError,
-    HasTyCtxt, TargetDataLayout, HasDataLayout,
+    LayoutOf, TyLayout, LayoutError, HasTyCtxt, TargetDataLayout, HasDataLayout, Size,
 };
 
 use crate::interpret::{
@@ -333,6 +332,12 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
                             this.ecx.operand_field(eval, field.index() as u64)
                         })?;
                     },
+                    ProjectionElem::Deref => {
+                        trace!("processing deref");
+                        eval = self.use_ecx(source_info, |this| {
+                            this.ecx.deref_operand(eval)
+                        })?.into();
+                    }
                     // We could get more projections by using e.g., `operand_projection`,
                     // but we do not even have the stack frame set up properly so
                     // an `Index` projection would throw us off-track.
@@ -363,8 +368,12 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
             Rvalue::Use(ref op) => {
                 self.eval_operand(op, source_info)
             },
+            Rvalue::Ref(_, _, ref place) => {
+                let src = self.eval_place(place, source_info)?;
+                let mplace = src.try_as_mplace().ok()?;
+                Some(ImmTy::from_scalar(mplace.ptr.into(), place_layout).into())
+            },
             Rvalue::Repeat(..) |
-            Rvalue::Ref(..) |
             Rvalue::Aggregate(..) |
             Rvalue::NullaryOp(NullOp::Box, _) |
             Rvalue::Discriminant(..) => None,
@@ -376,10 +385,30 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
                     this.ecx.cast(op, kind, dest.into())?;
                     Ok(dest.into())
                 })
-            }
+            },
+            Rvalue::Len(ref place) => {
+                let place = self.eval_place(&place, source_info)?;
+                let mplace = place.try_as_mplace().ok()?;
+
+                if let ty::Slice(_) = mplace.layout.ty.sty {
+                    let len = mplace.meta.unwrap().to_usize(&self.ecx).unwrap();
 
-            // FIXME(oli-obk): evaluate static/constant slice lengths
-            Rvalue::Len(_) => None,
+                    Some(ImmTy {
+                        imm: Immediate::Scalar(
+                            Scalar::from_uint(
+                                len,
+                                Size::from_bits(
+                                    self.tcx.sess.target.usize_ty.bit_width().unwrap() as u64
+                                )
+                            ).into(),
+                        ),
+                        layout: self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?,
+                    }.into())
+                } else {
+                    trace!("not slice: {:?}", mplace.layout.ty.sty);
+                    None
+                }
+            },
             Rvalue::NullaryOp(NullOp::SizeOf, ty) => {
                 type_size_of(self.tcx, self.param_env, ty).and_then(|n| Some(
                     ImmTy {
@@ -525,12 +554,10 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
         source_info: SourceInfo,
     ) {
         trace!("attepting to replace {:?} with {:?}", rval, value);
-        self.ecx.validate_operand(
-            value,
-            vec![],
-            None,
-            true,
-        ).expect("value should already be a valid const");
+        if let Err(e) = self.ecx.validate_operand(value, vec![], None, true) {
+            trace!("validation error, attempt failed: {:?}", e);
+            return;
+        }
 
         // FIXME> figure out what tho do when try_read_immediate fails
         let imm = self.use_ecx(source_info, |this| {

src/test/mir-opt/const_prop/ref_deref.rs

@@ -0,0 +1,21 @@
+fn main() {
+    *(&4);
+}
+
+// END RUST SOURCE
+// START rustc.main.ConstProp.before.mir
+// bb0: {
+//     ...
+//     _2 = &(promoted[0]: i32);
+//     _1 = (*_2);
+//     ...
+//}
+// END rustc.main.ConstProp.before.mir
+// START rustc.main.ConstProp.after.mir
+// bb0: {
+//     ...
+//     _2 = const Scalar(AllocId(0).0x0) : &i32;
+//     _1 = const 4i32;
+//     ...
+// }
+// END rustc.main.ConstProp.after.mir

src/test/mir-opt/const_prop/reify_fn_ptr.rs

@@ -0,0 +1,25 @@
+fn main() {
+    let _ = main as usize as *const fn();
+}
+
+// END RUST SOURCE
+// START rustc.main.ConstProp.before.mir
+//  bb0: {
+//      ...
+//      _3 = const main as fn() (Pointer(ReifyFnPointer));
+//      _2 = move _3 as usize (Misc);
+//      ...
+//      _1 = move _2 as *const fn() (Misc);
+//      ...
+//  }
+// END rustc.main.ConstProp.before.mir
+// START rustc.main.ConstProp.after.mir
+//  bb0: {
+//      ...
+//      _3 = const Scalar(AllocId(1).0x0) : fn();
+//      _2 = move _3 as usize (Misc);
+//      ...
+//      _1 = const Scalar(AllocId(1).0x0) : *const fn();
+//      ...
+//  }
+// END rustc.main.ConstProp.after.mir

src/test/mir-opt/const_prop/slice_len.rs

@@ -1,36 +1,40 @@
-fn test() -> &'static [u32] {
-    &[1, 2]
-}
-
 fn main() {
-    let x = test()[0];
+    (&[1u32, 2, 3] as &[u32])[1];
 }
 
 // END RUST SOURCE
 // START rustc.main.ConstProp.before.mir
-//  bb1: {
+//  bb0: {
 //      ...
-//      _3 = const 0usize;
-//      _4 = Len((*_2));
-//      _5 = Lt(_3, _4);
-//      assert(move _5, "index out of bounds: the len is move _4 but the index is _3") -> bb2;
+//      _4 = &(promoted[0]: [u32; 3]);
+//      _3 = _4;
+//      _2 = move _3 as &[u32] (Pointer(Unsize));
+//      ...
+//      _6 = const 1usize;
+//      _7 = Len((*_2));
+//      _8 = Lt(_6, _7);
+//      assert(move _8, "index out of bounds: the len is move _7 but the index is _6") -> bb1;
 //  }
-//  bb2: {
-//      _1 = (*_2)[_3];
+//  bb1: {
+//      _1 = (*_2)[_6];
 //      ...
 //      return;
 //  }
 // END rustc.main.ConstProp.before.mir
 // START rustc.main.ConstProp.after.mir
 //  bb0: {
 //      ...
-//      _3 = const 0usize;
-//      _4 = Len((*_2));
-//      _5 = Lt(_3, _4);
-//      assert(move _5, "index out of bounds: the len is move _4 but the index is _3") -> bb2;
+//      _4 = const Scalar(AllocId(0).0x0) : &[u32; 3];
+//      _3 = const Scalar(AllocId(0).0x0) : &[u32; 3];
+//      _2 = move _3 as &[u32] (Pointer(Unsize));
+//      ...
+//      _6 = const 1usize;
+//      _7 = const 3usize;
+//      _8 = const true;
+//      assert(const true, "index out of bounds: the len is move _7 but the index is _6") -> bb1;
 //  }
-//  bb2: {
-//      _1 = (*_2)[_3];
+//  bb1: {
+//      _1 = (*_2)[_6];
 //      ...
 //      return;
 //  }

src/test/ui/access-mode-in-closures.stderr

@@ -6,7 +6,7 @@ LL |         match *s { S(v) => v }
    |               |      |
    |               |      data moved here
    |               |      move occurs because `v` has type `std::vec::Vec<isize>`, which does not implement the `Copy` trait
-   |               help: consider removing the `*`: `s`
+   |               help: consider borrowing here: `&*s`
 
 error: aborting due to previous error
 

src/test/ui/borrowck/borrowck-issue-2657-2.stderr

@@ -5,7 +5,7 @@ LL |         let _b = *y;
    |                  ^^
    |                  |
    |                  move occurs because `*y` has type `std::boxed::Box<i32>`, which does not implement the `Copy` trait
-   |                  help: consider removing the `*`: `y`
+   |                  help: consider borrowing here: `&*y`
 
 error: aborting due to previous error
 

src/test/ui/borrowck/borrowck-move-error-with-note.stderr

@@ -2,7 +2,7 @@ error[E0507]: cannot move out of `f.0` which is behind a shared reference
   --> $DIR/borrowck-move-error-with-note.rs:11:11
    |
 LL |     match *f {
-   |           ^^ help: consider removing the `*`: `f`
+   |           ^^ help: consider borrowing here: `&*f`
 LL |         Foo::Foo1(num1,
    |                   ---- data moved here
 LL |                   num2) => (),

src/test/ui/borrowck/borrowck-move-from-unsafe-ptr.stderr

@@ -5,7 +5,7 @@ LL |     let y = *x;
    |             ^^
    |             |
    |             move occurs because `*x` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait
-   |             help: consider removing the `*`: `x`
+   |             help: consider borrowing here: `&*x`
 
 error: aborting due to previous error