changes after review

crates/compiler/can/src/builtins.rs

@@ -87,7 +87,7 @@ macro_rules! map_symbol_to_lowlevel_and_arity {
                 LowLevel::PtrCast => unimplemented!(),
                 LowLevel::PtrStore => unimplemented!(),
                 LowLevel::PtrLoad => unimplemented!(),
-                LowLevel::PtrToStackValue => unimplemented!(),
+                LowLevel::Alloca => unimplemented!(),
                 LowLevel::RefCountIncRcPtr => unimplemented!(),
                 LowLevel::RefCountDecRcPtr=> unimplemented!(),
                 LowLevel::RefCountIncDataPtr => unimplemented!(),

crates/compiler/collections/src/vec_set.rs

@@ -102,16 +102,6 @@ impl<T: PartialEq> VecSet<T> {
     {
         self.elements.retain(f)
     }
-
-    pub fn keep_if_in_both(&mut self, other: &Self) {
-        self.elements.retain(|e| other.contains(e));
-    }
-
-    pub fn keep_if_in_either(&mut self, other: Self) {
-        for e in other.elements {
-            self.insert(e);
-        }
-    }
 }
 
 impl<A: Ord> Extend<A> for VecSet<A> {

crates/compiler/gen_dev/src/generic64/mod.rs

@@ -2791,14 +2791,7 @@ impl<
                     .storage_manager
                     .load_to_general_reg(&mut self.buf, structure);
 
-                let mask_symbol = self.debug_symbol("tag_id_mask");
-                let mask_reg = self
-                    .storage_manager
-                    .claim_general_reg(&mut self.buf, &mask_symbol);
-                ASM::mov_reg64_imm64(&mut self.buf, mask_reg, (!0b111) as _);
-
-                // mask out the tag id bits
-                ASM::and_reg64_reg64_reg64(&mut self.buf, mask_reg, ptr_reg, mask_reg);
+                let (mask_symbol, mask_reg) = self.clear_tag_id(ptr_reg);
 
                 let mut offset = 0;
                 for field in &other_fields[..index as usize] {
@@ -2814,6 +2807,8 @@ impl<
                     element_layout,
                     *sym,
                 );
+
+                self.free_symbol(&mask_symbol)
             }
             UnionLayout::Recursive(tag_layouts) => {
                 let other_fields = tag_layouts[tag_id as usize];
@@ -2824,22 +2819,13 @@ impl<
                     .load_to_general_reg(&mut self.buf, structure);
 
                 // mask out the tag id bits
-                let unmasked_reg = if union_layout
-                    .stores_tag_id_as_data(self.storage_manager.target_info)
-                {
-                    ptr_reg
-                } else {
-                    let umasked_symbol = self.debug_symbol("unmasked");
-                    let unmasked_reg = self
-                        .storage_manager
-                        .claim_general_reg(&mut self.buf, &umasked_symbol);
-
-                    ASM::mov_reg64_imm64(&mut self.buf, unmasked_reg, (!0b111) as _);
-
-                    ASM::and_reg64_reg64_reg64(&mut self.buf, unmasked_reg, ptr_reg, unmasked_reg);
-
-                    unmasked_reg
-                };
+                let (unmasked_symbol, unmasked_reg) =
+                    if union_layout.stores_tag_id_as_data(self.storage_manager.target_info) {
+                        (None, ptr_reg)
+                    } else {
+                        let (mask_symbol, mask_reg) = self.clear_tag_id(ptr_reg);
+                        (Some(mask_symbol), mask_reg)
+                    };
 
                 let mut offset = 0;
                 for field in &other_fields[..index as usize] {
@@ -2855,6 +2841,10 @@ impl<
                     element_layout,
                     *sym,
                 );
+
+                if let Some(unmasked_symbol) = unmasked_symbol {
+                    self.free_symbol(&unmasked_symbol);
+                }
             }
         }
     }
@@ -2911,21 +2901,16 @@ impl<
                     other_tags[tag_id as usize - 1]
                 };
 
-                let mask_symbol = self.debug_symbol("tag_id_mask");
-                let mask_reg = self
-                    .storage_manager
-                    .claim_general_reg(&mut self.buf, &mask_symbol);
-                ASM::mov_reg64_imm64(&mut self.buf, mask_reg, (!0b111) as _);
-
-                // mask out the tag id bits
-                ASM::and_reg64_reg64_reg64(&mut self.buf, mask_reg, ptr_reg, mask_reg);
+                let (mask_symbol, mask_reg) = self.clear_tag_id(ptr_reg);
 
                 let mut offset = 0;
                 for field in &other_fields[..index as usize] {
                     offset += self.layout_interner.stack_size(*field);
                 }
 
                 ASM::add_reg64_reg64_imm32(&mut self.buf, sym_reg, mask_reg, offset as i32);
+
+                self.free_symbol(&mask_symbol);
             }
             UnionLayout::Recursive(tag_layouts) => {
                 let other_fields = tag_layouts[tag_id as usize];
@@ -2935,29 +2920,24 @@ impl<
                     .load_to_general_reg(&mut self.buf, structure);
 
                 // mask out the tag id bits
-                let unmasked_reg = if union_layout
-                    .stores_tag_id_as_data(self.storage_manager.target_info)
-                {
-                    ptr_reg
-                } else {
-                    let umasked_symbol = self.debug_symbol("unmasked");
-                    let unmasked_reg = self
-                        .storage_manager
-                        .claim_general_reg(&mut self.buf, &umasked_symbol);
-
-                    ASM::mov_reg64_imm64(&mut self.buf, unmasked_reg, (!0b111) as _);
-
-                    ASM::and_reg64_reg64_reg64(&mut self.buf, unmasked_reg, ptr_reg, unmasked_reg);
-
-                    unmasked_reg
-                };
+                let (unmasked_symbol, unmasked_reg) =
+                    if union_layout.stores_tag_id_as_data(self.storage_manager.target_info) {
+                        (None, ptr_reg)
+                    } else {
+                        let (mask_symbol, mask_reg) = self.clear_tag_id(ptr_reg);
+                        (Some(mask_symbol), mask_reg)
+                    };
 
                 let mut offset = 0;
                 for field in &other_fields[..index as usize] {
                     offset += self.layout_interner.stack_size(*field);
                 }
 
                 ASM::add_reg64_reg64_imm32(&mut self.buf, sym_reg, unmasked_reg, offset as i32);
+
+                if let Some(unmasked_symbol) = unmasked_symbol {
+                    self.free_symbol(&unmasked_symbol);
+                }
             }
         }
     }
@@ -3986,6 +3966,19 @@ impl<
         CC: CallConv<GeneralReg, FloatReg, ASM>,
     > Backend64Bit<'a, 'r, GeneralReg, FloatReg, ASM, CC>
 {
+    fn clear_tag_id(&mut self, ptr_reg: GeneralReg) -> (Symbol, GeneralReg) {
+        let unmasked_symbol = self.debug_symbol("unmasked");
+        let unmasked_reg = self
+            .storage_manager
+            .claim_general_reg(&mut self.buf, &unmasked_symbol);
+
+        ASM::mov_reg64_imm64(&mut self.buf, unmasked_reg, (!0b111) as _);
+
+        ASM::and_reg64_reg64_reg64(&mut self.buf, unmasked_reg, ptr_reg, unmasked_reg);
+
+        (unmasked_symbol, unmasked_reg)
+    }
+
     fn compare(
         &mut self,
         op: CompareOperation,

crates/compiler/gen_dev/src/lib.rs

@@ -1605,7 +1605,7 @@ trait Backend<'a> {
                 self.build_ptr_load(*sym, args[0], *ret_layout);
             }
 
-            LowLevel::PtrToStackValue => {
+            LowLevel::Alloca => {
                 self.build_ptr_to_stack_value(*sym, args[0], arg_layouts[0]);
             }
 

crates/compiler/gen_llvm/src/llvm/build.rs

@@ -1502,7 +1502,7 @@ pub(crate) fn build_exp_expr<'a, 'ctx>(
                     let ptr = tag_pointer_clear_tag_id(env, argument.into_pointer_value());
                     let target_loaded_type = basic_type_from_layout(env, layout_interner, ret_repr);
 
-                    union_field_at_index(
+                    union_field_ptr_at_index(
                         env,
                         layout_interner,
                         field_layouts,
@@ -1518,7 +1518,7 @@ pub(crate) fn build_exp_expr<'a, 'ctx>(
                     let struct_type = basic_type_from_layout(env, layout_interner, struct_layout);
                     let target_loaded_type = basic_type_from_layout(env, layout_interner, ret_repr);
 
-                    union_field_at_index(
+                    union_field_ptr_at_index(
                         env,
                         layout_interner,
                         field_layouts,
@@ -1546,7 +1546,7 @@ pub(crate) fn build_exp_expr<'a, 'ctx>(
                     let ptr = tag_pointer_clear_tag_id(env, argument.into_pointer_value());
                     let target_loaded_type = basic_type_from_layout(env, layout_interner, ret_repr);
 
-                    union_field_at_index(
+                    union_field_ptr_at_index(
                         env,
                         layout_interner,
                         field_layouts,
@@ -1569,7 +1569,7 @@ pub(crate) fn build_exp_expr<'a, 'ctx>(
                     let struct_type = basic_type_from_layout(env, layout_interner, struct_layout);
                     let target_loaded_type = basic_type_from_layout(env, layout_interner, ret_repr);
 
-                    union_field_at_index(
+                    union_field_ptr_at_index(
                         env,
                         layout_interner,
                         field_layouts,
@@ -2156,7 +2156,7 @@ fn lookup_at_index_ptr<'a, 'ctx>(
     struct_type: Option<StructType<'ctx>>,
     target_loaded_type: BasicTypeEnum<'ctx>,
 ) -> BasicValueEnum<'ctx> {
-    let elem_ptr = union_field_at_index_help(
+    let elem_ptr = union_field_ptr_at_index_help(
         env,
         layout_interner,
         field_layouts,
@@ -2179,7 +2179,7 @@ fn lookup_at_index_ptr<'a, 'ctx>(
     cast_if_necessary_for_opaque_recursive_pointers(env.builder, result, target_loaded_type)
 }
 
-fn union_field_at_index_help<'a, 'ctx>(
+fn union_field_ptr_at_index_help<'a, 'ctx>(
     env: &Env<'a, 'ctx, '_>,
     layout_interner: &STLayoutInterner<'a>,
     field_layouts: &'a [InLayout<'a>],
@@ -2213,7 +2213,7 @@ fn union_field_at_index_help<'a, 'ctx>(
         .unwrap()
 }
 
-fn union_field_at_index<'a, 'ctx>(
+fn union_field_ptr_at_index<'a, 'ctx>(
     env: &Env<'a, 'ctx, '_>,
     layout_interner: &STLayoutInterner<'a>,
     field_layouts: &'a [InLayout<'a>],
@@ -2222,7 +2222,7 @@ fn union_field_at_index<'a, 'ctx>(
     value: PointerValue<'ctx>,
     target_loaded_type: BasicTypeEnum<'ctx>,
 ) -> PointerValue<'ctx> {
-    let result = union_field_at_index_help(
+    let result = union_field_ptr_at_index_help(
         env,
         layout_interner,
         field_layouts,

crates/compiler/gen_llvm/src/llvm/lowlevel.rs

@@ -1325,7 +1325,7 @@ pub(crate) fn run_low_level<'a, 'ctx>(
                 .new_build_load(element_type, ptr.into_pointer_value(), "ptr_load")
         }
 
-        PtrToStackValue => {
+        Alloca => {
             arguments!(initial_value);
 
             let ptr = entry_block_alloca_zerofill(env, initial_value.get_type(), "stack_value");

crates/compiler/gen_wasm/src/low_level.rs

@@ -1979,8 +1979,8 @@ impl<'a> LowLevelCall<'a> {
                 );
             }
             PtrLoad => backend.expr_unbox(self.ret_symbol, self.arguments[0]),
-            PtrToStackValue => {
-                // PtrToStackValue : a -> Ptr a
+            Alloca => {
+                // Alloca : a -> Ptr a
                 let arg = self.arguments[0];
                 let arg_layout = backend.storage.symbol_layouts.get(&arg).unwrap();
 

crates/compiler/module/src/low_level.rs

@@ -120,7 +120,7 @@ pub enum LowLevel {
     PtrCast,
     PtrStore,
     PtrLoad,
-    PtrToStackValue,
+    Alloca,
     RefCountIncRcPtr,
     RefCountDecRcPtr,
     RefCountIncDataPtr,
@@ -232,7 +232,7 @@ macro_rules! map_symbol_to_lowlevel {
                 LowLevel::PtrCast => unimplemented!(),
                 LowLevel::PtrStore => unimplemented!(),
                 LowLevel::PtrLoad => unimplemented!(),
-                LowLevel::PtrToStackValue => unimplemented!(),
+                LowLevel::Alloca => unimplemented!(),
                 LowLevel::RefCountIncRcPtr => unimplemented!(),
                 LowLevel::RefCountDecRcPtr=> unimplemented!(),
                 LowLevel::RefCountIncDataPtr => unimplemented!(),

crates/compiler/mono/Cargo.toml

@@ -27,6 +27,7 @@ roc_types = { path = "../types" }
 ven_pretty = { path = "../../vendor/pretty" }
 
 bitvec.workspace = true
+arrayvec.workspace = true
 bumpalo.workspace = true
 hashbrown.workspace = true
 parking_lot.workspace = true

crates/compiler/mono/src/borrow.rs

@@ -1045,7 +1045,7 @@ pub fn lowlevel_borrow_signature(arena: &Bump, op: LowLevel) -> &[Ownership] {
 
         PtrStore => arena.alloc_slice_copy(&[owned, owned]),
         PtrLoad => arena.alloc_slice_copy(&[owned]),
-        PtrToStackValue => arena.alloc_slice_copy(&[owned]),
+        Alloca => arena.alloc_slice_copy(&[owned]),
 
         PtrCast | RefCountIncRcPtr | RefCountDecRcPtr | RefCountIncDataPtr | RefCountDecDataPtr
         | RefCountIsUnique => {

crates/compiler/mono/src/drop_specialization.rs

@@ -1680,7 +1680,7 @@ fn low_level_no_rc(lowlevel: &LowLevel) -> RC {
         // only inserted for internal purposes. RC should not touch it
         PtrStore => RC::NoRc,
         PtrLoad => RC::NoRc,
-        PtrToStackValue => RC::NoRc,
+        Alloca => RC::NoRc,
 
         PtrCast | RefCountIncRcPtr | RefCountDecRcPtr | RefCountIncDataPtr | RefCountDecDataPtr
         | RefCountIsUnique => {

crates/compiler/mono/src/ir.rs

@@ -7659,6 +7659,7 @@ fn substitute_in_expr<'a>(
             None => None,
         },
 
+        // currently only used for tail recursion modulo cons (TRMC)
         UnionFieldPtrAtIndex {
             structure,
             tag_id,

crates/compiler/mono/src/layout.rs

@@ -674,7 +674,10 @@ pub(crate) enum LayoutWrapper<'a> {
 pub enum LayoutRepr<'a> {
     Builtin(Builtin<'a>),
     Struct(&'a [InLayout<'a>]),
+    // A (heap allocated) reference-counted value
     Boxed(InLayout<'a>),
+    // A pointer (heap or stack) without any reference counting
+    // Ptr is not user-facing. The compiler author must make sure that invariants are upheld
     Ptr(InLayout<'a>),
     Union(UnionLayout<'a>),
     LambdaSet(LambdaSet<'a>),