jit: #1093 review follow-ups on the operation dump, the slice-index match, and the closure-select fixtures

majit/majit-translate/src/front/mir.rs

@@ -23888,12 +23888,14 @@ mod tests {
         );
     }
 
-    /// Array slicing keeps the residual RangeTo path because the general stop
-    /// has no length proof; mutable indexing is likewise residual because it
-    /// writes through a view.
+    /// Array slicing keeps the residual RangeTo path because a general stop has
+    /// no proof that `end <= slice.len()`.  Both halves are asserted — the
+    /// residual call is present AND no `__getslice_rangeto` marker was planted
+    /// — so a lowering change that drops the call for an unrelated reason is a
+    /// failure rather than a pass.
     #[test]
     #[ignore]
-    fn call_function_impl_result_has_no_residual_array_index() {
+    fn call_function_impl_result_keeps_residual_array_index() {
         use crate::model::OpKind;
         let path = concat!(
             env!("CARGO_MANIFEST_DIR"),
@@ -23902,19 +23904,31 @@ mod tests {
         let llbc = Llbc::load(path).expect("load real LLBC");
         let graph = super::lower_function(&llbc, "call_function_impl_result")
             .expect("lower call_function_impl_result");
+        let calls_path = |want: &[&str]| -> usize {
+            let want: Vec<String> = want.iter().map(|s| s.to_string()).collect();
+            graph
+                .blocks
+                .iter()
+                .flat_map(|b| &b.operations)
+                .filter(|op| {
+                    matches!(
+                        &op.kind,
+                        OpKind::Call {
+                            target: crate::model::CallTarget::FunctionPath { segments },
+                            ..
+                        } if segments == &want
+                    )
+                })
+                .count()
+        };
         assert!(
-            graph.blocks.iter().flat_map(|b| &b.operations).any(|op| {
-                matches!(
-                    &op.kind,
-                    OpKind::Call {
-                        target: crate::model::CallTarget::FunctionPath { segments },
-                        ..
-                    } if segments
-                        == &["core", "array", "<Impl>", "index"]
-                            .map(str::to_string)
-                )
-            }),
+            calls_path(&["core", "array", "<Impl>", "index"]) >= 1,
             "general RangeTo array index remains residual"
         );
+        assert_eq!(
+            calls_path(&["__getslice_rangeto"]),
+            0,
+            "no general RangeTo site is rewritten — the fold is declined"
+        );
     }
 }

majit/majit-translate/src/front/option_closure_select.rs

@@ -486,20 +486,37 @@ fn emit_call_once(
 mod tests {
     use super::*;
 
+    /// The receiver's `Option<T>` and the result's `Option<U>` are separate
+    /// instantiations with separate owner roots.  The fixture keeps them
+    /// distinct so an assertion on a constructed variant's `owner_root` fails
+    /// if the rewriter builds the result under the receiver's owners.
+    const RECV_OPTION: &str = "test::recv::Option";
+    const RECV_SOME: &str = "test::recv::Option::Some";
+    const RESULT_OPTION: &str = "test::result::Option";
+    const RESULT_SOME: &str = "test::result::Option::Some";
+
     fn site(kind: ClosureCombinator, result_var: Variable) -> ClosureSelectSite {
+        site_with_result_niche(kind, result_var, false)
+    }
+
+    fn site_with_result_niche(
+        kind: ClosureCombinator,
+        result_var: Variable,
+        result_niche: bool,
+    ) -> ClosureSelectSite {
         ClosureSelectSite {
             kind,
             result_var,
-            option_owner: "core::option::Option".into(),
-            some_owner: "core::option::Option::Some".into(),
+            option_owner: RECV_OPTION.into(),
+            some_owner: RECV_SOME.into(),
             call_once_owner: "test::closure".into(),
             payload_ty: ValueType::Int,
             call_result_ty: ValueType::Int,
             args_tuple_suffix: String::new(),
             niche: false,
-            result_option_owner: "core::option::Option".into(),
-            result_some_owner: "core::option::Option::Some".into(),
-            result_niche: false,
+            result_option_owner: RESULT_OPTION.into(),
+            result_some_owner: RESULT_SOME.into(),
+            result_niche,
         }
     }
 
@@ -514,7 +531,7 @@ mod tests {
             .push_op_var(
                 a,
                 OpKind::Call {
-                    target: CallTarget::method(method, Some("core::option::Option".into())),
+                    target: CallTarget::method(method, Some(RECV_OPTION.into())),
                     args: vec![opt, env],
                     result_ty: ValueType::Int,
                 },
@@ -544,6 +561,40 @@ mod tests {
         ) == 0
     }
 
+    /// The distinct `owner_root`s of every `FieldWrite` naming `field_name`,
+    /// sorted.  A built variant is spelled by its writes, so this is what tells
+    /// the result's owners apart from the receiver's.
+    fn field_write_owners(g: &FunctionGraph, field_name: &str) -> Vec<String> {
+        let mut owners: Vec<String> = g
+            .blocks
+            .iter()
+            .flat_map(|blk| &blk.operations)
+            .filter_map(|op| match &op.kind {
+                OpKind::FieldWrite { field, .. } if field.name == field_name => {
+                    Some(field.owner_root.clone().unwrap_or_default())
+                }
+                _ => None,
+            })
+            .collect();
+        owners.sort();
+        owners.dedup();
+        owners
+    }
+
+    fn count_ctors(g: &FunctionGraph) -> usize {
+        count_calls(
+            g,
+            |t| matches!(t, CallTarget::SyntheticTransparentCtor { name, .. } if name == "Option"),
+        )
+    }
+
+    fn count_null_mut(g: &FunctionGraph) -> usize {
+        count_calls(g, |t| {
+            matches!(t, CallTarget::FunctionPath { segments }
+                if segments == &["core", "ptr", "null_mut"].map(str::to_string))
+        })
+    }
+
     #[test]
     fn map_selects_some_call_wrapped_and_none() {
         let (g, a) = build_and_rewrite(ClosureCombinator::Map, "map");
@@ -558,11 +609,21 @@ mod tests {
         );
         assert_eq!(g.blocks[a].exits.len(), 2, "A branches to Some/None arms");
         // Two `Option` ctors: Some(f(x)) in the then arm, None in the else arm.
-        let ctors = count_calls(
-            &g,
-            |t| matches!(t, CallTarget::SyntheticTransparentCtor { name, .. } if name == "Option"),
+        assert_eq!(count_ctors(&g), 2, "map builds Some(U) and None");
+        // Both built variants are keyed to the RESULT roots, never the
+        // receiver's: `map`'s `Option<U>` is a different instantiation.
+        assert_eq!(
+            field_write_owners(&g, "__discriminant"),
+            vec![RESULT_OPTION],
+            "both built variants key the result enum root, not the receiver's"
+        );
+        // The other `__pos_0` write is the closure's `(x,)` Args tuple, which is
+        // a real `Tuple` and unrelated to either `Option` instantiation.
+        assert_eq!(
+            field_write_owners(&g, "__pos_0"),
+            vec!["Tuple".to_string(), RESULT_SOME.to_string()],
+            "the Some(U) payload keys the result Some variant"
         );
-        assert_eq!(ctors, 2, "map builds Some(U) and None");
     }
 
     #[test]
@@ -581,14 +642,90 @@ mod tests {
             "the Some arm calls the closure once"
         );
         // Only the None arm builds an Option; the Some arm forwards the call.
-        let ctors = count_calls(
-            &g,
-            |t| matches!(t, CallTarget::SyntheticTransparentCtor { name, .. } if name == "Option"),
+        assert_eq!(count_ctors(&g), 1, "and_then builds only None");
+        assert_eq!(
+            field_write_owners(&g, "__discriminant"),
+            vec![RESULT_OPTION],
+            "the built None keys the result enum root, not the receiver's"
         );
-        assert_eq!(ctors, 1, "and_then builds only None");
         assert_eq!(g.blocks[a].exits.len(), 2);
     }
 
+    /// A niche result `Option` is a one-word pointer: `Some(x)` is `x` and
+    /// `None` is null, so neither arm may build an aggregate.
+    fn build_and_rewrite_niche_result(kind: ClosureCombinator, method: &str) -> FunctionGraph {
+        let mut g = FunctionGraph::new("test_closure_select_niche_result");
+        let a = g.startblock;
+        let opt = g.push_op_var(a, OpKind::ConstInt(0), true).unwrap();
+        let env = g.push_op_var(a, OpKind::ConstInt(7), true).unwrap();
+        let result = g
+            .push_op_var(
+                a,
+                OpKind::Call {
+                    target: CallTarget::method(method, Some(RECV_OPTION.into())),
+                    args: vec![opt, env],
+                    result_ty: ValueType::Ref(None),
+                },
+                true,
+            )
+            .unwrap();
+        let (b, _b_args) = g.create_block_with_arg_vars(1);
+        g.set_return(b, None);
+        g.set_goto(a, b, vec![result.clone()]);
+        let rewritten =
+            rewire_closure_select_call_sites(&mut g, &[site_with_result_niche(kind, result, true)]);
+        assert_eq!(
+            rewritten, 1,
+            "the niche-result {method} site must be rewritten"
+        );
+        g
+    }
+
+    #[test]
+    fn map_with_niche_result_builds_no_aggregate() {
+        let g = build_and_rewrite_niche_result(ClosureCombinator::Map, "map");
+        assert!(residual_gone(&g, "map"), "residual map call removed");
+        assert_eq!(
+            count_calls(
+                &g,
+                |t| matches!(t, CallTarget::Method { name, .. } if name == "call_once")
+            ),
+            1,
+            "the Some arm still calls the closure once"
+        );
+        assert_eq!(
+            count_ctors(&g),
+            0,
+            "a niche Some(f(x)) is f(x) itself — no Option aggregate"
+        );
+        assert_eq!(
+            field_write_owners(&g, "__discriminant"),
+            Vec::<String>::new(),
+            "a niche Option has no discriminant field to write"
+        );
+        assert_eq!(count_null_mut(&g), 1, "the None arm is the null pointer");
+    }
+
+    #[test]
+    fn and_then_with_niche_result_builds_no_aggregate() {
+        let g = build_and_rewrite_niche_result(ClosureCombinator::AndThen, "and_then");
+        assert!(
+            residual_gone(&g, "and_then"),
+            "residual and_then call removed"
+        );
+        assert_eq!(
+            count_ctors(&g),
+            0,
+            "the Some arm forwards the closure's own Option — no aggregate"
+        );
+        assert_eq!(
+            field_write_owners(&g, "__discriminant"),
+            Vec::<String>::new(),
+            "a niche Option has no discriminant field to write"
+        );
+        assert_eq!(count_null_mut(&g), 1, "the None arm is the null pointer");
+    }
+
     #[test]
     fn unwrap_or_else_forwards_payload_and_calls_on_none() {
         let (g, a) = build_and_rewrite(ClosureCombinator::UnwrapOrElse, "unwrap_or_else");
@@ -713,7 +850,7 @@ mod tests {
             .push_op_var(
                 a,
                 OpKind::Call {
-                    target: CallTarget::method("map", Some("core::option::Option".into())),
+                    target: CallTarget::method("map", Some(RECV_OPTION.into())),
                     args: vec![opt, env],
                     result_ty: ValueType::Int,
                 },

majit/majit-translate/src/front/slice_index.rs

@@ -313,36 +313,34 @@ fn rewire_one_slice_index_site(
                 .map(|oi| (bi, oi))
         })
         .ok_or_else(|| format!("{name}: slice::index op vanished before rewrite"))?;
-    if let SliceIndexBounds::MinusOne { .. } = bounds {
-        graph.blocks[rb].operations[ri] = SpaceOperation {
-            result: Some(index_result),
-            kind: OpKind::Call {
-                target: CallTarget::FunctionPath {
-                    segments: vec!["__getslice_minusone".to_string()],
-                },
-                args: vec![slice],
-                result_ty: index_result_ty,
-            },
-        };
-    } else {
-        match bounds {
-            SliceIndexBounds::RangeFrom { start } => {
-                let synthetic_bound = graph.alloc_value_var();
-                graph.blocks[rb].operations[ri] = SpaceOperation {
-                    result: Some(index_result),
-                    kind: OpKind::GetSlice {
-                        args: vec![slice, start.clone(), synthetic_bound.clone()],
-                    },
-                };
-                graph.blocks[rb].operations.insert(
-                    ri,
-                    SpaceOperation {
-                        result: Some(synthetic_bound),
-                        kind: OpKind::ConstNone,
+    match bounds {
+        SliceIndexBounds::MinusOne { .. } => {
+            graph.blocks[rb].operations[ri] = SpaceOperation {
+                result: Some(index_result),
+                kind: OpKind::Call {
+                    target: CallTarget::FunctionPath {
+                        segments: vec!["__getslice_minusone".to_string()],
                     },
-                );
-            }
-            SliceIndexBounds::MinusOne { .. } => unreachable!(),
+                    args: vec![slice],
+                    result_ty: index_result_ty,
+                },
+            };
+        }
+        SliceIndexBounds::RangeFrom { start } => {
+            let synthetic_bound = graph.alloc_value_var();
+            graph.blocks[rb].operations[ri] = SpaceOperation {
+                result: Some(index_result),
+                kind: OpKind::GetSlice {
+                    args: vec![slice, start.clone(), synthetic_bound.clone()],
+                },
+            };
+            graph.blocks[rb].operations.insert(
+                ri,
+                SpaceOperation {
+                    result: Some(synthetic_bound),
+                    kind: OpKind::ConstNone,
+                },
+            );
         }
     }
     Ok(())