Future-proof struct encodings

Author: madsmtm

crates/objc2-encode/CHANGELOG.md

@@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased - YYYY-MM-DD
 
+### Changed
+* **BREAKING**: Changed the type of `EncodingBox::Struct` and
+  `EncodingBox::Union` to no longer contain an `Option`.
+
+### Fixed
+* Fixed encoding equivalence between empty structs and unions.
+
 
 ## 3.0.0 - 2023-07-31
 

crates/objc2-encode/src/encoding.rs

@@ -104,8 +104,8 @@ pub enum Encoding {
     /// are possible for the type.
     ///
     /// A `BitField(_, Some(_))` and a `BitField(_, None)` do _not_ compare
-    /// equal; instead, you should set the bitfield depending on the target
-    /// platform.
+    /// equal; instead, you should set the bitfield correctly depending on the
+    /// target platform.
     BitField(u8, Option<&'static (u64, Encoding)>),
     /// A pointer to the given type.
     ///
@@ -129,12 +129,18 @@ pub enum Encoding {
     /// It is not uncommon for the name to be `"?"`.
     ///
     /// Corresponds to the `"{" name "=" fields... "}"` code.
+    ///
+    /// Note that the `=` may be omitted in some situations; this is
+    /// considered equal to the case where there are no fields.
     Struct(&'static str, &'static [Encoding]),
-    /// A union with the given name and fields.
+    /// A union with the given name and members.
     ///
-    /// The order of the fields must match the order of the order in this.
+    /// The order of the members must match the order of the order in this.
+    ///
+    /// Corresponds to the `"(" name "=" members... ")"` code.
     ///
-    /// Corresponds to the `"(" name "=" fields... ")"` code.
+    /// Note that the `=` may be omitted in some situations; this is
+    /// considered equal to the case where there are no members.
     Union(&'static str, &'static [Encoding]),
     // TODO: "Vector" types have the '!' encoding, but are not implemented in
     // clang
@@ -189,6 +195,9 @@ impl Encoding {
     /// - Structs or unions behind multiple pointers are considered
     ///   equivalent, since Objective-C compilers strip this information to
     ///   avoid unnecessary nesting.
+    /// - Structs or unions with no fields/members are considered to represent
+    ///   "opqaue" types, and will therefore be equivalent to all other
+    ///   structs / unions.
     ///
     /// The comparison may be changed in the future to e.g. ignore struct
     /// names or similar changes that may be required because of limitations
@@ -303,16 +312,16 @@ mod tests {
 
                 // Check equivalence comparisons
                 assert!(E.equivalent_to(&E), "equivalent self");
-                assert!(E.equivalent_to_str($string), "equivalent self string");
+                assert!(E.equivalent_to_str($string), "equivalent self string {}", $string);
                 assert!(E.equivalent_to_box(&boxed), "equivalent self boxed");
                 $(
-                    assert!(E.equivalent_to(&$equivalent_encoding), "equivalent encoding");
+                    assert!(E.equivalent_to(&$equivalent_encoding), "equivalent encoding {}", $equivalent_encoding);
                     assert!(E.equivalent_to_str(&$equivalent_encoding.to_string()), "equivalent encoding string");
                     let boxed = EncodingBox::from_str(&$equivalent_encoding.to_string()).expect("parse equivalent encoding");
                     assert!(E.equivalent_to_box(&boxed), "equivalent encoding boxed");
                 )*
                 $(
-                    assert!(E.equivalent_to_str($equivalent_string), "equivalent string");
+                    assert!(E.equivalent_to_str($equivalent_string), "equivalent string {}", $equivalent_string);
                     let boxed = EncodingBox::from_str($equivalent_string).expect("parse equivalent string");
                     assert!(E.equivalent_to_box(&boxed), "equivalent string boxed");
                 )*
@@ -447,52 +456,61 @@ mod tests {
 
         fn struct_() {
             Encoding::Struct("SomeStruct", &[Encoding::Char, Encoding::Int]);
+            ~Encoding::Struct("SomeStruct", &[]);
             !Encoding::Union("SomeStruct", &[Encoding::Char, Encoding::Int]);
             !Encoding::Int;
             !Encoding::Struct("SomeStruct", &[Encoding::Int]);
             !Encoding::Struct("SomeStruct", &[Encoding::Char, Encoding::Int, Encoding::Int]);
             !Encoding::Struct("SomeStruct", &[Encoding::Int, Encoding::Char]);
             !Encoding::Struct("AnotherName", &[Encoding::Char, Encoding::Int]);
             "{SomeStruct=ci}";
-            !"{SomeStruct=ci";
+            ~"{SomeStruct=}";
             !"{SomeStruct}";
-            !"{SomeStruct=}";
+            !"{SomeStruct=ic}";
+            !"{SomeStruct=malformed";
         }
 
         fn pointer_struct() {
             Encoding::Pointer(&Encoding::Struct("SomeStruct", &[Encoding::Char, Encoding::Int]));
+            ~Encoding::Pointer(&Encoding::Struct("SomeStruct", &[]));
             !Encoding::Pointer(&Encoding::Struct("SomeStruct", &[Encoding::Int, Encoding::Char]));
             !Encoding::Pointer(&Encoding::Struct("AnotherName", &[Encoding::Char, Encoding::Int]));
             "^{SomeStruct=ci}";
-            !"^{SomeStruct=ci";
+            ~"^{SomeStruct=}";
             !"^{SomeStruct}";
-            !"^{SomeStruct=}";
+            !"^{SomeStruct=ic}";
+            !"^{SomeStruct=malformed";
         }
 
         fn pointer_pointer_struct() {
             Encoding::Pointer(&Encoding::Pointer(&Encoding::Struct("SomeStruct", &[Encoding::Char, Encoding::Int])));
+            ~Encoding::Pointer(&Encoding::Pointer(&Encoding::Struct("SomeStruct", &[])));
             ~Encoding::Pointer(&Encoding::Pointer(&Encoding::Struct("SomeStruct", &[Encoding::Int, Encoding::Char])));
             !Encoding::Pointer(&Encoding::Pointer(&Encoding::Struct("AnotherName", &[Encoding::Char, Encoding::Int])));
             "^^{SomeStruct}";
-            !"^^{SomeStruct=ci}";
-            !"^^{SomeStruct=ii}";
-            !"^^{SomeStruct=ci";
             !"^^{SomeStruct=}";
+            !"^^{SomeStruct=ci}";
+            !"^^{SomeStruct=ic}";
+            !"^^{AnotherName=ic}";
+            !"^^{SomeStruct=malformed";
         }
 
         fn atomic_struct() {
             Encoding::Atomic(&Encoding::Struct("SomeStruct", &[Encoding::Char, Encoding::Int]));
+            ~Encoding::Atomic(&Encoding::Struct("SomeStruct", &[]));
             ~Encoding::Atomic(&Encoding::Struct("SomeStruct", &[Encoding::Int, Encoding::Char]));
             !Encoding::Atomic(&Encoding::Struct("AnotherName", &[Encoding::Char, Encoding::Int]));
             "A{SomeStruct}";
-            !"A{SomeStruct=ci}";
-            !"A{SomeStruct=ci";
             !"A{SomeStruct=}";
+            !"A{SomeStruct=ci}";
+            !"A{SomeStruct=ic}";
+            !"A{SomeStruct=malformed";
         }
 
         fn empty_struct() {
             Encoding::Struct("SomeStruct", &[]);
             "{SomeStruct=}";
+            ~"{SomeStruct=ci}";
             !"{SomeStruct}";
         }
 
@@ -554,12 +572,25 @@ mod tests {
                 ]
             );
             "{abc=^[8B](def=@?)^^b255c?}";
+            ~"{abc=}";
+            !"{abc}";
         }
 
         fn identifier() {
             Encoding::Struct("_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789", &[]);
             "{_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789=}";
         }
+
+        // Regression test. The encoding of the `CGLContextObj` object changed
+        // between versions of macOS. As such, this is something that we must
+        // be prepared to handle.
+        fn cgl_context_obj() {
+            Encoding::Pointer(&Encoding::Struct("_CGLContextObject", &[]));
+            "^{_CGLContextObject=}";
+            ~"^{_CGLContextObject=^{__GLIContextRec}{__GLIFunctionDispatchRec=^?^?^?^?^?}^{_CGLPrivateObject}^v}";
+            !"^{_CGLContextObject}";
+            !"^{SomeOtherStruct=}";
+        }
     }
 
     #[test]
@@ -592,7 +623,7 @@ mod tests {
         let parsed = EncodingBox::from_str(s).unwrap();
         let expected = EncodingBox::Struct(
             "S".to_string(),
-            Some(vec![EncodingBox::Block, EncodingBox::Block]),
+            vec![EncodingBox::Block, EncodingBox::Block],
         );
         assert_eq!(parsed, expected);
 

crates/objc2-encode/src/encoding_box.rs

@@ -85,9 +85,9 @@ pub enum EncodingBox {
     /// Same as [`Encoding::Array`].
     Array(u64, Box<Self>),
     /// Same as [`Encoding::Struct`].
-    Struct(String, Option<Vec<Self>>),
+    Struct(String, Vec<Self>),
     /// Same as [`Encoding::Union`].
-    Union(String, Option<Vec<Self>>),
+    Union(String, Vec<Self>),
 }
 
 impl EncodingBox {
@@ -188,20 +188,20 @@ mod tests {
         ));
         let enc2 = EncodingBox::Atomic(Box::new(EncodingBox::Struct(
             "test".to_string(),
-            Some(vec![EncodingBox::Array(2, Box::new(EncodingBox::Int))]),
+            vec![EncodingBox::Array(2, Box::new(EncodingBox::Int))],
         )));
         let enc3 = EncodingBox::Atomic(Box::new(EncodingBox::Struct(
             "test".to_string(),
-            Some(vec![EncodingBox::Array(2, Box::new(EncodingBox::Char))]),
+            vec![EncodingBox::Array(2, Box::new(EncodingBox::Char))],
         )));
         assert_eq!(enc1, enc2);
         assert_ne!(enc1, enc3);
     }
 
     #[test]
-    fn ne_struct_with_without_fields() {
-        let enc1 = EncodingBox::Struct("test".to_string(), Some(vec![EncodingBox::Char]));
-        let enc2 = EncodingBox::Struct("test".to_string(), None);
+    fn struct_nested_in_pointer() {
+        let enc1 = EncodingBox::Struct("test".to_string(), vec![EncodingBox::Char]);
+        let enc2 = EncodingBox::Struct("test".to_string(), vec![EncodingBox::Int]);
         const ENC3A: Encoding = Encoding::Struct("test", &[Encoding::Char]);
         assert_ne!(enc1, enc2);
         assert!(ENC3A.equivalent_to_box(&enc1));
@@ -225,15 +225,12 @@ mod tests {
     #[test]
     fn parse_atomic_struct() {
         let expected = EncodingBox::Atomic(Box::new(EncodingBox::Atomic(Box::new(
-            EncodingBox::Struct("a".into(), Some(Vec::new())),
+            EncodingBox::Struct("a".into(), vec![]),
         ))));
         let actual = EncodingBox::from_str("AA{a=}").unwrap();
         assert_eq!(expected, actual);
         assert_eq!(expected.to_string(), "AA{a}");
 
-        let expected = EncodingBox::Atomic(Box::new(EncodingBox::Atomic(Box::new(
-            EncodingBox::Struct("a".into(), None),
-        ))));
         let actual = EncodingBox::from_str("AA{a}").unwrap();
         assert_eq!(expected, actual);
         assert_eq!(expected.to_string(), "AA{a}");
@@ -243,7 +240,7 @@ mod tests {
     fn parse_part_of_string() {
         let mut s = "{a}cb0i16";
 
-        let expected = EncodingBox::Struct("a".into(), None);
+        let expected = EncodingBox::Struct("a".into(), vec![]);
         let actual = EncodingBox::from_start_of_str(&mut s).unwrap();
         assert_eq!(expected, actual);
 

crates/objc2-encode/src/helper.rs

@@ -49,7 +49,7 @@ impl NestingLevel {
         }
     }
 
-    const fn include_container_fields(self) -> bool {
+    pub(crate) const fn include_container_fields(self) -> bool {
         match self {
             Self::Top | Self::Within => true,
             Self::Bottom => false,
@@ -80,7 +80,7 @@ pub(crate) fn compare_encodings<E1: EncodingType, E2: EncodingType>(
         level2
     };
 
-    // TODO: Are level1 and level2 ever be different?
+    // TODO: Are level1 and level2 ever different?
 
     match (enc1.helper(level1), enc2.helper(level2)) {
         (Primitive(p1), Primitive(p2)) => p1 == p2,
@@ -106,9 +106,16 @@ pub(crate) fn compare_encodings<E1: EncodingType, E2: EncodingType>(
         (Container(kind1, name1, items1, level1), Container(kind2, name2, items2, level2)) => {
             kind1 == kind2
                 && name1 == name2
-                && match (items1, items2) {
-                    (None, None) => true,
-                    (Some(items1), Some(items2)) => {
+                && match (
+                    items1,
+                    items2,
+                    level1.include_container_fields() && level2.include_container_fields(),
+                ) {
+                    (_, _, false) => true,
+                    // If one container is empty, then they are equivalent
+                    ([], _, true) => true,
+                    (_, [], true) => true,
+                    (items1, items2, true) => {
                         if items1.len() != items2.len() {
                             return false;
                         }
@@ -119,14 +126,6 @@ pub(crate) fn compare_encodings<E1: EncodingType, E2: EncodingType>(
                         }
                         true
                     }
-                    // A bit unsure about this one, but the "safe" default
-                    // here is to say that a container with items does not
-                    // compare equal to another container without items.
-                    //
-                    // Note that this may be confusing, since a `Pointer` to
-                    // the two containers might suddenly start comparing
-                    // equal, but
-                    _ => false,
                 }
         }
         (_, _) => false,
@@ -263,7 +262,7 @@ pub(crate) enum Helper<'a, E = Encoding> {
     BitField(u8, Option<&'a (u64, E)>, NestingLevel),
     Indirection(IndirectionKind, &'a E, NestingLevel),
     Array(u64, &'a E, NestingLevel),
-    Container(ContainerKind, &'a str, Option<&'a [E]>, NestingLevel),
+    Container(ContainerKind, &'a str, &'a [E], NestingLevel),
 }
 
 impl<E: EncodingType> fmt::Display for Helper<'_, E> {
@@ -285,7 +284,7 @@ impl<E: EncodingType> fmt::Display for Helper<'_, E> {
             Self::Container(kind, name, items, level) => {
                 write!(f, "{}", kind.start())?;
                 write!(f, "{name}")?;
-                if let Some(items) = items {
+                if level.include_container_fields() {
                     write!(f, "=")?;
                     for item in *items {
                         write!(f, "{}", item.helper(*level))?;
@@ -333,22 +332,12 @@ impl Helper<'_> {
                 if !verify_name(name) {
                     panic!("Struct name was not a valid identifier");
                 }
-                let fields = if level.include_container_fields() {
-                    Some(*fields)
-                } else {
-                    None
-                };
                 Self::Container(ContainerKind::Struct, name, fields, level.container())
             }
             Union(name, members) => {
                 if !verify_name(name) {
                     panic!("Union name was not a valid identifier");
                 }
-                let members = if level.include_container_fields() {
-                    Some(*members)
-                } else {
-                    None
-                };
                 Self::Container(ContainerKind::Union, name, members, level.container())
             }
         }
@@ -391,22 +380,12 @@ impl<'a> Helper<'a, EncodingBox> {
                 if !verify_name(name) {
                     panic!("Struct name was not a valid identifier");
                 }
-                let fields = if level.include_container_fields() {
-                    fields.as_deref()
-                } else {
-                    None
-                };
                 Self::Container(ContainerKind::Struct, name, fields, level.container())
             }
             Union(name, members) => {
                 if !verify_name(name) {
                     panic!("Union name was not a valid identifier");
                 }
-                let members = if level.include_container_fields() {
-                    members.as_deref()
-                } else {
-                    None
-                };
                 Self::Container(ContainerKind::Union, name, members, level.container())
             }
         }