From 5783e73f4662595ccb3261c3cb42cdc45eb157e0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Aug 2024 21:05:51 +0200 Subject: [PATCH] make some Frame fields more private --- compiler/rustc_const_eval/src/interpret/stack.rs | 16 ++++++++++++---- src/tools/miri/src/concurrency/thread.rs | 4 ---- src/tools/miri/src/helpers.rs | 10 +++++----- src/tools/miri/src/machine.rs | 8 ++++---- src/tools/miri/src/shims/backtrace.rs | 4 ++-- src/tools/miri/src/shims/panic.rs | 4 ++-- 6 files changed, 25 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/stack.rs b/compiler/rustc_const_eval/src/interpret/stack.rs index b5f55434d5a..5733e2316fd 100644 --- a/compiler/rustc_const_eval/src/interpret/stack.rs +++ b/compiler/rustc_const_eval/src/interpret/stack.rs @@ -61,10 +61,10 @@ pub struct Frame<'tcx, Prov: Provenance = CtfeProvenance, Extra = ()> { // Function and callsite information //////////////////////////////////////////////////////////////////////////////// /// The MIR for the function called on this frame. - pub body: &'tcx mir::Body<'tcx>, + pub(super) body: &'tcx mir::Body<'tcx>, /// The def_id and args of the current function. - pub instance: ty::Instance<'tcx>, + pub(super) instance: ty::Instance<'tcx>, /// Extra data for the machine. pub extra: Extra, @@ -73,7 +73,7 @@ pub struct Frame<'tcx, Prov: Provenance = CtfeProvenance, Extra = ()> { // Return place and locals //////////////////////////////////////////////////////////////////////////////// /// Work to perform when returning from this function. - pub return_to_block: StackPopCleanup, + return_to_block: StackPopCleanup, /// The location where the result of the current stack frame should be written to, /// and its layout in the caller. @@ -101,7 +101,7 @@ pub struct Frame<'tcx, Prov: Provenance = CtfeProvenance, Extra = ()> { /// frames without cleanup code). /// /// Needs to be public because ConstProp does unspeakable things to it. - pub loc: Either, + pub(super) loc: Either, } #[derive(Clone, Copy, Eq, PartialEq, Debug)] // Miri debug-prints these @@ -269,6 +269,14 @@ impl<'tcx, Prov: Provenance, Extra> Frame<'tcx, Prov, Extra> { self.loc } + pub fn body(&self) -> &'tcx mir::Body<'tcx> { + self.body + } + + pub fn instance(&self) -> ty::Instance<'tcx> { + self.instance + } + /// Return the `SourceInfo` of the current instruction. pub fn current_source_info(&self) -> Option<&mir::SourceInfo> { self.loc.left().map(|loc| self.body.source_info(loc)) diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index afa91df78f9..f72591f0c4b 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -377,10 +377,6 @@ impl VisitProvenance for Frame<'_, Provenance, FrameExtra<'_>> { return_place, locals, extra, - body: _, - instance: _, - return_to_block: _, - loc: _, // There are some private fields we cannot access; they contain no tags. .. } = self; diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 5c89889506f..1bdf9f06dcd 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1105,12 +1105,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Make an attempt to get at the instance of the function this is inlined from. let instance: Option<_> = try { let scope = frame.current_source_info()?.scope; - let inlined_parent = frame.body.source_scopes[scope].inlined_parent_scope?; - let source = &frame.body.source_scopes[inlined_parent]; + let inlined_parent = frame.body().source_scopes[scope].inlined_parent_scope?; + let source = &frame.body().source_scopes[inlined_parent]; source.inlined.expect("inlined_parent_scope points to scope without inline info").0 }; // Fall back to the instance of the function itself. - let instance = instance.unwrap_or(frame.instance); + let instance = instance.unwrap_or(frame.instance()); // Now check the crate it is in. We could try to be clever here and e.g. check if this is // the same crate as `start_fn`, but that would not work for running std tests in Miri, so // we'd need some more hacks anyway. So we just check the name of the crate. If someone @@ -1350,9 +1350,9 @@ impl<'tcx> MiriMachine<'tcx> { /// This is the source of truth for the `is_user_relevant` flag in our `FrameExtra`. pub fn is_user_relevant(&self, frame: &Frame<'tcx, Provenance>) -> bool { - let def_id = frame.instance.def_id(); + let def_id = frame.instance().def_id(); (def_id.is_local() || self.local_crates.contains(&def_id.krate)) - && !frame.instance.def.requires_caller_location(self.tcx) + && !frame.instance().def.requires_caller_location(self.tcx) } } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 3e45a3a7e1a..94598e7d2e3 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1352,7 +1352,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { ) -> InterpResult<'tcx, Frame<'tcx, Provenance, FrameExtra<'tcx>>> { // Start recording our event before doing anything else let timing = if let Some(profiler) = ecx.machine.profiler.as_ref() { - let fn_name = frame.instance.to_string(); + let fn_name = frame.instance().to_string(); let entry = ecx.machine.string_cache.entry(fn_name.clone()); let name = entry.or_insert_with(|| profiler.alloc_string(&*fn_name)); @@ -1443,7 +1443,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { // tracing-tree can autoamtically annotate scope changes, but it gets very confused by our // concurrency and what it prints is just plain wrong. So we print our own information // instead. (Cc https://github.com/rust-lang/miri/issues/2266) - info!("Leaving {}", ecx.frame().instance); + info!("Leaving {}", ecx.frame().instance()); Ok(()) } @@ -1473,7 +1473,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { // Needs to be done after dropping frame to show up on the right nesting level. // (Cc https://github.com/rust-lang/miri/issues/2266) if !ecx.active_thread_stack().is_empty() { - info!("Continuing in {}", ecx.frame().instance); + info!("Continuing in {}", ecx.frame().instance()); } res } @@ -1486,7 +1486,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { let Some(Provenance::Concrete { alloc_id, .. }) = mplace.ptr().provenance else { panic!("after_local_allocated should only be called on fresh allocations"); }; - let local_decl = &ecx.frame().body.local_decls[local]; + let local_decl = &ecx.frame().body().local_decls[local]; let span = local_decl.source_info.span; ecx.machine.allocation_spans.borrow_mut().insert(alloc_id, (span, None)); Ok(()) diff --git a/src/tools/miri/src/shims/backtrace.rs b/src/tools/miri/src/shims/backtrace.rs index 42babb4c78d..9bb6777a9b0 100644 --- a/src/tools/miri/src/shims/backtrace.rs +++ b/src/tools/miri/src/shims/backtrace.rs @@ -46,8 +46,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mut data = Vec::new(); for frame in this.active_thread_stack().iter().rev() { // Match behavior of debuginfo (`FunctionCx::adjusted_span_and_dbg_scope`). - let span = hygiene::walk_chain_collapsed(frame.current_span(), frame.body.span); - data.push((frame.instance, span.lo())); + let span = hygiene::walk_chain_collapsed(frame.current_span(), frame.body().span); + data.push((frame.instance(), span.lo())); } let ptrs: Vec<_> = data diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index 2fb0319a843..ab705ddccab 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/src/shims/panic.rs @@ -48,7 +48,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn handle_miri_start_unwind(&mut self, payload: &OpTy<'tcx>) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - trace!("miri_start_unwind: {:?}", this.frame().instance); + trace!("miri_start_unwind: {:?}", this.frame().instance()); let payload = this.read_immediate(payload)?; let thread = this.active_thread_mut(); @@ -124,7 +124,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // and we are unwinding, so we should catch that. trace!( "unwinding: found catch_panic frame during unwinding: {:?}", - this.frame().instance + this.frame().instance() ); // We set the return value of `try` to 1, since there was a panic.