Use static dispatch in the visitor

saethlin · RalfJung · commit eb2a36b01801 · 2022-10-04T15:32:10.000+02:00
diff --git a/src/concurrency/thread.rs b/src/concurrency/thread.rs
@@ -182,15 +182,15 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {
 }
 
 impl VisitMachineValues for Thread<'_, '_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let Thread { panic_payload, last_error, stack, state: _, thread_name: _, join_status: _ } =
             self;
 
         if let Some(payload) = panic_payload {
-            visit(&Operand::Immediate(Immediate::Scalar(*payload)))
+            visit.visit(*payload);
         }
         if let Some(error) = last_error {
-            visit(&Operand::Indirect(**error))
+            visit.visit(**error);
         }
         for frame in stack {
             frame.visit_machine_values(visit)
@@ -199,7 +199,7 @@ impl VisitMachineValues for Thread<'_, '_> {
 }
 
 impl VisitMachineValues for Frame<'_, '_, Provenance, FrameData<'_>> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let Frame {
             return_place,
             locals,
@@ -213,12 +213,12 @@ impl VisitMachineValues for Frame<'_, '_, Provenance, FrameData<'_>> {
 
         // Return place.
         if let Place::Ptr(mplace) = **return_place {
-            visit(&Operand::Indirect(mplace));
+            visit.visit(mplace);
         }
         // Locals.
         for local in locals.iter() {
             if let LocalValue::Live(value) = &local.value {
-                visit(value);
+                visit.visit(value);
             }
         }
 
@@ -299,7 +299,7 @@ impl<'mir, 'tcx> Default for ThreadManager<'mir, 'tcx> {
 }
 
 impl VisitMachineValues for ThreadManager<'_, '_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let ThreadManager {
             threads,
             thread_local_alloc_ids,
@@ -313,8 +313,7 @@ impl VisitMachineValues for ThreadManager<'_, '_> {
             thread.visit_machine_values(visit);
         }
         for ptr in thread_local_alloc_ids.borrow().values().copied() {
-            let ptr: Pointer<Option<Provenance>> = ptr.into();
-            visit(&Operand::Indirect(MemPlace::from_ptr(ptr)));
+            visit.visit(ptr);
         }
         // FIXME: Do we need to do something for TimeoutCallback? That's a Box<dyn>, not sure what
         // to do.
diff --git a/src/concurrency/weak_memory.rs b/src/concurrency/weak_memory.rs
@@ -108,19 +108,15 @@ pub struct StoreBufferAlloc {
     store_buffers: RefCell<RangeObjectMap<StoreBuffer>>,
 }
 
-impl VisitProvenance for StoreBufferAlloc {
-    fn visit_provenance(&self, visitor: &mut impl FnMut(SbTag)) {
+impl VisitMachineValues for StoreBufferAlloc {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         for val in self
             .store_buffers
             .borrow()
             .iter()
             .flat_map(|buf| buf.buffer.iter().map(|element| &element.val))
         {
-            if let Scalar::Ptr(ptr, _) = val {
-                if let Provenance::Concrete { sb, .. } = ptr.provenance {
-                    visitor(sb);
-                }
-            }
+            visit.visit(val);
         }
     }
 }
diff --git a/src/lib.rs b/src/lib.rs
@@ -112,7 +112,7 @@ pub use crate::range_map::RangeMap;
 pub use crate::stacked_borrows::{
     CallId, EvalContextExt as StackedBorEvalContextExt, Item, Permission, SbTag, Stack, Stacks,
 };
-pub use crate::tag_gc::{EvalContextExt as _, VisitMachineValues, VisitProvenance};
+pub use crate::tag_gc::{EvalContextExt as _, ProvenanceVisitor, VisitMachineValues};
 
 /// Insert rustc arguments at the beginning of the argument list that Miri wants to be
 /// set per default, for maximal validation power.
diff --git a/src/machine.rs b/src/machine.rs
@@ -64,7 +64,7 @@ impl<'tcx> std::fmt::Debug for FrameData<'tcx> {
 }
 
 impl VisitMachineValues for FrameData<'_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let FrameData { catch_unwind, stacked_borrows: _, timing: _ } = self;
 
         if let Some(catch_unwind) = catch_unwind {
@@ -261,6 +261,20 @@ pub struct AllocExtra {
     pub weak_memory: Option<weak_memory::AllocExtra>,
 }
 
+impl VisitMachineValues for AllocExtra {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
+        let AllocExtra { stacked_borrows, data_race: _, weak_memory } = self;
+
+        if let Some(stacked_borrows) = stacked_borrows {
+            stacked_borrows.borrow().visit_machine_values(visit);
+        }
+
+        if let Some(weak_memory) = weak_memory {
+            weak_memory.visit_machine_values(visit);
+        }
+    }
+}
+
 /// Precomputed layouts of primitive types
 pub struct PrimitiveLayouts<'tcx> {
     pub unit: TyAndLayout<'tcx>,
@@ -602,7 +616,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
 }
 
 impl VisitMachineValues for MiriMachine<'_, '_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let MiriMachine {
             threads,
             tls,
@@ -621,17 +635,16 @@ impl VisitMachineValues for MiriMachine<'_, '_> {
         dir_handler.visit_machine_values(visit);
 
         if let Some(argc) = argc {
-            visit(&Operand::Indirect(*argc));
+            visit.visit(argc);
         }
         if let Some(argv) = argv {
-            visit(&Operand::Indirect(*argv));
+            visit.visit(argv);
         }
         if let Some(cmd_line) = cmd_line {
-            visit(&Operand::Indirect(*cmd_line));
+            visit.visit(cmd_line);
         }
         for ptr in extern_statics.values().copied() {
-            let ptr: Pointer<Option<Provenance>> = ptr.into();
-            visit(&Operand::Indirect(MemPlace::from_ptr(ptr)));
+            visit.visit(ptr);
         }
     }
 }
diff --git a/src/shims/env.rs b/src/shims/env.rs
@@ -37,15 +37,14 @@ pub struct EnvVars<'tcx> {
 }
 
 impl VisitMachineValues for EnvVars<'_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let EnvVars { map, environ } = self;
 
         for ptr in map.values() {
-            visit(&Operand::Indirect(MemPlace::from_ptr(*ptr)));
+            visit.visit(*ptr);
         }
-
         if let Some(env) = environ {
-            visit(&Operand::Indirect(**env));
+            visit.visit(**env);
         }
     }
 }
diff --git a/src/shims/panic.rs b/src/shims/panic.rs
@@ -36,10 +36,10 @@ pub struct CatchUnwindData<'tcx> {
 }
 
 impl VisitMachineValues for CatchUnwindData<'_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let CatchUnwindData { catch_fn, data, dest: _, ret: _ } = self;
-        visit(&Operand::Indirect(MemPlace::from_ptr(*catch_fn)));
-        visit(&Operand::Immediate(Immediate::Scalar(*data)));
+        visit.visit(catch_fn);
+        visit.visit(data);
     }
 }
 
diff --git a/src/shims/tls.rs b/src/shims/tls.rs
@@ -236,14 +236,14 @@ impl<'tcx> TlsData<'tcx> {
 }
 
 impl VisitMachineValues for TlsData<'_> {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let TlsData { keys, macos_thread_dtors, next_key: _, dtors_running: _ } = self;
 
         for scalar in keys.values().flat_map(|v| v.data.values()) {
-            visit(&Operand::Immediate(Immediate::Scalar(*scalar)));
+            visit.visit(scalar);
         }
         for (_, scalar) in macos_thread_dtors.values() {
-            visit(&Operand::Immediate(Immediate::Scalar(*scalar)));
+            visit.visit(scalar);
         }
     }
 }
diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs
@@ -463,11 +463,11 @@ impl Default for DirHandler {
 }
 
 impl VisitMachineValues for DirHandler {
-    fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         let DirHandler { streams, next_id: _ } = self;
 
         for dir in streams.values() {
-            visit(&Operand::Indirect(MemPlace::from_ptr(dir.entry)));
+            visit.visit(dir.entry);
         }
     }
 }
diff --git a/src/stacked_borrows/mod.rs b/src/stacked_borrows/mod.rs
@@ -513,10 +513,10 @@ impl Stacks {
     }
 }
 
-impl VisitProvenance for Stacks {
-    fn visit_provenance(&self, visit: &mut impl FnMut(SbTag)) {
+impl VisitMachineValues for Stacks {
+    fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {
         for tag in self.exposed_tags.iter().copied() {
-            visit(tag);
+            visit.visit(tag);
         }
     }
 }
diff --git a/src/tag_gc.rs b/src/tag_gc.rs

Original file line number	Diff line number	Diff line change
`@@ -182,15 +182,15 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {`
`182`	`182`	`}`
`183`	`183`
`184`	`184`	`impl VisitMachineValues for Thread<'_, '_> {`
`185`		`- fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {`
	`185`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`186`	`186`	`let Thread { panic_payload, last_error, stack, state: _, thread_name: _, join_status: _ } =`
`187`	`187`	`self;`
`188`	`188`
`189`	`189`	`if let Some(payload) = panic_payload {`
`190`		`- visit(&Operand::Immediate(Immediate::Scalar(*payload)))`
	`190`	`+ visit.visit(*payload);`
`191`	`191`	`}`
`192`	`192`	`if let Some(error) = last_error {`
`193`		`- visit(&Operand::Indirect(**error))`
	`193`	`+ visit.visit(**error);`
`194`	`194`	`}`
`195`	`195`	`for frame in stack {`
`196`	`196`	`frame.visit_machine_values(visit)`
`@@ -199,7 +199,7 @@ impl VisitMachineValues for Thread<'_, '_> {`
`199`	`199`	`}`
`200`	`200`
`201`	`201`	`impl VisitMachineValues for Frame<'_, '_, Provenance, FrameData<'_>> {`
`202`		`- fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {`
	`202`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`203`	`203`	`let Frame {`
`204`	`204`	`return_place,`
`205`	`205`	`locals,`
`@@ -213,12 +213,12 @@ impl VisitMachineValues for Frame<'_, '_, Provenance, FrameData<'_>> {`
`213`	`213`
`214`	`214`	`// Return place.`
`215`	`215`	`if let Place::Ptr(mplace) = **return_place {`
`216`		`- visit(&Operand::Indirect(mplace));`
	`216`	`+ visit.visit(mplace);`
`217`	`217`	`}`
`218`	`218`	`// Locals.`
`219`	`219`	`for local in locals.iter() {`
`220`	`220`	`if let LocalValue::Live(value) = &local.value {`
`221`		`- visit(value);`
	`221`	`+ visit.visit(value);`
`222`	`222`	`}`
`223`	`223`	`}`
`224`	`224`
`@@ -299,7 +299,7 @@ impl<'mir, 'tcx> Default for ThreadManager<'mir, 'tcx> {`
`299`	`299`	`}`
`300`	`300`
`301`	`301`	`impl VisitMachineValues for ThreadManager<'_, '_> {`
`302`		`- fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {`
	`302`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`303`	`303`	`let ThreadManager {`
`304`	`304`	`threads,`
`305`	`305`	`thread_local_alloc_ids,`
`@@ -313,8 +313,7 @@ impl VisitMachineValues for ThreadManager<'_, '_> {`
`313`	`313`	`thread.visit_machine_values(visit);`
`314`	`314`	`}`
`315`	`315`	`for ptr in thread_local_alloc_ids.borrow().values().copied() {`
`316`		`- let ptr: Pointer<Option<Provenance>> = ptr.into();`
`317`		`- visit(&Operand::Indirect(MemPlace::from_ptr(ptr)));`
	`316`	`+ visit.visit(ptr);`
`318`	`317`	`}`
`319`	`318`	`// FIXME: Do we need to do something for TimeoutCallback? That's a Box<dyn>, not sure what`
`320`	`319`	`// to do.`
Original file line number	Diff line number	Diff line change
`@@ -108,19 +108,15 @@ pub struct StoreBufferAlloc {`
`108`	`108`	`store_buffers: RefCell<RangeObjectMap<StoreBuffer>>,`
`109`	`109`	`}`
`110`	`110`
`111`		`-impl VisitProvenance for StoreBufferAlloc {`
`112`		`- fn visit_provenance(&self, visitor: &mut impl FnMut(SbTag)) {`
	`111`	`+impl VisitMachineValues for StoreBufferAlloc {`
	`112`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`113`	`113`	`for val in self`
`114`	`114`	`.store_buffers`
`115`	`115`	`.borrow()`
`116`	`116`	`.iter()`
`117`	`117`	`.flat_map(\|buf\| buf.buffer.iter().map(\|element\| &element.val))`
`118`	`118`	`{`
`119`		`- if let Scalar::Ptr(ptr, _) = val {`
`120`		`- if let Provenance::Concrete { sb, .. } = ptr.provenance {`
`121`		`- visitor(sb);`
`122`		`- }`
`123`		`- }`
	`119`	`+ visit.visit(val);`
`124`	`120`	`}`
`125`	`121`	`}`
`126`	`122`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,15 +37,14 @@ pub struct EnvVars<'tcx> {`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`impl VisitMachineValues for EnvVars<'_> {`
`40`		`- fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {`
	`40`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`41`	`41`	`let EnvVars { map, environ } = self;`
`42`	`42`
`43`	`43`	`for ptr in map.values() {`
`44`		`- visit(&Operand::Indirect(MemPlace::from_ptr(*ptr)));`
	`44`	`+ visit.visit(*ptr);`
`45`	`45`	`}`
`46`		`-`
`47`	`46`	`if let Some(env) = environ {`
`48`		`- visit(&Operand::Indirect(**env));`
	`47`	`+ visit.visit(**env);`
`49`	`48`	`}`
`50`	`49`	`}`
`51`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,10 +36,10 @@ pub struct CatchUnwindData<'tcx> {`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`impl VisitMachineValues for CatchUnwindData<'_> {`
`39`		`- fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {`
	`39`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`40`	`40`	`let CatchUnwindData { catch_fn, data, dest: _, ret: _ } = self;`
`41`		`- visit(&Operand::Indirect(MemPlace::from_ptr(*catch_fn)));`
`42`		`- visit(&Operand::Immediate(Immediate::Scalar(*data)));`
	`41`	`+ visit.visit(catch_fn);`
	`42`	`+ visit.visit(data);`
`43`	`43`	`}`
`44`	`44`	`}`
`45`	`45`
Original file line number	Diff line number	Diff line change
`@@ -236,14 +236,14 @@ impl<'tcx> TlsData<'tcx> {`
`236`	`236`	`}`
`237`	`237`
`238`	`238`	`impl VisitMachineValues for TlsData<'_> {`
`239`		`- fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {`
	`239`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`240`	`240`	`let TlsData { keys, macos_thread_dtors, next_key: _, dtors_running: _ } = self;`
`241`	`241`
`242`	`242`	`for scalar in keys.values().flat_map(\|v\| v.data.values()) {`
`243`		`- visit(&Operand::Immediate(Immediate::Scalar(*scalar)));`
	`243`	`+ visit.visit(scalar);`
`244`	`244`	`}`
`245`	`245`	`for (_, scalar) in macos_thread_dtors.values() {`
`246`		`- visit(&Operand::Immediate(Immediate::Scalar(*scalar)));`
	`246`	`+ visit.visit(scalar);`
`247`	`247`	`}`
`248`	`248`	`}`
`249`	`249`	`}`
Original file line number	Diff line number	Diff line change
`@@ -463,11 +463,11 @@ impl Default for DirHandler {`
`463`	`463`	`}`
`464`	`464`
`465`	`465`	`impl VisitMachineValues for DirHandler {`
`466`		`- fn visit_machine_values(&self, visit: &mut impl FnMut(&Operand<Provenance>)) {`
	`466`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`467`	`467`	`let DirHandler { streams, next_id: _ } = self;`
`468`	`468`
`469`	`469`	`for dir in streams.values() {`
`470`		`- visit(&Operand::Indirect(MemPlace::from_ptr(dir.entry)));`
	`470`	`+ visit.visit(dir.entry);`
`471`	`471`	`}`
`472`	`472`	`}`
`473`	`473`	`}`
Original file line number	Diff line number	Diff line change
`@@ -513,10 +513,10 @@ impl Stacks {`
`513`	`513`	`}`
`514`	`514`	`}`
`515`	`515`
`516`		`-impl VisitProvenance for Stacks {`
`517`		`- fn visit_provenance(&self, visit: &mut impl FnMut(SbTag)) {`
	`516`	`+impl VisitMachineValues for Stacks {`
	`517`	`+ fn visit_machine_values(&self, visit: &mut ProvenanceVisitor) {`
`518`	`518`	`for tag in self.exposed_tags.iter().copied() {`
`519`		`- visit(tag);`
	`519`	`+ visit.visit(tag);`
`520`	`520`	`}`
`521`	`521`	`}`
`522`	`522`	`}`