remove equality impls for introspect::Type, in favor of new loose_equals() method

Author: dwrensha

capnp/src/data_list.rs

@@ -218,10 +218,9 @@ impl<'a> From<Reader<'a>> for crate::dynamic_value::Reader<'a> {
 impl<'a> crate::dynamic_value::DowncastReader<'a> for Reader<'a> {
     fn downcast_reader(v: crate::dynamic_value::Reader<'a>) -> Self {
         let dl: crate::dynamic_list::Reader = v.downcast();
-        assert_eq!(
-            dl.element_type(),
-            crate::introspect::TypeVariant::Data.into()
-        );
+        assert!(dl
+            .element_type()
+            .loose_equals(crate::introspect::TypeVariant::Data.into()));
         Reader { reader: dl.reader }
     }
 }
@@ -238,10 +237,9 @@ impl<'a> From<Builder<'a>> for crate::dynamic_value::Builder<'a> {
 impl<'a> crate::dynamic_value::DowncastBuilder<'a> for Builder<'a> {
     fn downcast_builder(v: crate::dynamic_value::Builder<'a>) -> Self {
         let dl: crate::dynamic_list::Builder = v.downcast();
-        assert_eq!(
-            dl.element_type(),
-            crate::introspect::TypeVariant::Data.into()
-        );
+        assert!(dl
+            .element_type()
+            .loose_equals(crate::introspect::TypeVariant::Data.into()));
         Builder {
             builder: dl.builder,
         }

capnp/src/dynamic_struct.rs

@@ -246,7 +246,7 @@ impl<'a> Reader<'a> {
             Into::<crate::introspect::Type>::into(crate::introspect::TypeVariant::Struct(
                 self.schema.raw
             ))
-            .may_downcast_to(T::introspect())
+            .loose_equals(T::introspect())
         );
         self.reader.into()
     }
@@ -814,7 +814,7 @@ impl<'a> Builder<'a> {
             Into::<crate::introspect::Type>::into(crate::introspect::TypeVariant::Struct(
                 self.schema.raw
             ))
-            .may_downcast_to(T::introspect())
+            .loose_equals(T::introspect())
         );
         self.builder.into()
     }

capnp/src/enum_list.rs

@@ -260,7 +260,7 @@ impl<'a, T: TryFrom<u16, Error = NotInSchema> + crate::introspect::Introspect>
 {
     fn downcast_reader(v: crate::dynamic_value::Reader<'a>) -> Self {
         let dl: crate::dynamic_list::Reader = v.downcast();
-        assert!(dl.element_type().may_downcast_to(T::introspect()));
+        assert!(dl.element_type().loose_equals(T::introspect()));
         Reader {
             reader: dl.reader,
             marker: PhantomData,
@@ -284,7 +284,7 @@ impl<'a, T: TryFrom<u16, Error = NotInSchema> + crate::introspect::Introspect>
 {
     fn downcast_builder(v: crate::dynamic_value::Builder<'a>) -> Self {
         let dl: crate::dynamic_list::Builder = v.downcast();
-        assert!(dl.element_type().may_downcast_to(T::introspect()));
+        assert!(dl.element_type().loose_equals(T::introspect()));
         Builder {
             builder: dl.builder,
             marker: PhantomData,

capnp/src/introspect.rs

@@ -13,7 +13,7 @@ pub trait Introspect {
 /// optimized to avoid heap allocation.
 ///
 /// To examine a `Type`, you should call the `which()` method.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug)]
 pub struct Type {
     /// The type, minus any outer `List( )`.
     base: BaseType,
@@ -105,9 +105,9 @@ impl Type {
         }
     }
 
-    /// Returns true is a dynamic value of type `self` is
-    /// allowed to be downcast to type `other`.
-    pub(crate) fn may_downcast_to(&self, other: Self) -> bool {
+    /// Returns true if `self` is equal to `other` modulo
+    /// type parameters and interface types.
+    pub fn loose_equals(&self, other: Self) -> bool {
         match (self.which(), other.which()) {
             (TypeVariant::Void, TypeVariant::Void) => true,
             (TypeVariant::UInt8, TypeVariant::UInt8) => true,
@@ -133,7 +133,7 @@ impl Type {
                 core::ptr::eq(rbs1.generic, rbs2.generic)
             }
             (TypeVariant::List(element1), TypeVariant::List(element2)) => {
-                element1.may_downcast_to(element2)
+                element1.loose_equals(element2)
             }
             (TypeVariant::AnyPointer, TypeVariant::AnyPointer) => true,
             (TypeVariant::Capability, TypeVariant::Capability) => true,
@@ -142,7 +142,7 @@ impl Type {
     }
 }
 
-#[derive(Copy, Clone, PartialEq, Eq)]
+#[derive(Copy, Clone)]
 /// A `Type` unfolded one level. Suitable for pattern matching. Can be trivially
 /// converted to `Type` via the `From`/`Into` traits.
 pub enum TypeVariant {
@@ -194,7 +194,7 @@ impl From<TypeVariant> for Type {
 }
 
 /// A Cap'n Proto type, excluding `List`.
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[derive(Copy, Clone, Debug)]
 enum BaseType {
     Void,
     Bool,
@@ -270,34 +270,6 @@ pub struct RawBrandedStructSchema {
     pub annotation_types: fn(Option<u16>, u32) -> Type,
 }
 
-/// WARNING!
-///
-/// In capnproto-c++, this method is implemented as a single pointer
-/// comparison. That works because C++ guarantees unique addresses
-/// for static template instantiations. In contrast, the below Rust
-/// implementation uses function pointer comparison, which might
-/// give unexpected results (particularly in the presence of type
-/// parameter instantiation).
-///
-/// For this reason, usage of equality on types in capnproto-rust
-/// is discouraged.
-///
-/// You might think that we could still implement the method
-/// in a slower way by recursively comparing types of fields.
-/// However, that (or at least the naive version of it) can hit
-/// infinite loops because e.g. a struct Foo might have a field of
-/// type Foo.
-///
-/// TODO(api_bump): remove this `impl PartialEq`.
-impl core::cmp::PartialEq for RawBrandedStructSchema {
-    #[allow(unpredictable_function_pointer_comparisons)]
-    fn eq(&self, other: &Self) -> bool {
-        core::ptr::eq(self.generic, other.generic) && self.field_types == other.field_types
-    }
-}
-
-impl core::cmp::Eq for RawBrandedStructSchema {}
-
 impl core::fmt::Debug for RawBrandedStructSchema {
     fn fmt(&self, f: &mut core::fmt::Formatter) -> core::result::Result<(), core::fmt::Error> {
         write!(

capnp/src/list_list.rs

@@ -287,7 +287,7 @@ impl<'a, T: crate::traits::Owned> From<Reader<'a, T>> for crate::dynamic_value::
 impl<'a, T: crate::traits::Owned> crate::dynamic_value::DowncastReader<'a> for Reader<'a, T> {
     fn downcast_reader(v: crate::dynamic_value::Reader<'a>) -> Self {
         let dl: crate::dynamic_list::Reader = v.downcast();
-        assert!(dl.element_type().may_downcast_to(T::introspect()));
+        assert!(dl.element_type().loose_equals(T::introspect()));
         Reader {
             reader: dl.reader,
             marker: core::marker::PhantomData,
@@ -307,7 +307,7 @@ impl<'a, T: crate::traits::Owned> From<Builder<'a, T>> for crate::dynamic_value:
 impl<'a, T: crate::traits::Owned> crate::dynamic_value::DowncastBuilder<'a> for Builder<'a, T> {
     fn downcast_builder(v: crate::dynamic_value::Builder<'a>) -> Self {
         let dl: crate::dynamic_list::Builder = v.downcast();
-        assert!(dl.element_type().may_downcast_to(T::introspect()));
+        assert!(dl.element_type().loose_equals(T::introspect()));
         Builder {
             builder: dl.builder,
             marker: core::marker::PhantomData,

capnp/src/primitive_list.rs

@@ -349,7 +349,7 @@ impl<'a, T: PrimitiveElement + crate::introspect::Introspect>
 {
     fn downcast_reader(v: crate::dynamic_value::Reader<'a>) -> Self {
         let dl: crate::dynamic_list::Reader = v.downcast();
-        assert!(dl.element_type().may_downcast_to(T::introspect()));
+        assert!(dl.element_type().loose_equals(T::introspect()));
         Reader {
             reader: dl.reader,
             marker: marker::PhantomData,
@@ -373,7 +373,7 @@ impl<'a, T: PrimitiveElement + crate::introspect::Introspect>
 {
     fn downcast_builder(v: crate::dynamic_value::Builder<'a>) -> Self {
         let dl: crate::dynamic_list::Builder = v.downcast();
-        assert!(dl.element_type().may_downcast_to(T::introspect()));
+        assert!(dl.element_type().loose_equals(T::introspect()));
         Builder {
             builder: dl.builder,
             marker: marker::PhantomData,

capnp/src/struct_list.rs

@@ -294,7 +294,7 @@ impl<'a, T: crate::traits::OwnedStruct> From<Reader<'a, T>> for crate::dynamic_v
 impl<'a, T: crate::traits::OwnedStruct> crate::dynamic_value::DowncastReader<'a> for Reader<'a, T> {
     fn downcast_reader(v: crate::dynamic_value::Reader<'a>) -> Self {
         let dl: crate::dynamic_list::Reader = v.downcast();
-        assert!(dl.element_type().may_downcast_to(T::introspect()));
+        assert!(dl.element_type().loose_equals(T::introspect()));
         Reader {
             reader: dl.reader,
             marker: PhantomData,
@@ -316,7 +316,7 @@ impl<'a, T: crate::traits::OwnedStruct> crate::dynamic_value::DowncastBuilder<'a
 {
     fn downcast_builder(v: crate::dynamic_value::Builder<'a>) -> Self {
         let dl: crate::dynamic_list::Builder = v.downcast();
-        assert!(dl.element_type().may_downcast_to(T::introspect()));
+        assert!(dl.element_type().loose_equals(T::introspect()));
         Builder {
             builder: dl.builder,
             marker: PhantomData,

capnp/src/text_list.rs

@@ -253,10 +253,9 @@ impl<'a> From<Reader<'a>> for crate::dynamic_value::Reader<'a> {
 impl<'a> crate::dynamic_value::DowncastReader<'a> for Reader<'a> {
     fn downcast_reader(v: crate::dynamic_value::Reader<'a>) -> Self {
         let dl: crate::dynamic_list::Reader = v.downcast();
-        assert_eq!(
-            dl.element_type(),
-            crate::introspect::TypeVariant::Text.into()
-        );
+        assert!(dl
+            .element_type()
+            .loose_equals(crate::introspect::TypeVariant::Text.into()));
         Reader { reader: dl.reader }
     }
 }
@@ -273,10 +272,9 @@ impl<'a> From<Builder<'a>> for crate::dynamic_value::Builder<'a> {
 impl<'a> crate::dynamic_value::DowncastBuilder<'a> for Builder<'a> {
     fn downcast_builder(v: crate::dynamic_value::Builder<'a>) -> Self {
         let dl: crate::dynamic_list::Builder = v.downcast();
-        assert_eq!(
-            dl.element_type(),
-            crate::introspect::TypeVariant::Text.into()
-        );
+        assert!(dl
+            .element_type()
+            .loose_equals(crate::introspect::TypeVariant::Text.into()));
         Builder {
             builder: dl.builder,
         }

capnpc/test/dynamic.rs

@@ -507,24 +507,11 @@ fn test_downcasts() {
     }
 }
 
-// Function pointer equality is used in
-// `impl PartialEq for RawBrandedStructSchema`. On MIRI that implies
-// that all struct types are inequal.
-//
-// The plan is to remove that impl in the future, to avoid
-// unpleasant surprises.
-#[cfg_attr(miri, ignore)]
 #[test]
-fn introspect_equality() {
+fn introspect_loose_equals() {
     use capnp::introspect::Introspect;
 
-    assert_eq!(
-        test_all_types::Owned::introspect(),
-        test_all_types::Owned::introspect()
-    );
+    assert!(test_all_types::Owned::introspect().loose_equals(test_all_types::Owned::introspect()));
 
-    assert_ne!(
-        test_all_types::Owned::introspect(),
-        test_defaults::Owned::introspect()
-    )
+    assert!(!test_all_types::Owned::introspect().loose_equals(test_defaults::Owned::introspect()))
 }

example/fill_random_values/src/lib.rs

@@ -76,7 +76,7 @@ impl<R: Rng> Filler<R> {
         for annotation in annotations {
             if annotation.get_id() == fill_capnp::select_from::choices::ID {
                 if let TypeVariant::List(element_type) = annotation.get_type().which() {
-                    if element_type != field.get_type() {
+                    if !element_type.loose_equals(field.get_type()) {
                         return Err(::capnp::Error::failed(
                             "choices annotation element type mismatch".into(),
                         ));