Rollup merge of rust-lang#129751 - RalfJung:interpret-visit-field-order, r=compiler-errors

interpret/visitor: make memory order iteration slightly more efficient

Finally I know enough about RPIT to write this iterator signature correctly. :D

This means memory-order iteration now needs an allocation, but it avoids quadratic complexity (where it has to do a linear scan n times to find the n-th field in memory order), so that seems like a win overall. The changed code only affects Miri; the rustc changes are NOPs.

Author: workingjubilee

compiler/rustc_const_eval/src/interpret/visitor.rs

@@ -25,14 +25,15 @@ pub trait ValueVisitor<'tcx, M: Machine<'tcx>>: Sized {
     }
 
     /// This function provides the chance to reorder the order in which fields are visited for
-    /// `FieldsShape::Aggregate`: The order of fields will be
-    /// `(0..num_fields).map(aggregate_field_order)`.
+    /// `FieldsShape::Aggregate`.
     ///
-    /// The default means we iterate in source declaration order; alternative this can do an inverse
-    /// lookup in `memory_index` to use memory field order instead.
+    /// The default means we iterate in source declaration order; alternatively this can do some
+    /// work with `memory_index` to iterate in memory order.
     #[inline(always)]
-    fn aggregate_field_order(_memory_index: &IndexVec<FieldIdx, u32>, idx: usize) -> usize {
-        idx
+    fn aggregate_field_iter(
+        memory_index: &IndexVec<FieldIdx, u32>,
+    ) -> impl Iterator<Item = FieldIdx> + 'static {
+        memory_index.indices()
     }
 
     // Recursive actions, ready to be overloaded.
@@ -172,9 +173,9 @@ pub trait ValueVisitor<'tcx, M: Machine<'tcx>>: Sized {
             &FieldsShape::Union(fields) => {
                 self.visit_union(v, fields)?;
             }
-            FieldsShape::Arbitrary { offsets, memory_index } => {
-                for idx in 0..offsets.len() {
-                    let idx = Self::aggregate_field_order(memory_index, idx);
+            FieldsShape::Arbitrary { memory_index, .. } => {
+                for idx in Self::aggregate_field_iter(memory_index) {
+                    let idx = idx.as_usize();
                     let field = self.ecx().project_field(v, idx)?;
                     self.visit_field(v, idx, &field)?;
                 }

src/tools/miri/src/helpers.rs

@@ -630,14 +630,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 self.ecx
             }
 
-            fn aggregate_field_order(memory_index: &IndexVec<FieldIdx, u32>, idx: usize) -> usize {
-                // We need to do an *inverse* lookup: find the field that has position `idx` in memory order.
-                for (src_field, &mem_pos) in memory_index.iter_enumerated() {
-                    if mem_pos as usize == idx {
-                        return src_field.as_usize();
-                    }
-                }
-                panic!("invalid `memory_index`, could not find {}-th field in memory order", idx);
+            fn aggregate_field_iter(
+                memory_index: &IndexVec<FieldIdx, u32>,
+            ) -> impl Iterator<Item = FieldIdx> + 'static {
+                let inverse_memory_index = memory_index.invert_bijective_mapping();
+                inverse_memory_index.into_iter()
             }
 
             // Hook to detect `UnsafeCell`.