Skip to content

Commit

Permalink
Auto merge of #61201 - Centril:rollup-975knrk, r=Centril
Browse files Browse the repository at this point in the history
Rollup of 9 pull requests

Successful merges:

 - #61087 (Tweak `self` arg not as first argument of a method diagnostic)
 - #61114 (Vec: avoid creating slices to the elements)
 - #61144 (Suggest borrowing for loop head on move error)
 - #61149 (Fix spelling in release notes)
 - #61161 (MaybeUninit doctest: remove unnecessary type ascription)
 - #61173 (Auto-derive Encode and Decode implementations of DefPathTable)
 - #61184 (Add additional trace statements to the const propagator)
 - #61189 (Turn turbo 🐟 🍨 into an error)
 - #61193 (Add comment to explain why we change the layout for Projection)

Failed merges:

r? @ghost
  • Loading branch information
bors committed May 26, 2019
2 parents 572892c + 8d247e7 commit 2268d99
Show file tree
Hide file tree
Showing 24 changed files with 870 additions and 654 deletions.
2 changes: 1 addition & 1 deletion RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ Libraries
produce a warning if their returning type is unused.
- [The methods `checked_pow`, `saturating_pow`, `wrapping_pow`, and
`overflowing_pow` are now available for all numeric types.][57873] These are
equivalvent to methods such as `wrapping_add` for the `pow` operation.
equivalent to methods such as `wrapping_add` for the `pow` operation.


Stabilized APIs
Expand Down
21 changes: 21 additions & 0 deletions src/liballoc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1152,3 +1152,24 @@ fn test_try_reserve_exact() {
}

}

#[test]
fn test_stable_push_pop() {
// Test that, if we reserved enough space, adding and removing elements does not
// invalidate references into the vector (such as `v0`). This test also
// runs in Miri, which would detect such problems.
let mut v = Vec::with_capacity(10);
v.push(13);

// laundering the lifetime -- we take care that `v` does not reallocate, so that's okay.
let v0 = unsafe { &*(&v[0] as *const _) };

// Now do a bunch of things and occasionally use `v0` again to assert it is still valid.
v.push(1);
v.push(2);
v.insert(1, 1);
assert_eq!(*v0, 13);
v.remove(1);
v.pop().unwrap();
assert_eq!(*v0, 13);
}
78 changes: 71 additions & 7 deletions src/liballoc/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,75 @@ impl<T> Vec<T> {
self
}

/// Returns a raw pointer to the vector's buffer.
///
/// The caller must ensure that the vector outlives the pointer this
/// function returns, or else it will end up pointing to garbage.
/// Modifying the vector may cause its buffer to be reallocated,
/// which would also make any pointers to it invalid.
///
/// The caller must also ensure that the memory the pointer (non-transitively) points to
/// is never written to (except inside an `UnsafeCell`) using this pointer or any pointer
/// derived from it. If you need to mutate the contents of the slice, use [`as_mut_ptr`].
///
/// # Examples
///
/// ```
/// let x = vec![1, 2, 4];
/// let x_ptr = x.as_ptr();
///
/// unsafe {
/// for i in 0..x.len() {
/// assert_eq!(*x_ptr.add(i), 1 << i);
/// }
/// }
/// ```
///
/// [`as_mut_ptr`]: #method.as_mut_ptr
#[stable(feature = "vec_as_ptr", since = "1.37.0")]
#[inline]
pub fn as_ptr(&self) -> *const T {
// We shadow the slice method of the same name to avoid going through
// `deref`, which creates an intermediate reference.
let ptr = self.buf.ptr();
unsafe { assume(!ptr.is_null()); }
ptr
}

/// Returns an unsafe mutable pointer to the vector's buffer.
///
/// The caller must ensure that the vector outlives the pointer this
/// function returns, or else it will end up pointing to garbage.
/// Modifying the vector may cause its buffer to be reallocated,
/// which would also make any pointers to it invalid.
///
/// # Examples
///
/// ```
/// // Allocate vector big enough for 4 elements.
/// let size = 4;
/// let mut x: Vec<i32> = Vec::with_capacity(size);
/// let x_ptr = x.as_mut_ptr();
///
/// // Initialize elements via raw pointer writes, then set length.
/// unsafe {
/// for i in 0..size {
/// *x_ptr.add(i) = i as i32;
/// }
/// x.set_len(size);
/// }
/// assert_eq!(&*x, &[0,1,2,3]);
/// ```
#[stable(feature = "vec_as_ptr", since = "1.37.0")]
#[inline]
pub fn as_mut_ptr(&mut self) -> *mut T {
// We shadow the slice method of the same name to avoid going through
// `deref_mut`, which creates an intermediate reference.
let ptr = self.buf.ptr();
unsafe { assume(!ptr.is_null()); }
ptr
}

/// Forces the length of the vector to `new_len`.
///
/// This is a low-level operation that maintains none of the normal
Expand Down Expand Up @@ -1706,9 +1775,7 @@ impl<T> ops::Deref for Vec<T> {

fn deref(&self) -> &[T] {
unsafe {
let p = self.buf.ptr();
assume(!p.is_null());
slice::from_raw_parts(p, self.len)
slice::from_raw_parts(self.as_ptr(), self.len)
}
}
}
Expand All @@ -1717,9 +1784,7 @@ impl<T> ops::Deref for Vec<T> {
impl<T> ops::DerefMut for Vec<T> {
fn deref_mut(&mut self) -> &mut [T] {
unsafe {
let ptr = self.buf.ptr();
assume(!ptr.is_null());
slice::from_raw_parts_mut(ptr, self.len)
slice::from_raw_parts_mut(self.as_mut_ptr(), self.len)
}
}
}
Expand Down Expand Up @@ -1754,7 +1819,6 @@ impl<T> IntoIterator for Vec<T> {
fn into_iter(mut self) -> IntoIter<T> {
unsafe {
let begin = self.as_mut_ptr();
assume(!begin.is_null());
let end = if mem::size_of::<T>() == 0 {
arith_offset(begin as *const i8, self.len() as isize) as *const T
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/libcore/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,7 @@ impl<T: ?Sized> DerefMut for ManuallyDrop<T> {
/// out.write(vec![1, 2, 3]);
/// }
///
/// let mut v: MaybeUninit<Vec<i32>> = MaybeUninit::uninit();
/// let mut v = MaybeUninit::uninit();
/// unsafe { make_vec(v.as_mut_ptr()); }
/// // Now we know `v` is initialized! This also makes sure the vector gets
/// // properly dropped.
Expand Down Expand Up @@ -1071,7 +1071,7 @@ impl<T: ?Sized> DerefMut for ManuallyDrop<T> {
/// optimizations, potentially resulting in a larger size:
///
/// ```rust
/// # use std::mem::{MaybeUninit, size_of, align_of};
/// # use std::mem::{MaybeUninit, size_of};
/// assert_eq!(size_of::<Option<bool>>(), 1);
/// assert_eq!(size_of::<Option<MaybeUninit<bool>>>(), 2);
/// ```
Expand Down
26 changes: 1 addition & 25 deletions src/librustc/hir/map/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::ich::Fingerprint;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::indexed_vec::{IndexVec};
use rustc_data_structures::stable_hasher::StableHasher;
use serialize::{Encodable, Decodable, Encoder, Decoder};
use crate::session::CrateDisambiguator;
use std::borrow::Borrow;
use std::fmt::Write;
Expand All @@ -25,14 +24,13 @@ use crate::util::nodemap::NodeMap;
/// Internally the DefPathTable holds a tree of DefKeys, where each DefKey
/// stores the DefIndex of its parent.
/// There is one DefPathTable for each crate.
#[derive(Clone, Default)]
#[derive(Clone, Default, RustcDecodable, RustcEncodable)]
pub struct DefPathTable {
index_to_key: Vec<DefKey>,
def_path_hashes: Vec<DefPathHash>,
}

impl DefPathTable {

fn allocate(&mut self,
key: DefKey,
def_path_hash: DefPathHash)
Expand Down Expand Up @@ -86,28 +84,6 @@ impl DefPathTable {
}
}


impl Encodable for DefPathTable {
fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
// Index to key
self.index_to_key.encode(s)?;

// DefPath hashes
self.def_path_hashes.encode(s)?;

Ok(())
}
}

impl Decodable for DefPathTable {
fn decode<D: Decoder>(d: &mut D) -> Result<DefPathTable, D::Error> {
Ok(DefPathTable {
index_to_key: Decodable::decode(d)?,
def_path_hashes : Decodable::decode(d)?,
})
}
}

/// The definition table containing node definitions.
/// It holds the `DefPathTable` for local `DefId`s/`DefPath`s and it also stores a
/// mapping from `NodeId`s to local `DefId`s.
Expand Down
24 changes: 11 additions & 13 deletions src/librustc_mir/borrow_check/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,18 +158,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
span,
format!("value moved{} here, in previous iteration of loop", move_msg),
);
if Some(CompilerDesugaringKind::ForLoop) == span.compiler_desugaring_kind() {
if let Ok(snippet) = self.infcx.tcx.sess.source_map()
.span_to_snippet(span)
{
err.span_suggestion(
move_span,
"consider borrowing this to avoid moving it into the for loop",
format!("&{}", snippet),
Applicability::MaybeIncorrect,
);
}
}
is_loop_move = true;
} else if move_site.traversed_back_edge {
err.span_label(
Expand All @@ -185,7 +173,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
&mut err,
format!("variable moved due to use{}", move_spans.describe()),
);
};
}
if Some(CompilerDesugaringKind::ForLoop) == move_span.compiler_desugaring_kind() {
if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) {
err.span_suggestion(
move_span,
"consider borrowing to avoid moving into the for loop",
format!("&{}", snippet),
Applicability::MaybeIncorrect,
);
}
}
}

use_spans.var_span_label(
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M>
PlaceBase::Local(mir::RETURN_PLACE) => return err!(ReadFromReturnPointer),
PlaceBase::Local(local) => {
// FIXME use place_projection.is_empty() when is available
// Do not use the layout passed in as argument if the base we are looking at
// here is not the entire place.
let layout = if let Place::Base(_) = mir_place {
layout
} else {
Expand Down
12 changes: 11 additions & 1 deletion src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
}

fn eval_place(&mut self, place: &Place<'tcx>, source_info: SourceInfo) -> Option<Const<'tcx>> {
trace!("eval_place(place={:?})", place);
match *place {
Place::Base(PlaceBase::Local(loc)) => self.places[loc].clone(),
Place::Projection(ref proj) => match proj.elem {
Expand Down Expand Up @@ -516,6 +517,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
}

fn replace_with_const(&self, rval: &mut Rvalue<'tcx>, value: Const<'tcx>, span: Span) {
trace!("attepting to replace {:?} with {:?}", rval, value);
self.ecx.validate_operand(
value,
vec![],
Expand Down Expand Up @@ -579,6 +581,10 @@ impl CanConstProp {
// FIXME(oli-obk): lint variables until they are used in a condition
// FIXME(oli-obk): lint if return value is constant
*val = mir.local_kind(local) == LocalKind::Temp;

if !*val {
trace!("local {:?} can't be propagated because it's not a temporary", local);
}
}
cpv.visit_mir(mir);
cpv.can_const_prop
Expand All @@ -598,6 +604,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp {
// FIXME(oli-obk): we could be more powerful here, if the multiple writes
// only occur in independent execution paths
MutatingUse(MutatingUseContext::Store) => if self.found_assignment[local] {
trace!("local {:?} can't be propagated because of multiple assignments", local);
self.can_const_prop[local] = false;
} else {
self.found_assignment[local] = true
Expand All @@ -609,7 +616,10 @@ impl<'tcx> Visitor<'tcx> for CanConstProp {
NonMutatingUse(NonMutatingUseContext::Projection) |
MutatingUse(MutatingUseContext::Projection) |
NonUse(_) => {},
_ => self.can_const_prop[local] = false,
_ => {
trace!("local {:?} can't be propagaged because it's used: {:?}", local, context);
self.can_const_prop[local] = false;
},
}
}
}
Expand Down
Loading

0 comments on commit 2268d99

Please sign in to comment.