Merge pull request #418 from Chronostasys/fix/set_body

fix: allow call of `set_body` on none opaque structs
TheDan64 · May 13, 2023 · 41d335b · 41d335b
2 parents 74f0f96 + 61d3bb1
commit 41d335b
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 14 deletions.
diff --git a/src/types/struct_type.rs b/src/types/struct_type.rs
@@ -352,11 +352,11 @@ impl<'ctx> StructType<'ctx> {
         unsafe { StructValue::new(self.struct_type.get_undef()) }
     }
 
-    // REVIEW: SubTypes should allow this to only be implemented for StructType<Opaque> one day
-    // but would have to return StructType<Tys>. Maybe this is valid for non opaques, though
-    // it might just override types?
-    // REVIEW: What happens if called with &[] on an opaque? Should that still be opaque? Does the call break?
-    /// Defines the body of an opaue `StructType`.
+    /// Defines the body of a `StructType`.
+    ///
+    /// If the struct is an opaque type, it will no longer be after this call.
+    /// 
+    /// Resetting the `packed` state of a non-opaque struct type may not work.
     ///
     /// # Example
     ///
@@ -374,15 +374,13 @@ impl<'ctx> StructType<'ctx> {
     pub fn set_body(self, field_types: &[BasicTypeEnum<'ctx>], packed: bool) -> bool {
         let is_opaque = self.is_opaque();
         let mut field_types: Vec<LLVMTypeRef> = field_types.iter().map(|val| val.as_type_ref()).collect();
-        if is_opaque {
-            unsafe {
-                LLVMStructSetBody(
-                    self.as_type_ref(),
-                    field_types.as_mut_ptr(),
-                    field_types.len() as u32,
-                    packed as i32,
-                );
-            }
+        unsafe {
+            LLVMStructSetBody(
+                self.as_type_ref(),
+                field_types.as_mut_ptr(),
+                field_types.len() as u32,
+                packed as i32,
+            );
         }
 
         is_opaque

diff --git a/tests/all/test_types.rs b/tests/all/test_types.rs
@@ -88,6 +88,18 @@ fn test_struct_type() {
     assert!(no_longer_opaque_struct.get_field_type_at_index(2).is_none());
     assert!(no_longer_opaque_struct.get_field_type_at_index(200).is_none());
     assert_eq!(no_longer_opaque_struct.get_field_types(), vec![field_1, field_2]);
+
+    no_longer_opaque_struct.set_body(&[float_array.into(), int_vector.into(), float_array.into()], false);
+    let fields_changed_struct = no_longer_opaque_struct;
+    // assert!(!fields_changed_struct.is_packed()); FIXME: This seems to be a bug in LLVM
+    assert!(!fields_changed_struct.is_opaque());
+    assert!(fields_changed_struct.is_sized());
+    assert_eq!(fields_changed_struct.count_fields(), 3);
+    assert_eq!(
+        fields_changed_struct.get_field_types(),
+        &[float_array.into(), int_vector.into(), float_array.into(),]
+    );
+    assert!(fields_changed_struct.get_field_type_at_index(3).is_none());
 }
 
 #[test]