Merge pull request #526 from madsmtm/future-proof-struct-encodings

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
@@ -197,7 +206,7 @@ impl Encoding {
     /// For example, you should not rely on two equivalent encodings to have
     /// the same size or ABI - that is provided on a best-effort basis.
     pub fn equivalent_to(&self, other: &Self) -> bool {
-        compare_encodings(self, NestingLevel::new(), other, NestingLevel::new(), false)
+        compare_encodings(self, other, NestingLevel::new(), false)
     }
 
     /// Check if an encoding is equivalent to the given string representation.
@@ -223,7 +232,7 @@ impl Encoding {
     /// See [`Encoding::equivalent_to`] for details about the meaning of
     /// "equivalence".
     pub fn equivalent_to_box(&self, other: &EncodingBox) -> bool {
-        compare_encodings(self, NestingLevel::new(), other, NestingLevel::new(), false)
+        compare_encodings(self, other, NestingLevel::new(), false)
     }
 }
 
@@ -235,7 +244,7 @@ impl Encoding {
 /// Objective-C compilers.
 impl fmt::Display for Encoding {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", Helper::new(self, NestingLevel::new()))
+        Helper::new(self).fmt(f, NestingLevel::new())
     }
 }
 
@@ -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 {
@@ -135,13 +135,13 @@ impl EncodingBox {
 /// Same formatting as [`Encoding`]'s `Display` implementation.
 impl fmt::Display for EncodingBox {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", Helper::from_box(self, NestingLevel::new()))
+        Helper::from_box(self).fmt(f, NestingLevel::new())
     }
 }
 
 impl PartialEq<Encoding> for EncodingBox {
     fn eq(&self, other: &Encoding) -> bool {
-        compare_encodings(self, NestingLevel::new(), other, NestingLevel::new(), true)
+        compare_encodings(self, other, NestingLevel::new(), true)
     }
 }
 
@@ -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);