diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index f3db359ec3348..987e3d8aed260 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2605,9 +2605,30 @@ pub enum Rvalue<'tcx> { static_assert_size!(Rvalue<'_>, 40); impl<'tcx> Rvalue<'tcx> { + /// Returns true if rvalue can be safely removed when the result is unused. #[inline] - pub fn is_pointer_int_cast(&self) -> bool { - matches!(self, Rvalue::Cast(CastKind::PointerExposeAddress, _, _)) + pub fn is_safe_to_remove(&self) -> bool { + match self { + // Pointer to int casts may be side-effects due to exposing the provenance. + // While the model is undecided, we should be conservative. See + // + Rvalue::Cast(CastKind::PointerExposeAddress, _, _) => false, + + Rvalue::Use(_) + | Rvalue::Repeat(_, _) + | Rvalue::Ref(_, _, _) + | Rvalue::ThreadLocalRef(_) + | Rvalue::AddressOf(_, _) + | Rvalue::Len(_) + | Rvalue::Cast(CastKind::Misc | CastKind::Pointer(_), _, _) + | Rvalue::BinaryOp(_, _) + | Rvalue::CheckedBinaryOp(_, _) + | Rvalue::NullaryOp(_, _) + | Rvalue::UnaryOp(_, _) + | Rvalue::Discriminant(_) + | Rvalue::Aggregate(_, _) + | Rvalue::ShallowInitBox(_, _) => true, + } } } diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index b99e7573000c9..c3b79917dda91 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -317,9 +317,11 @@ pub enum ExprKind<'tcx> { lhs: ExprId, rhs: ExprId, }, - /// Access to a struct or tuple field. + /// Access to a field of a struct, a tuple, an union, or an enum. Field { lhs: ExprId, + /// Variant containing the field. + variant_index: VariantIdx, /// This can be a named (`.foo`) or unnamed (`.0`) field. name: Field, }, diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index f57569522d58d..8c8ebb0a6b87a 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -80,7 +80,7 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp visitor.visit_expr(&visitor.thir()[lhs]); visitor.visit_expr(&visitor.thir()[rhs]); } - Field { lhs, name: _ } => visitor.visit_expr(&visitor.thir()[lhs]), + Field { lhs, variant_index: _, name: _ } => visitor.visit_expr(&visitor.thir()[lhs]), Index { lhs, index } => { visitor.visit_expr(&visitor.thir()[lhs]); visitor.visit_expr(&visitor.thir()[index]); diff --git a/compiler/rustc_mir_build/src/build/expr/as_place.rs b/compiler/rustc_mir_build/src/build/expr/as_place.rs index 5d55c4d637b6e..981441fab040f 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -5,6 +5,7 @@ use crate::build::ForGuard::{OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; use rustc_hir::def_id::DefId; use rustc_hir::HirId; +use rustc_middle::hir::place::Projection as HirProjection; use rustc_middle::hir::place::ProjectionKind as HirProjectionKind; use rustc_middle::middle::region; use rustc_middle::mir::AssertKind::BoundsCheck; @@ -268,20 +269,52 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>( ty::UpvarCapture::ByValue => upvar_resolved_place_builder, }; - let next_projection = capture.place.projections.len(); - let mut curr_projections = from_builder.projection; - // We used some of the projections to build the capture itself, // now we apply the remaining to the upvar resolved place. - upvar_resolved_place_builder - .projection - .extend(curr_projections.drain(next_projection..)); + let remaining_projections = strip_prefix( + capture.place.base_ty, + from_builder.projection, + &capture.place.projections, + ); + upvar_resolved_place_builder.projection.extend(remaining_projections); Ok(upvar_resolved_place_builder) } } } +/// Returns projections remaining after stripping an initial prefix of HIR +/// projections. +/// +/// Supports only HIR projection kinds that represent a path that might be +/// captured by a closure or a generator, i.e., an `Index` or a `Subslice` +/// projection kinds are unsupported. +fn strip_prefix<'tcx>( + mut base_ty: Ty<'tcx>, + projections: Vec>, + prefix_projections: &[HirProjection<'tcx>], +) -> impl Iterator> { + let mut iter = projections.into_iter(); + for projection in prefix_projections { + match projection.kind { + HirProjectionKind::Deref => { + assert!(matches!(iter.next(), Some(ProjectionElem::Deref))); + } + HirProjectionKind::Field(..) => { + if base_ty.is_enum() { + assert!(matches!(iter.next(), Some(ProjectionElem::Downcast(..)))); + } + assert!(matches!(iter.next(), Some(ProjectionElem::Field(..)))); + } + HirProjectionKind::Index | HirProjectionKind::Subslice => { + bug!("unexpected projection kind: {:?}", projection); + } + } + base_ty = projection.ty; + } + iter +} + impl<'tcx> PlaceBuilder<'tcx> { pub(crate) fn into_place<'a>( self, @@ -438,11 +471,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.expr_as_place(block, &this.thir[value], mutability, fake_borrow_temps) }) } - ExprKind::Field { lhs, name } => { - let place_builder = unpack!( - block = - this.expr_as_place(block, &this.thir[lhs], mutability, fake_borrow_temps,) - ); + ExprKind::Field { lhs, variant_index, name } => { + let lhs = &this.thir[lhs]; + let mut place_builder = + unpack!(block = this.expr_as_place(block, lhs, mutability, fake_borrow_temps,)); + if let ty::Adt(adt_def, _) = lhs.ty.kind() { + if adt_def.is_enum() { + place_builder = place_builder.downcast(*adt_def, variant_index); + } + } block.and(place_builder.field(name, expr.ty)) } ExprKind::Deref { arg } => { diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 480c5b195cc2d..bd9f599fff0a1 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -591,6 +591,7 @@ impl<'tcx> Cx<'tcx> { } hir::ExprKind::Field(ref source, ..) => ExprKind::Field { lhs: self.mirror_expr(source), + variant_index: VariantIdx::new(0), name: Field::new(tcx.field_index(expr.hir_id, self.typeck_results)), }, hir::ExprKind::Cast(ref source, ref cast_ty) => { @@ -994,14 +995,11 @@ impl<'tcx> Cx<'tcx> { HirProjectionKind::Deref => { ExprKind::Deref { arg: self.thir.exprs.push(captured_place_expr) } } - HirProjectionKind::Field(field, ..) => { - // Variant index will always be 0, because for multi-variant - // enums, we capture the enum entirely. - ExprKind::Field { - lhs: self.thir.exprs.push(captured_place_expr), - name: Field::new(field as usize), - } - } + HirProjectionKind::Field(field, variant_index) => ExprKind::Field { + lhs: self.thir.exprs.push(captured_place_expr), + variant_index, + name: Field::new(field as usize), + }, HirProjectionKind::Index | HirProjectionKind::Subslice => { // We don't capture these projections, so we can ignore them here continue; diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs index 7076fbe1bdb53..9b62ee5473c7b 100644 --- a/compiler/rustc_mir_dataflow/src/impls/liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/liveness.rs @@ -244,13 +244,10 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> { // Compute the place that we are storing to, if any let destination = match &statement.kind { StatementKind::Assign(assign) => { - if assign.1.is_pointer_int_cast() { - // Pointer to int casts may be side-effects due to exposing the provenance. - // While the model is undecided, we should be conservative. See - // - None - } else { + if assign.1.is_safe_to_remove() { Some(assign.0) + } else { + None } } StatementKind::SetDiscriminant { place, .. } | StatementKind::Deinit(place) => { diff --git a/compiler/rustc_mir_transform/src/dead_store_elimination.rs b/compiler/rustc_mir_transform/src/dead_store_elimination.rs index 8becac34ed7ee..779f3c778156b 100644 --- a/compiler/rustc_mir_transform/src/dead_store_elimination.rs +++ b/compiler/rustc_mir_transform/src/dead_store_elimination.rs @@ -34,7 +34,7 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitS for (statement_index, statement) in bb_data.statements.iter().enumerate().rev() { let loc = Location { block: bb, statement_index }; if let StatementKind::Assign(assign) = &statement.kind { - if assign.1.is_pointer_int_cast() { + if !assign.1.is_safe_to_remove() { continue; } } diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs index 72e0834392576..8a78ea5c82bb1 100644 --- a/compiler/rustc_mir_transform/src/simplify.rs +++ b/compiler/rustc_mir_transform/src/simplify.rs @@ -494,8 +494,12 @@ impl<'tcx> Visitor<'tcx> for UsedLocals { StatementKind::StorageLive(_local) | StatementKind::StorageDead(_local) => {} StatementKind::Assign(box (ref place, ref rvalue)) => { - self.visit_lhs(place, location); - self.visit_rvalue(rvalue, location); + if rvalue.is_safe_to_remove() { + self.visit_lhs(place, location); + self.visit_rvalue(rvalue, location); + } else { + self.super_statement(statement, location); + } } StatementKind::SetDiscriminant { ref place, variant_index: _ } diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 92d153990c22b..10bf95abd3959 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -196,20 +196,32 @@ impl CStr { /// allows inspection and interoperation of non-owned C strings. The total /// size of the raw C string must be smaller than `isize::MAX` **bytes** /// in memory due to calling the `slice::from_raw_parts` function. - /// This method is unsafe for a number of reasons: /// - /// * There is no guarantee to the validity of `ptr`. - /// * The returned lifetime is not guaranteed to be the actual lifetime of - /// `ptr`. - /// * There is no guarantee that the memory pointed to by `ptr` contains a - /// valid nul terminator byte at the end of the string. - /// * It is not guaranteed that the memory pointed by `ptr` won't change - /// before the `CStr` has been destroyed. + /// # Safety + /// + /// * The memory pointed to by `ptr` must contain a valid nul terminator at the + /// end of the string. + /// + /// * `ptr` must be [valid] for reads of bytes up to and including the null terminator. + /// This means in particular: + /// + /// * The entire memory range of this `CStr` must be contained within a single allocated object! + /// * `ptr` must be non-null even for a zero-length cstr. + /// + /// * The memory referenced by the returned `CStr` must not be mutated for + /// the duration of lifetime `'a`. /// /// > **Note**: This operation is intended to be a 0-cost cast but it is /// > currently implemented with an up-front calculation of the length of /// > the string. This is not guaranteed to always be the case. /// + /// # Caveat + /// + /// The lifetime for the returned slice is inferred from its usage. To prevent accidental misuse, + /// it's suggested to tie the lifetime to whichever source lifetime is safe in the context, + /// such as by providing a helper function taking the lifetime of a host value for the slice, + /// or by explicit annotation. + /// /// # Examples /// /// ```ignore (extern-declaration) @@ -227,6 +239,8 @@ impl CStr { /// } /// # } /// ``` + /// + /// [valid]: core::ptr#safety #[inline] #[must_use] #[stable(feature = "rust1", since = "1.0.0")] @@ -349,8 +363,11 @@ impl CStr { /// Unsafely creates a C string wrapper from a byte slice. /// /// This function will cast the provided `bytes` to a `CStr` wrapper without - /// performing any sanity checks. The provided slice **must** be nul-terminated - /// and not contain any interior nul bytes. + /// performing any sanity checks. + /// + /// # Safety + /// The provided slice **must** be nul-terminated and not contain any interior + /// nul bytes. /// /// # Examples /// diff --git a/library/std/src/sync/mutex.rs b/library/std/src/sync/mutex.rs index 3d8281fe59389..b2fbb77204aa0 100644 --- a/library/std/src/sync/mutex.rs +++ b/library/std/src/sync/mutex.rs @@ -10,11 +10,10 @@ use crate::sys_common::mutex as sys; /// A mutual exclusion primitive useful for protecting shared data /// /// This mutex will block threads waiting for the lock to become available. The -/// mutex can also be statically initialized or created via a [`new`] -/// constructor. Each mutex has a type parameter which represents the data that -/// it is protecting. The data can only be accessed through the RAII guards -/// returned from [`lock`] and [`try_lock`], which guarantees that the data is only -/// ever accessed when the mutex is locked. +/// mutex can be created via a [`new`] constructor. Each mutex has a type parameter +/// which represents the data that it is protecting. The data can only be accessed +/// through the RAII guards returned from [`lock`] and [`try_lock`], which +/// guarantees that the data is only ever accessed when the mutex is locked. /// /// # Poisoning /// diff --git a/src/test/mir-opt/simplify-locals.rs b/src/test/mir-opt/simplify-locals.rs index 5862cf2eb2905..f6bf396cd0515 100644 --- a/src/test/mir-opt/simplify-locals.rs +++ b/src/test/mir-opt/simplify-locals.rs @@ -62,6 +62,12 @@ fn t4() -> u32 { unsafe { X + 1 } } +// EMIT_MIR simplify_locals.expose_addr.SimplifyLocals.diff +fn expose_addr(p: *const usize) { + // Used pointer to address cast. Has a side effect of exposing the provenance. + p as usize; +} + fn main() { c(); d1(); @@ -71,4 +77,5 @@ fn main() { t2(); t3(); t4(); + expose_addr(&0); } diff --git a/src/test/mir-opt/simplify_locals.expose_addr.SimplifyLocals.diff b/src/test/mir-opt/simplify_locals.expose_addr.SimplifyLocals.diff new file mode 100644 index 0000000000000..93d77ad40aa4b --- /dev/null +++ b/src/test/mir-opt/simplify_locals.expose_addr.SimplifyLocals.diff @@ -0,0 +1,21 @@ +- // MIR for `expose_addr` before SimplifyLocals ++ // MIR for `expose_addr` after SimplifyLocals + + fn expose_addr(_1: *const usize) -> () { + debug p => _1; // in scope 0 at $DIR/simplify-locals.rs:66:16: 66:17 + let mut _0: (); // return place in scope 0 at $DIR/simplify-locals.rs:66:33: 66:33 + let _2: usize; // in scope 0 at $DIR/simplify-locals.rs:68:5: 68:15 + let mut _3: *const usize; // in scope 0 at $DIR/simplify-locals.rs:68:5: 68:6 + + bb0: { + StorageLive(_2); // scope 0 at $DIR/simplify-locals.rs:68:5: 68:15 + StorageLive(_3); // scope 0 at $DIR/simplify-locals.rs:68:5: 68:6 + _3 = _1; // scope 0 at $DIR/simplify-locals.rs:68:5: 68:6 + _2 = move _3 as usize (PointerExposeAddress); // scope 0 at $DIR/simplify-locals.rs:68:5: 68:15 + StorageDead(_3); // scope 0 at $DIR/simplify-locals.rs:68:14: 68:15 + StorageDead(_2); // scope 0 at $DIR/simplify-locals.rs:68:15: 68:16 + _0 = const (); // scope 0 at $DIR/simplify-locals.rs:66:33: 69:2 + return; // scope 0 at $DIR/simplify-locals.rs:69:2: 69:2 + } + } + diff --git a/src/test/ui/closures/2229_closure_analysis/capture-enum-field.rs b/src/test/ui/closures/2229_closure_analysis/capture-enum-field.rs new file mode 100644 index 0000000000000..bbe3aa31a98df --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/capture-enum-field.rs @@ -0,0 +1,27 @@ +// edition:2021 +// run-pass + +#[derive(Debug, PartialEq, Eq)] +pub enum Color { + RGB(u8, u8, u8), +} + +fn main() { + let mut color = Color::RGB(0, 0, 0); + let mut red = |v| { + let Color::RGB(ref mut r, _, _) = color; + *r = v; + }; + let mut green = |v| { + let Color::RGB(_, ref mut g, _) = color; + *g = v; + }; + let mut blue = |v| { + let Color::RGB(_, _, ref mut b) = color; + *b = v; + }; + red(1); + green(2); + blue(3); + assert_eq!(Color::RGB(1, 2, 3), color); +} diff --git a/src/tools/rust-analyzer b/src/tools/rust-analyzer index f94fa62d69faf..ad6810e90bf89 160000 --- a/src/tools/rust-analyzer +++ b/src/tools/rust-analyzer @@ -1 +1 @@ -Subproject commit f94fa62d69faf5bd63b3772d3ec4f0c76cf2db57 +Subproject commit ad6810e90bf89a4ef0ae21349d077050bc2a4fa2