diff options
Diffstat (limited to 'compiler/rustc_mir')
21 files changed, 351 insertions, 303 deletions
diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/region_name.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/region_name.rs index 2a90fb042dd..6211cf8a9da 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/region_name.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/region_name.rs @@ -445,7 +445,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { "highlight_if_we_cannot_match_hir_ty: type_name={:?} needle_fr={:?}", type_name, needle_fr ); - if type_name.find(&format!("'{}", counter)).is_some() { + if type_name.contains(&format!("'{}", counter)) { // Only add a label if we can confirm that a region was labelled. RegionNameHighlight::CannotMatchHirTy(span, type_name) } else { diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index de54c5582e0..80eabdd9af8 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -9,6 +9,7 @@ use rustc_hir::{HirId, Node}; use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; +use rustc_middle::hir::place::PlaceBase as HirPlaceBase; use rustc_middle::mir::{ traversal, Body, ClearCrossCrate, Local, Location, Mutability, Operand, Place, PlaceElem, PlaceRef, @@ -75,6 +76,7 @@ crate use region_infer::RegionInferenceContext; crate struct Upvar { name: Symbol, + // FIXME(project-rfc-2229#8): This should use Place or something similar var_hir_id: HirId, /// If true, the capture is behind a reference. @@ -155,13 +157,13 @@ fn do_mir_borrowck<'a, 'tcx>( infcx.set_tainted_by_errors(); } let upvars: Vec<_> = tables - .closure_captures - .get(&def.did.to_def_id()) - .into_iter() - .flat_map(|v| v.values()) - .map(|upvar_id| { - let var_hir_id = upvar_id.var_path.hir_id; - let capture = tables.upvar_capture(*upvar_id); + .closure_min_captures_flattened(def.did.to_def_id()) + .map(|captured_place| { + let var_hir_id = match captured_place.place.base { + HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, + _ => bug!("Expected upvar"), + }; + let capture = captured_place.info.capture_kind; let by_ref = match capture { ty::UpvarCapture::ByValue(_) => false, ty::UpvarCapture::ByRef(..) => true, diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index a5c45452dec..543b7e7ebaa 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -749,7 +749,11 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> { (&adt_def.variants[VariantIdx::new(0)], substs) } ty::Closure(_, substs) => { - return match substs.as_closure().upvar_tys().nth(field.index()) { + return match substs + .as_closure() + .tupled_upvars_ty() + .tuple_element_ty(field.index()) + { Some(ty) => Ok(ty), None => Err(FieldAccessError::OutOfRange { field_count: substs.as_closure().upvar_tys().count(), diff --git a/compiler/rustc_mir/src/const_eval/error.rs b/compiler/rustc_mir/src/const_eval/error.rs index 39358e03e75..0e610e37552 100644 --- a/compiler/rustc_mir/src/const_eval/error.rs +++ b/compiler/rustc_mir/src/const_eval/error.rs @@ -20,6 +20,7 @@ pub enum ConstEvalErrKind { ModifiedGlobal, AssertFailure(AssertKind<ConstInt>), Panic { msg: Symbol, line: u32, col: u32, file: Symbol }, + Abort(String), } // The errors become `MachineStop` with plain strings when being raised. @@ -46,6 +47,7 @@ impl fmt::Display for ConstEvalErrKind { Panic { msg, line, col, file } => { write!(f, "the evaluated program panicked at '{}', {}:{}:{}", msg, file, line, col) } + Abort(ref msg) => write!(f, "{}", msg), } } } diff --git a/compiler/rustc_mir/src/const_eval/machine.rs b/compiler/rustc_mir/src/const_eval/machine.rs index c72089ec55a..740c965e591 100644 --- a/compiler/rustc_mir/src/const_eval/machine.rs +++ b/compiler/rustc_mir/src/const_eval/machine.rs @@ -1,17 +1,20 @@ use rustc_middle::mir; use rustc_middle::ty::layout::HasTyCtxt; +use rustc_middle::ty::InstanceDef; use rustc_middle::ty::{self, Ty}; use std::borrow::Borrow; use std::collections::hash_map::Entry; use std::hash::Hash; use rustc_data_structures::fx::FxHashMap; +use std::fmt; use rustc_ast::Mutability; use rustc_hir::def_id::DefId; use rustc_middle::mir::AssertMessage; use rustc_session::Limit; use rustc_span::symbol::{sym, Symbol}; +use rustc_target::abi::{Align, Size}; use crate::interpret::{ self, compile_time_machine, AllocId, Allocation, Frame, GlobalId, ImmTy, InterpCx, @@ -37,6 +40,14 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> { if instance.def.requires_caller_location(self.tcx()) { return Ok(false); } + // Only memoize instrinsics. This was added in #79594 while adding the `const_allocate` intrinsic. + // We only memoize intrinsics because it would be unsound to memoize functions + // which might interact with the heap. + // Additionally, const_allocate intrinsic is impure and thus should not be memoized; + // it will not be memoized because it has non-ZST args + if !matches!(instance.def, InstanceDef::Intrinsic(_)) { + return Ok(false); + } // For the moment we only do this for functions which take no arguments // (or all arguments are ZSTs) so that we don't memoize too much. if args.iter().any(|a| !a.layout.is_zst()) { @@ -169,6 +180,28 @@ impl<K: Hash + Eq, V> interpret::AllocMap<K, V> for FxHashMap<K, V> { crate type CompileTimeEvalContext<'mir, 'tcx> = InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>; +#[derive(Debug, PartialEq, Eq, Copy, Clone)] +pub enum MemoryKind { + Heap, +} + +impl fmt::Display for MemoryKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MemoryKind::Heap => write!(f, "heap allocation"), + } + } +} + +impl interpret::MayLeak for MemoryKind { + #[inline(always)] + fn may_leak(self) -> bool { + match self { + MemoryKind::Heap => false, + } + } +} + impl interpret::MayLeak for ! { #[inline(always)] fn may_leak(self) -> bool { @@ -212,6 +245,8 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> { impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, 'tcx> { compile_time_machine!(<'mir, 'tcx>); + type MemoryKind = MemoryKind; + type MemoryExtra = MemoryExtra; fn find_mir_or_eval_fn( @@ -295,6 +330,22 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, }; ecx.write_scalar(Scalar::from_bool(cmp), dest)?; } + sym::const_allocate => { + let size = ecx.read_scalar(args[0])?.to_machine_usize(ecx)?; + let align = ecx.read_scalar(args[1])?.to_machine_usize(ecx)?; + + let align = match Align::from_bytes(align) { + Ok(a) => a, + Err(err) => throw_ub_format!("align has to be a power of 2, {}", err), + }; + + let ptr = ecx.memory.allocate( + Size::from_bytes(size as u64), + align, + interpret::MemoryKind::Machine(MemoryKind::Heap), + ); + ecx.write_scalar(Scalar::Ptr(ptr), dest)?; + } _ => { return Err(ConstEvalErrKind::NeedsRfc(format!( "calling intrinsic `{}`", @@ -333,6 +384,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, Err(ConstEvalErrKind::AssertFailure(err).into()) } + fn abort(_ecx: &mut InterpCx<'mir, 'tcx, Self>, msg: String) -> InterpResult<'tcx, !> { + Err(ConstEvalErrKind::Abort(msg).into()) + } + fn ptr_to_int(_mem: &Memory<'mir, 'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx, u64> { Err(ConstEvalErrKind::NeedsRfc("pointer-to-integer cast".to_string()).into()) } diff --git a/compiler/rustc_mir/src/interpret/eval_context.rs b/compiler/rustc_mir/src/interpret/eval_context.rs index 05b4d1c410d..3d955576f0f 100644 --- a/compiler/rustc_mir/src/interpret/eval_context.rs +++ b/compiler/rustc_mir/src/interpret/eval_context.rs @@ -4,7 +4,7 @@ use std::mem; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_hir::{self as hir, def::DefKind, def_id::DefId, definitions::DefPathData}; +use rustc_hir::{self as hir, def_id::DefId, definitions::DefPathData}; use rustc_index::vec::IndexVec; use rustc_macros::HashStable; use rustc_middle::ich::StableHashingContext; @@ -700,21 +700,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let mut locals = IndexVec::from_elem(dummy, &body.local_decls); // Now mark those locals as dead that we do not want to initialize - match self.tcx.def_kind(instance.def_id()) { - // statics and constants don't have `Storage*` statements, no need to look for them - // - // FIXME: The above is likely untrue. See - // <https://github.com/rust-lang/rust/pull/70004#issuecomment-602022110>. Is it - // okay to ignore `StorageDead`/`StorageLive` annotations during CTFE? - DefKind::Static | DefKind::Const | DefKind::AssocConst => {} - _ => { - // Mark locals that use `Storage*` annotations as dead on function entry. - let always_live = AlwaysLiveLocals::new(self.body()); - for local in locals.indices() { - if !always_live.contains(local) { - locals[local].value = LocalValue::Dead; - } - } + // Mark locals that use `Storage*` annotations as dead on function entry. + let always_live = AlwaysLiveLocals::new(self.body()); + for local in locals.indices() { + if !always_live.contains(local) { + locals[local].value = LocalValue::Dead; } } // done @@ -850,36 +840,31 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } - /// Mark a storage as live, killing the previous content and returning it. - /// Remember to deallocate that! - pub fn storage_live( - &mut self, - local: mir::Local, - ) -> InterpResult<'tcx, LocalValue<M::PointerTag>> { + /// Mark a storage as live, killing the previous content. + pub fn storage_live(&mut self, local: mir::Local) -> InterpResult<'tcx> { assert!(local != mir::RETURN_PLACE, "Cannot make return place live"); trace!("{:?} is now live", local); let local_val = LocalValue::Uninitialized; - // StorageLive *always* kills the value that's currently stored. - // However, we do not error if the variable already is live; - // see <https://github.com/rust-lang/rust/issues/42371>. - Ok(mem::replace(&mut self.frame_mut().locals[local].value, local_val)) + // StorageLive expects the local to be dead, and marks it live. + let old = mem::replace(&mut self.frame_mut().locals[local].value, local_val); + if !matches!(old, LocalValue::Dead) { + throw_ub_format!("StorageLive on a local that was already live"); + } + Ok(()) } - /// Returns the old value of the local. - /// Remember to deallocate that! - pub fn storage_dead(&mut self, local: mir::Local) -> LocalValue<M::PointerTag> { + pub fn storage_dead(&mut self, local: mir::Local) -> InterpResult<'tcx> { assert!(local != mir::RETURN_PLACE, "Cannot make return place dead"); trace!("{:?} is now dead", local); - mem::replace(&mut self.frame_mut().locals[local].value, LocalValue::Dead) + // It is entirely okay for this local to be already dead (at least that's how we currently generate MIR) + let old = mem::replace(&mut self.frame_mut().locals[local].value, LocalValue::Dead); + self.deallocate_local(old)?; + Ok(()) } - pub(super) fn deallocate_local( - &mut self, - local: LocalValue<M::PointerTag>, - ) -> InterpResult<'tcx> { - // FIXME: should we tell the user that there was a local which was never written to? + fn deallocate_local(&mut self, local: LocalValue<M::PointerTag>) -> InterpResult<'tcx> { if let LocalValue::Live(Operand::Indirect(MemPlace { ptr, .. })) = local { // All locals have a backing allocation, even if the allocation is empty // due to the local having ZST type. diff --git a/compiler/rustc_mir/src/interpret/intern.rs b/compiler/rustc_mir/src/interpret/intern.rs index 413be427339..01d58c47e3a 100644 --- a/compiler/rustc_mir/src/interpret/intern.rs +++ b/compiler/rustc_mir/src/interpret/intern.rs @@ -25,19 +25,20 @@ use rustc_target::abi::Size; use rustc_ast::Mutability; use super::{AllocId, Allocation, InterpCx, MPlaceTy, Machine, MemoryKind, Scalar, ValueVisitor}; +use crate::const_eval; -pub trait CompileTimeMachine<'mir, 'tcx> = Machine< +pub trait CompileTimeMachine<'mir, 'tcx, T> = Machine< 'mir, 'tcx, - MemoryKind = !, + MemoryKind = T, PointerTag = (), ExtraFnVal = !, FrameExtra = (), AllocExtra = (), - MemoryMap = FxHashMap<AllocId, (MemoryKind<!>, Allocation)>, + MemoryMap = FxHashMap<AllocId, (MemoryKind<T>, Allocation)>, >; -struct InternVisitor<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> { +struct InternVisitor<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx, const_eval::MemoryKind>> { /// The ectx from which we intern. ecx: &'rt mut InterpCx<'mir, 'tcx, M>, /// Previously encountered safe references. @@ -74,7 +75,7 @@ struct IsStaticOrFn; /// `immutable` things might become mutable if `ty` is not frozen. /// `ty` can be `None` if there is no potential interior mutability /// to account for (e.g. for vtables). -fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>( +fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx, const_eval::MemoryKind>>( ecx: &'rt mut InterpCx<'mir, 'tcx, M>, leftover_allocations: &'rt mut FxHashSet<AllocId>, alloc_id: AllocId, @@ -104,7 +105,10 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>( // This match is just a canary for future changes to `MemoryKind`, which most likely need // changes in this function. match kind { - MemoryKind::Stack | MemoryKind::Vtable | MemoryKind::CallerLocation => {} + MemoryKind::Stack + | MemoryKind::Machine(const_eval::MemoryKind::Heap) + | MemoryKind::Vtable + | MemoryKind::CallerLocation => {} } // Set allocation mutability as appropriate. This is used by LLVM to put things into // read-only memory, and also by Miri when evaluating other globals that @@ -138,7 +142,9 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>( None } -impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> InternVisitor<'rt, 'mir, 'tcx, M> { +impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx, const_eval::MemoryKind>> + InternVisitor<'rt, 'mir, 'tcx, M> +{ fn intern_shallow( &mut self, alloc_id: AllocId, @@ -149,8 +155,8 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> InternVisitor<'rt, 'mir } } -impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> - for InternVisitor<'rt, 'mir, 'tcx, M> +impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::MemoryKind>> + ValueVisitor<'mir, 'tcx, M> for InternVisitor<'rt, 'mir, 'tcx, M> { type V = MPlaceTy<'tcx>; @@ -287,7 +293,7 @@ pub enum InternKind { /// Any errors here would anyway be turned into `const_err` lints, whereas validation failures /// are hard errors. #[tracing::instrument(skip(ecx))] -pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>( +pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx, const_eval::MemoryKind>>( ecx: &mut InterpCx<'mir, 'tcx, M>, intern_kind: InternKind, ret: MPlaceTy<'tcx>, @@ -418,7 +424,9 @@ where Ok(()) } -impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { +impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx, !>> + InterpCx<'mir, 'tcx, M> +{ /// A helper function that allocates memory for the layout given and gives you access to mutate /// it. Once your own mutation code is done, the backing `Allocation` is removed from the /// current `Memory` and returned. diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index f666a89ca56..dfd77a8fca9 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -126,7 +126,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { None => match intrinsic_name { sym::transmute => throw_ub_format!("transmuting to uninhabited type"), sym::unreachable => throw_ub!(Unreachable), - sym::abort => M::abort(self)?, + sym::abort => M::abort(self, "the program aborted execution".to_owned())?, // Unsupported diverging intrinsic. _ => return Ok(false), }, @@ -407,6 +407,22 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { sym::transmute => { self.copy_op_transmute(args[0], dest)?; } + sym::assert_inhabited => { + let ty = instance.substs.type_at(0); + let layout = self.layout_of(ty)?; + + if layout.abi.is_uninhabited() { + // The run-time intrinsic panics just to get a good backtrace; here we abort + // since there is no problem showing a backtrace even for aborts. + M::abort( + self, + format!( + "aborted execution: attempted to instantiate uninhabited type `{}`", + ty + ), + )?; + } + } sym::simd_insert => { let index = u64::from(self.read_scalar(args[1])?.to_u32()?); let elem = args[2]; diff --git a/compiler/rustc_mir/src/interpret/machine.rs b/compiler/rustc_mir/src/interpret/machine.rs index 66dbacb2f9d..74625569432 100644 --- a/compiler/rustc_mir/src/interpret/machine.rs +++ b/compiler/rustc_mir/src/interpret/machine.rs @@ -176,7 +176,7 @@ pub trait Machine<'mir, 'tcx>: Sized { ) -> InterpResult<'tcx>; /// Called to evaluate `Abort` MIR terminator. - fn abort(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx, !> { + fn abort(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _msg: String) -> InterpResult<'tcx, !> { throw_unsup_format!("aborting execution is not supported") } @@ -366,9 +366,9 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) { type PointerTag = (); type ExtraFnVal = !; - type MemoryKind = !; - type MemoryMap = rustc_data_structures::fx::FxHashMap<AllocId, (MemoryKind<!>, Allocation)>; - const GLOBAL_KIND: Option<!> = None; // no copying of globals from `tcx` to machine memory + type MemoryMap = + rustc_data_structures::fx::FxHashMap<AllocId, (MemoryKind<Self::MemoryKind>, Allocation)>; + const GLOBAL_KIND: Option<Self::MemoryKind> = None; // no copying of globals from `tcx` to machine memory type AllocExtra = (); type FrameExtra = (); @@ -407,7 +407,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) { _memory_extra: &Self::MemoryExtra, _id: AllocId, alloc: Cow<'b, Allocation>, - _kind: Option<MemoryKind<!>>, + _kind: Option<MemoryKind<Self::MemoryKind>>, ) -> (Cow<'b, Allocation<Self::PointerTag>>, Self::PointerTag) { // We do not use a tag so we can just cheaply forward the allocation (alloc, ()) diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 156da84f291..95738db1f55 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -95,14 +95,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Mark locals as alive StorageLive(local) => { - let old_val = self.storage_live(*local)?; - self.deallocate_local(old_val)?; + self.storage_live(*local)?; } // Mark locals as dead StorageDead(local) => { - let old_val = self.storage_dead(*local); - self.deallocate_local(old_val)?; + self.storage_dead(*local)?; } // No dynamic semantics attached to `FakeRead`; MIR diff --git a/compiler/rustc_mir/src/interpret/terminator.rs b/compiler/rustc_mir/src/interpret/terminator.rs index bb11c2a23bd..a2931325a28 100644 --- a/compiler/rustc_mir/src/interpret/terminator.rs +++ b/compiler/rustc_mir/src/interpret/terminator.rs @@ -110,7 +110,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } Abort => { - M::abort(self)?; + M::abort(self, "the program aborted execution".to_owned())?; } // When we encounter Resume, we've finished unwinding diff --git a/compiler/rustc_mir/src/interpret/util.rs b/compiler/rustc_mir/src/interpret/util.rs index ec90f063a55..c2165db278f 100644 --- a/compiler/rustc_mir/src/interpret/util.rs +++ b/compiler/rustc_mir/src/interpret/util.rs @@ -13,12 +13,13 @@ where return Ok(()); } + struct FoundParam; struct UsedParamsNeedSubstVisitor<'tcx> { tcx: TyCtxt<'tcx>, } impl<'tcx> TypeVisitor<'tcx> for UsedParamsNeedSubstVisitor<'tcx> { - type BreakTy = (); + type BreakTy = FoundParam; fn visit_const(&mut self, c: &'tcx ty::Const<'tcx>) -> ControlFlow<Self::BreakTy> { if !c.needs_subst() { @@ -26,7 +27,7 @@ where } match c.val { - ty::ConstKind::Param(..) => ControlFlow::BREAK, + ty::ConstKind::Param(..) => ControlFlow::Break(FoundParam), _ => c.super_visit_with(self), } } @@ -37,7 +38,7 @@ where } match *ty.kind() { - ty::Param(_) => ControlFlow::BREAK, + ty::Param(_) => ControlFlow::Break(FoundParam), ty::Closure(def_id, substs) | ty::Generator(def_id, substs, ..) | ty::FnDef(def_id, substs) => { @@ -76,7 +77,7 @@ where } let mut vis = UsedParamsNeedSubstVisitor { tcx }; - if ty.visit_with(&mut vis).is_break() { + if matches!(ty.visit_with(&mut vis), ControlFlow::Break(FoundParam)) { throw_inval!(TooGeneric); } else { Ok(()) diff --git a/compiler/rustc_mir/src/lib.rs b/compiler/rustc_mir/src/lib.rs index 2ed115b1297..e6d822086f5 100644 --- a/compiler/rustc_mir/src/lib.rs +++ b/compiler/rustc_mir/src/lib.rs @@ -28,6 +28,7 @@ Rust MIR: a lowered representation of Rust. #![feature(or_patterns)] #![feature(once_cell)] #![feature(control_flow_enum)] +#![feature(str_split_once)] #![recursion_limit = "256"] #[macro_use] diff --git a/compiler/rustc_mir/src/transform/const_prop.rs b/compiler/rustc_mir/src/transform/const_prop.rs index abcf1862fd8..1d949e020ed 100644 --- a/compiler/rustc_mir/src/transform/const_prop.rs +++ b/compiler/rustc_mir/src/transform/const_prop.rs @@ -180,6 +180,8 @@ impl<'mir, 'tcx> ConstPropMachine<'mir, 'tcx> { impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> { compile_time_machine!(<'mir, 'tcx>); + type MemoryKind = !; + type MemoryExtra = (); fn find_mir_or_eval_fn( diff --git a/compiler/rustc_mir/src/transform/coverage/debug.rs b/compiler/rustc_mir/src/transform/coverage/debug.rs index e9528557b33..af81d9af0e2 100644 --- a/compiler/rustc_mir/src/transform/coverage/debug.rs +++ b/compiler/rustc_mir/src/transform/coverage/debug.rs @@ -148,40 +148,46 @@ impl DebugOptions { if let Ok(env_debug_options) = std::env::var(RUSTC_COVERAGE_DEBUG_OPTIONS) { for setting_str in env_debug_options.replace(" ", "").replace("-", "_").split(',') { - let mut setting = setting_str.splitn(2, '='); - match setting.next() { - Some(option) if option == "allow_unused_expressions" => { - allow_unused_expressions = bool_option_val(option, setting.next()); + let (option, value) = match setting_str.split_once('=') { + None => (setting_str, None), + Some((k, v)) => (k, Some(v)), + }; + match option { + "allow_unused_expressions" => { + allow_unused_expressions = bool_option_val(option, value); debug!( "{} env option `allow_unused_expressions` is set to {}", RUSTC_COVERAGE_DEBUG_OPTIONS, allow_unused_expressions ); } - Some(option) if option == "counter_format" => { - if let Some(strval) = setting.next() { - counter_format = counter_format_option_val(strval); - debug!( - "{} env option `counter_format` is set to {:?}", - RUSTC_COVERAGE_DEBUG_OPTIONS, counter_format - ); - } else { - bug!( - "`{}` option in environment variable {} requires one or more \ - plus-separated choices (a non-empty subset of \ - `id+block+operation`)", - option, - RUSTC_COVERAGE_DEBUG_OPTIONS - ); - } + "counter_format" => { + match value { + None => { + bug!( + "`{}` option in environment variable {} requires one or more \ + plus-separated choices (a non-empty subset of \ + `id+block+operation`)", + option, + RUSTC_COVERAGE_DEBUG_OPTIONS + ); + } + Some(val) => { + counter_format = counter_format_option_val(val); + debug!( + "{} env option `counter_format` is set to {:?}", + RUSTC_COVERAGE_DEBUG_OPTIONS, counter_format + ); + } + }; } - Some("") => {} - Some(invalid) => bug!( - "Unsupported setting `{}` in environment variable {}", - invalid, - RUSTC_COVERAGE_DEBUG_OPTIONS - ), - None => {} - } + _ => { + bug!( + "Unsupported setting `{}` in environment variable {}", + option, + RUSTC_COVERAGE_DEBUG_OPTIONS + ) + } + }; } } diff --git a/compiler/rustc_mir/src/transform/coverage/graph.rs b/compiler/rustc_mir/src/transform/coverage/graph.rs index 9d375633dcf..2408a999c05 100644 --- a/compiler/rustc_mir/src/transform/coverage/graph.rs +++ b/compiler/rustc_mir/src/transform/coverage/graph.rs @@ -33,7 +33,7 @@ impl CoverageGraph { // Pre-transform MIR `BasicBlock` successors and predecessors into the BasicCoverageBlock // equivalents. Note that since the BasicCoverageBlock graph has been fully simplified, the // each predecessor of a BCB leader_bb should be in a unique BCB, and each successor of a - // BCB last_bb should bin in its own unique BCB. Therefore, collecting the BCBs using + // BCB last_bb should be in its own unique BCB. Therefore, collecting the BCBs using // `bb_to_bcb` should work without requiring a deduplication step. let successors = IndexVec::from_fn_n( @@ -118,18 +118,8 @@ impl CoverageGraph { match term.kind { TerminatorKind::Return { .. } - // FIXME(richkadel): Add test(s) for `Abort` coverage. | TerminatorKind::Abort - // FIXME(richkadel): Add test(s) for `Assert` coverage. - // Should `Assert` be handled like `FalseUnwind` instead? Since we filter out unwind - // branches when creating the BCB CFG, aren't `Assert`s (without unwinds) just like - // `FalseUnwinds` (which are kind of like `Goto`s)? - | TerminatorKind::Assert { .. } - // FIXME(richkadel): Add test(s) for `Yield` coverage, and confirm coverage is - // sensible for code using the `yield` keyword. | TerminatorKind::Yield { .. } - // FIXME(richkadel): Also add coverage tests using async/await, and threading. - | TerminatorKind::SwitchInt { .. } => { // The `bb` has more than one _outgoing_ edge, or exits the function. Save the // current sequence of `basic_blocks` gathered to this point, as a new @@ -147,6 +137,16 @@ impl CoverageGraph { // `Terminator`s `successors()` list) checking the number of successors won't // work. } + + // The following `TerminatorKind`s are either not expected outside an unwind branch, + // or they should not (under normal circumstances) branch. Coverage graphs are + // simplified by assuring coverage results are accurate for program executions that + // don't panic. + // + // Programs that panic and unwind may record slightly inaccurate coverage results + // for a coverage region containing the `Terminator` that began the panic. This + // is as intended. (See Issue #78544 for a possible future option to support + // coverage in test programs that panic.) TerminatorKind::Goto { .. } | TerminatorKind::Resume | TerminatorKind::Unreachable @@ -154,6 +154,7 @@ impl CoverageGraph { | TerminatorKind::DropAndReplace { .. } | TerminatorKind::Call { .. } | TerminatorKind::GeneratorDrop + | TerminatorKind::Assert { .. } | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } | TerminatorKind::InlineAsm { .. } => {} @@ -278,10 +279,13 @@ rustc_index::newtype_index! { /// A node in the [control-flow graph][CFG] of CoverageGraph. pub(super) struct BasicCoverageBlock { DEBUG_FORMAT = "bcb{}", + const START_BCB = 0, } } -/// A BasicCoverageBlockData (BCB) represents the maximal-length sequence of MIR BasicBlocks without +/// `BasicCoverageBlockData` holds the data indexed by a `BasicCoverageBlock`. +/// +/// A `BasicCoverageBlock` (BCB) represents the maximal-length sequence of MIR `BasicBlock`s without /// conditional branches, and form a new, simplified, coverage-specific Control Flow Graph, without /// altering the original MIR CFG. /// diff --git a/compiler/rustc_mir/src/transform/coverage/mod.rs b/compiler/rustc_mir/src/transform/coverage/mod.rs index 192bb6680e4..f69748db238 100644 --- a/compiler/rustc_mir/src/transform/coverage/mod.rs +++ b/compiler/rustc_mir/src/transform/coverage/mod.rs @@ -88,6 +88,8 @@ struct Instrumentor<'a, 'tcx> { pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>, + source_file: Lrc<SourceFile>, + fn_sig_span: Span, body_span: Span, basic_coverage_blocks: CoverageGraph, coverage_counters: CoverageCounters, @@ -95,14 +97,24 @@ struct Instrumentor<'a, 'tcx> { impl<'a, 'tcx> Instrumentor<'a, 'tcx> { fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { - let hir_body = hir_body(tcx, mir_body.source.def_id()); + let source_map = tcx.sess.source_map(); + let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, mir_body.source.def_id()); let body_span = hir_body.value.span; + let source_file = source_map.lookup_source_file(body_span.lo()); + let fn_sig_span = match some_fn_sig.filter(|fn_sig| { + Lrc::ptr_eq(&source_file, &source_map.lookup_source_file(fn_sig.span.hi())) + }) { + Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()), + None => body_span.shrink_to_lo(), + }; let function_source_hash = hash_mir_source(tcx, hir_body); let basic_coverage_blocks = CoverageGraph::from_mir(mir_body); Self { pass_name, tcx, mir_body, + source_file, + fn_sig_span, body_span, basic_coverage_blocks, coverage_counters: CoverageCounters::new(function_source_hash), @@ -114,9 +126,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { let source_map = tcx.sess.source_map(); let mir_source = self.mir_body.source; let def_id = mir_source.def_id(); + let fn_sig_span = self.fn_sig_span; let body_span = self.body_span; - debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span)); + debug!( + "instrumenting {:?}, fn sig span: {}, body span: {}", + def_id, + source_map.span_to_string(fn_sig_span), + source_map.span_to_string(body_span) + ); let mut graphviz_data = debug::GraphvizData::new(); let mut debug_used_expressions = debug::UsedExpressions::new(); @@ -138,6 +156,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { // Compute `CoverageSpan`s from the `CoverageGraph`. let coverage_spans = CoverageSpans::generate_coverage_spans( &self.mir_body, + fn_sig_span, body_span, &self.basic_coverage_blocks, ); @@ -255,8 +274,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { let tcx = self.tcx; let source_map = tcx.sess.source_map(); let body_span = self.body_span; - let source_file = source_map.lookup_source_file(body_span.lo()); - let file_name = Symbol::intern(&source_file.name.to_string()); + let file_name = Symbol::intern(&self.source_file.name.to_string()); let mut bcb_counters = IndexVec::from_elem_n(None, self.basic_coverage_blocks.num_nodes()); for covspan in coverage_spans { @@ -272,47 +290,22 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { bug!("Every BasicCoverageBlock should have a Counter or Expression"); }; graphviz_data.add_bcb_coverage_span_with_counter(bcb, &covspan, &counter_kind); - // FIXME(#78542): Can spans for `TerminatorKind::Goto` be improved to avoid special - // cases? - let some_code_region = if self.is_code_region_redundant(bcb, span, body_span) { - None - } else { - Some(make_code_region(file_name, &source_file, span, body_span)) - }; - inject_statement(self.mir_body, counter_kind, self.bcb_last_bb(bcb), some_code_region); - } - } - /// Returns true if the type of `BasicCoverageBlock` (specifically, it's `BasicBlock`s - /// `TerminatorKind`) with the given `Span` (relative to the `body_span`) is known to produce - /// a redundant coverage count. - /// - /// There is at least one case for this, and if it's not handled, the last line in a function - /// will be double-counted. - /// - /// If this method returns `true`, the counter (which other `Expressions` may depend on) is - /// still injected, but without an associated code region. - // FIXME(#78542): Can spans for `TerminatorKind::Goto` be improved to avoid special cases? - fn is_code_region_redundant( - &self, - bcb: BasicCoverageBlock, - span: Span, - body_span: Span, - ) -> bool { - if span.hi() == body_span.hi() { - // All functions execute a `Return`-terminated `BasicBlock`, regardless of how the - // function returns; but only some functions also _can_ return after a `Goto` block - // that ends on the closing brace of the function (with the `Return`). When this - // happens, the last character is counted 2 (or possibly more) times, when we know - // the function returned only once (of course). By giving all `Goto` terminators at - // the end of a function a `non-reportable` code region, they are still counted - // if appropriate, but they don't increment the line counter, as long as their is - // also a `Return` on that last line. - if let TerminatorKind::Goto { .. } = self.bcb_terminator(bcb).kind { - return true; - } + debug!( + "Calling make_code_region(file_name={}, source_file={:?}, span={}, body_span={})", + file_name, + self.source_file, + source_map.span_to_string(span), + source_map.span_to_string(body_span) + ); + + inject_statement( + self.mir_body, + counter_kind, + self.bcb_last_bb(bcb), + Some(make_code_region(file_name, &self.source_file, span, body_span)), + ); } - false } /// `inject_coverage_span_counters()` looped through the `CoverageSpan`s and injected the @@ -412,11 +405,6 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { } #[inline] - fn bcb_terminator(&self, bcb: BasicCoverageBlock) -> &Terminator<'tcx> { - self.bcb_data(bcb).terminator(self.mir_body) - } - - #[inline] fn bcb_data(&self, bcb: BasicCoverageBlock) -> &BasicCoverageBlockData { &self.basic_coverage_blocks[bcb] } @@ -521,10 +509,15 @@ fn make_code_region( } } -fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { +fn fn_sig_and_body<'tcx>( + tcx: TyCtxt<'tcx>, + def_id: DefId, +) -> (Option<&'tcx rustc_hir::FnSig<'tcx>>, &'tcx rustc_hir::Body<'tcx>) { + // FIXME(#79625): Consider improving MIR to provide the information needed, to avoid going back + // to HIR for it. let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); - tcx.hir().body(fn_body_id) + (hir::map::fn_sig(hir_node), tcx.hir().body(fn_body_id)) } fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { diff --git a/compiler/rustc_mir/src/transform/coverage/query.rs b/compiler/rustc_mir/src/transform/coverage/query.rs index e86bb96d29c..aa34ae70ef1 100644 --- a/compiler/rustc_mir/src/transform/coverage/query.rs +++ b/compiler/rustc_mir/src/transform/coverage/query.rs @@ -1,6 +1,8 @@ +use super::*; + use rustc_middle::mir::coverage::*; use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{Coverage, CoverageInfo, Location}; +use rustc_middle::mir::{self, Coverage, CoverageInfo, Location}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::DefId; @@ -9,6 +11,8 @@ use rustc_span::def_id::DefId; /// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR). pub(crate) fn provide(providers: &mut Providers) { providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); + providers.covered_file_name = |tcx, def_id| covered_file_name(tcx, def_id); + providers.covered_code_regions = |tcx, def_id| covered_code_regions(tcx, def_id); } /// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in @@ -123,3 +127,34 @@ fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo coverage_visitor.info } + +fn covered_file_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option<Symbol> { + let mir_body = tcx.optimized_mir(def_id); + for bb_data in mir_body.basic_blocks().iter() { + for statement in bb_data.statements.iter() { + if let StatementKind::Coverage(box ref coverage) = statement.kind { + if let Some(code_region) = coverage.code_region.as_ref() { + return Some(code_region.file_name); + } + } + } + } + None +} + +fn covered_code_regions<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Vec<&'tcx CodeRegion> { + let mir_body: &'tcx mir::Body<'tcx> = tcx.optimized_mir(def_id); + mir_body + .basic_blocks() + .iter() + .map(|data| { + data.statements.iter().filter_map(|statement| match statement.kind { + StatementKind::Coverage(box ref coverage) => { + coverage.code_region.as_ref() // may be None + } + _ => None, + }) + }) + .flatten() + .collect() +} diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index 95c49922262..fd3e782f6df 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -1,10 +1,9 @@ use super::debug::term_type; -use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; +use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB}; use crate::util::spanview::source_range_no_file; use rustc_data_structures::graph::WithNumNodes; -use rustc_index::bit_set::BitSet; use rustc_middle::mir::{ self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, @@ -74,6 +73,10 @@ pub(super) struct CoverageSpan { } impl CoverageSpan { + pub fn for_fn_sig(fn_sig_span: Span) -> Self { + Self { span: fn_sig_span, bcb: START_BCB, coverage_statements: vec![], is_closure: false } + } + pub fn for_statement( statement: &Statement<'tcx>, span: Span, @@ -82,10 +85,10 @@ impl CoverageSpan { stmt_index: usize, ) -> Self { let is_closure = match statement.kind { - StatementKind::Assign(box ( - _, - Rvalue::Aggregate(box AggregateKind::Closure(_, _), _), - )) => true, + StatementKind::Assign(box (_, Rvalue::Aggregate(box ref kind, _))) => match kind { + AggregateKind::Closure(_, _) | AggregateKind::Generator(_, _, _) => true, + _ => false, + }, _ => false, }; @@ -109,9 +112,6 @@ impl CoverageSpan { pub fn merge_from(&mut self, mut other: CoverageSpan) { debug_assert!(self.is_mergeable(&other)); self.span = self.span.to(other.span); - if other.is_closure { - self.is_closure = true; - } self.coverage_statements.append(&mut other.coverage_statements); } @@ -171,6 +171,9 @@ pub struct CoverageSpans<'a, 'tcx> { /// The MIR, used to look up `BasicBlockData`. mir_body: &'a mir::Body<'tcx>, + /// A `Span` covering the signature of function for the MIR. + fn_sig_span: Span, + /// A `Span` covering the function body of the MIR (typically from left curly brace to right /// curly brace). body_span: Span, @@ -214,13 +217,36 @@ pub struct CoverageSpans<'a, 'tcx> { } impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { + /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be + /// counted. + /// + /// The basic steps are: + /// + /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each + /// `BasicCoverageBlockData`. + /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position + /// are sorted with longer spans before shorter spans; and equal spans are sorted + /// (deterministically) based on "dominator" relationship (if any). + /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance, + /// if another span or spans are already counting the same code region), or should be merged + /// into a broader combined span (because it represents a contiguous, non-branching, and + /// uninterrupted region of source code). + /// + /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since + /// closures have their own MIR, their `Span` in their enclosing function should be left + /// "uncovered". + /// + /// Note the resulting vector of `CoverageSpan`s may not be fully sorted (and does not need + /// to be). pub(super) fn generate_coverage_spans( mir_body: &'a mir::Body<'tcx>, + fn_sig_span: Span, body_span: Span, basic_coverage_blocks: &'a CoverageGraph, ) -> Vec<CoverageSpan> { let mut coverage_spans = CoverageSpans { mir_body, + fn_sig_span, body_span, basic_coverage_blocks, sorted_spans_iter: None, @@ -242,27 +268,6 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { coverage_spans.to_refined_spans() } - /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be - /// counted. - /// - /// The basic steps are: - /// - /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each - /// `BasicCoverageBlockData`. - /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position - /// are sorted with longer spans before shorter spans; and equal spans are sorted - /// (deterministically) based on "dominator" relationship (if any). - /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance, - /// if another span or spans are already counting the same code region), or should be merged - /// into a broader combined span (because it represents a contiguous, non-branching, and - /// uninterrupted region of source code). - /// - /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since - /// closures have their own MIR, their `Span` in their enclosing function should be left - /// "uncovered". - /// - /// Note the resulting vector of `CoverageSpan`s does may not be fully sorted (and does not need - /// to be). fn mir_to_initial_sorted_coverage_spans(&self) -> Vec<CoverageSpan> { let mut initial_spans = Vec::<CoverageSpan>::with_capacity(self.mir_body.num_nodes() * 2); for (bcb, bcb_data) in self.basic_coverage_blocks.iter_enumerated() { @@ -277,6 +282,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { return initial_spans; } + initial_spans.push(CoverageSpan::for_fn_sig(self.fn_sig_span)); + initial_spans.sort_unstable_by(|a, b| { if a.span.lo() == b.span.lo() { if a.span.hi() == b.span.hi() { @@ -331,7 +338,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { prev={:?}", self.prev() ); - self.discard_curr(); + self.take_curr(); } else if self.curr().is_closure { self.carve_out_span_for_closure(); } else if self.prev_original_span == self.curr().span { @@ -345,28 +352,28 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { debug!(" AT END, adding last prev={:?}", self.prev()); let prev = self.take_prev(); - let CoverageSpans { - mir_body, basic_coverage_blocks, pending_dups, mut refined_spans, .. - } = self; + let CoverageSpans { pending_dups, mut refined_spans, .. } = self; for dup in pending_dups { debug!(" ...adding at least one pending dup={:?}", dup); refined_spans.push(dup); } - refined_spans.push(prev); - - // Remove `CoverageSpan`s with empty spans ONLY if the empty `CoverageSpan`s BCB also has at - // least one other non-empty `CoverageSpan`. - let mut has_coverage = BitSet::new_empty(basic_coverage_blocks.num_nodes()); - for covspan in &refined_spans { - if !covspan.span.is_empty() { - has_coverage.insert(covspan.bcb); - } + + // Async functions wrap a closure that implements the body to be executed. The enclosing + // function is called and returns an `impl Future` without initially executing any of the + // body. To avoid showing the return from the enclosing function as a "covered" return from + // the closure, the enclosing function's `TerminatorKind::Return`s `CoverageSpan` is + // excluded. The closure's `Return` is the only one that will be counted. This provides + // adequate coverage, and more intuitive counts. (Avoids double-counting the closing brace + // of the function body.) + let body_ends_with_closure = if let Some(last_covspan) = refined_spans.last() { + last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi() + } else { + false + }; + + if !body_ends_with_closure { + refined_spans.push(prev); } - refined_spans.retain(|covspan| { - !(covspan.span.is_empty() - && is_goto(&basic_coverage_blocks[covspan.bcb].terminator(mir_body).kind) - && has_coverage.contains(covspan.bcb)) - }); // Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage // regions for the current function leave room for the closure's own coverage regions @@ -491,8 +498,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the /// `curr` coverage span. - fn discard_curr(&mut self) { - self.some_curr = None; + fn take_curr(&mut self) -> CoverageSpan { + self.some_curr.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) } /// Returns true if the curr span should be skipped because prev has already advanced beyond the @@ -508,11 +515,11 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { self.prev().span.hi() <= self.curr().span.lo() } - /// If `prev`s span extends left of the closure (`curr`), carve out the closure's - /// span from `prev`'s span. (The closure's coverage counters will be injected when - /// processing the closure's own MIR.) Add the portion of the span to the left of the - /// closure; and if the span extends to the right of the closure, update `prev` to - /// that portion of the span. For any `pending_dups`, repeat the same process. + /// If `prev`s span extends left of the closure (`curr`), carve out the closure's span from + /// `prev`'s span. (The closure's coverage counters will be injected when processing the + /// closure's own MIR.) Add the portion of the span to the left of the closure; and if the span + /// extends to the right of the closure, update `prev` to that portion of the span. For any + /// `pending_dups`, repeat the same process. fn carve_out_span_for_closure(&mut self) { let curr_span = self.curr().span; let left_cutoff = curr_span.lo(); @@ -541,7 +548,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { dup.span = dup.span.with_lo(right_cutoff); } self.pending_dups.append(&mut pending_dups); - self.discard_curr(); // since self.prev() was already updated + let closure_covspan = self.take_curr(); + self.refined_spans.push(closure_covspan); // since self.prev() was already updated } else { pending_dups.clear(); } @@ -705,30 +713,8 @@ pub(super) fn filtered_terminator_span( | TerminatorKind::DropAndReplace { .. } | TerminatorKind::SwitchInt { .. } // For `FalseEdge`, only the `real` branch is taken, so it is similar to a `Goto`. - // FIXME(richkadel): Note that `Goto` was moved to it's own match arm, for the reasons - // described below. Add tests to confirm whether or not similar cases also apply to - // `FalseEdge`. - | TerminatorKind::FalseEdge { .. } => None, - - // FIXME(#78542): Can spans for `TerminatorKind::Goto` be improved to avoid special cases? - // - // `Goto`s are often the targets of `SwitchInt` branches, and certain important - // optimizations to replace some `Counter`s with `Expression`s require a separate - // `BasicCoverageBlock` for each branch, to support the `Counter`, when needed. - // - // Also, some test cases showed that `Goto` terminators, and to some degree their `Span`s, - // provided useful context for coverage, such as to count and show when `if` blocks - // _without_ `else` blocks execute the `false` case (counting when the body of the `if` - // was _not_ taken). In these cases, the `Goto` span is ultimately given a `CoverageSpan` - // of 1 character, at the end of it's original `Span`. - // - // However, in other cases, a visible `CoverageSpan` is not wanted, but the `Goto` - // block must still be counted (for example, to contribute its count to an `Expression` - // that reports the execution count for some other block). In these cases, the code region - // is set to `None`. (See `Instrumentor::is_code_region_redundant()`.) - TerminatorKind::Goto { .. } => { - Some(function_source_span(terminator.source_info.span.shrink_to_hi(), body_span)) - } + | TerminatorKind::FalseEdge { .. } + | TerminatorKind::Goto { .. } => None, // Retain spans from all other terminators TerminatorKind::Resume @@ -749,11 +735,3 @@ fn function_source_span(span: Span, body_span: Span) -> Span { let span = original_sp(span, body_span).with_ctxt(SyntaxContext::root()); if body_span.contains(span) { span } else { body_span } } - -#[inline(always)] -fn is_goto(term_kind: &TerminatorKind<'tcx>) -> bool { - match term_kind { - TerminatorKind::Goto { .. } => true, - _ => false, - } -} diff --git a/compiler/rustc_mir/src/transform/promote_consts.rs b/compiler/rustc_mir/src/transform/promote_consts.rs index 927aae82a36..8d5ed747c3f 100644 --- a/compiler/rustc_mir/src/transform/promote_consts.rs +++ b/compiler/rustc_mir/src/transform/promote_consts.rs @@ -22,7 +22,7 @@ use rustc_middle::ty::cast::CastTy; use rustc_middle::ty::subst::InternalSubsts; use rustc_middle::ty::{self, List, TyCtxt, TypeFoldable}; use rustc_span::symbol::sym; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::Span; use rustc_index::vec::{Idx, IndexVec}; use rustc_target::spec::abi::Abi; @@ -326,41 +326,16 @@ impl<'tcx> Validator<'_, 'tcx> { if place.projection.contains(&ProjectionElem::Deref) { return Err(Unpromotable); } - - let mut has_mut_interior = - self.qualif_local::<qualifs::HasMutInterior>(place.local); - // HACK(eddyb) this should compute the same thing as - // `<HasMutInterior as Qualif>::in_projection` from - // `check_consts::qualifs` but without recursion. - if has_mut_interior { - // This allows borrowing fields which don't have - // `HasMutInterior`, from a type that does, e.g.: - // `let _: &'static _ = &(Cell::new(1), 2).1;` - let mut place_projection = &place.projection[..]; - // FIXME(eddyb) use a forward loop instead of a reverse one. - while let &[ref proj_base @ .., elem] = place_projection { - // FIXME(eddyb) this is probably excessive, with - // the exception of `union` member accesses. - let ty = - Place::ty_from(place.local, proj_base, self.body, self.tcx) - .projection_ty(self.tcx, elem) - .ty; - if ty.is_freeze(self.tcx.at(DUMMY_SP), self.param_env) { - has_mut_interior = false; - break; - } - - place_projection = proj_base; - } + if self.qualif_local::<qualifs::NeedsDrop>(place.local) { + return Err(Unpromotable); } // FIXME(eddyb) this duplicates part of `validate_rvalue`. + let has_mut_interior = + self.qualif_local::<qualifs::HasMutInterior>(place.local); if has_mut_interior { return Err(Unpromotable); } - if self.qualif_local::<qualifs::NeedsDrop>(place.local) { - return Err(Unpromotable); - } if let BorrowKind::Mut { .. } = kind { let ty = place.ty(self.body, self.tcx).ty; @@ -692,28 +667,7 @@ impl<'tcx> Validator<'_, 'tcx> { self.validate_place(place)?; - // HACK(eddyb) this should compute the same thing as - // `<HasMutInterior as Qualif>::in_projection` from - // `check_consts::qualifs` but without recursion. - let mut has_mut_interior = - self.qualif_local::<qualifs::HasMutInterior>(place.local); - if has_mut_interior { - let mut place_projection = place.projection; - // FIXME(eddyb) use a forward loop instead of a reverse one. - while let &[ref proj_base @ .., elem] = place_projection { - // FIXME(eddyb) this is probably excessive, with - // the exception of `union` member accesses. - let ty = Place::ty_from(place.local, proj_base, self.body, self.tcx) - .projection_ty(self.tcx, elem) - .ty; - if ty.is_freeze(self.tcx.at(DUMMY_SP), self.param_env) { - has_mut_interior = false; - break; - } - - place_projection = proj_base; - } - } + let has_mut_interior = self.qualif_local::<qualifs::HasMutInterior>(place.local); if has_mut_interior { return Err(Unpromotable); } diff --git a/compiler/rustc_mir/src/util/generic_graphviz.rs b/compiler/rustc_mir/src/util/generic_graphviz.rs index 8bd4a512bbb..fd55a4dfc4c 100644 --- a/compiler/rustc_mir/src/util/generic_graphviz.rs +++ b/compiler/rustc_mir/src/util/generic_graphviz.rs @@ -116,9 +116,13 @@ impl< write!(w, r#"<table border="0" cellborder="1" cellspacing="0">"#)?; - // FIXME(richkadel): Need generic way to know if node header should have a different color + // FIXME(richkadel): If/when migrating the MIR graphviz to this generic implementation, + // we need generic way to know if node header should have a different color. For example, + // for MIR: + // // let (blk, bgcolor) = if data.is_cleanup { - // (format!("{:?} (cleanup)", node), "lightblue") + // let color = if dark_mode { "royalblue" } else { "lightblue" }; + // (format!("{:?} (cleanup)", node), color) // } else { // let color = if dark_mode { "dimgray" } else { "gray" }; // (format!("{:?}", node), color) |