Auto merge of #139 - faern:fix-to-untyped-array, r=jdm

Fix bug in CFArray::to_untyped that made the retain count wrong

The following code segfaults on the `0.4.5` release:
```rust
CFArray::<CFBoolean>::from_CFTypes(&[]).to_untyped().to_untyped().to_untyped()
```
Because `to_untyped` don't increment the retain count, but when the typed array it's created from goes out of scope that one will decrement it.

And since we need to wrap under the get rule (call `CFRetain`) anyway I did not see any reason to have it as a consuming `to_` method. It's more flexible as a borrowing `as_` method.

Since you probably don't want to cause API breakage I kept the old version but fixed the bug and added a deprecation notice on it.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-foundation-rs/139)
<!-- Reviewable:end -->

core-foundation-sys/Cargo.toml

@@ -3,7 +3,7 @@ name = "core-foundation-sys"
 description = "Bindings to Core Foundation for OS X"
 homepage = "https://github.com/servo/core-foundation-rs"
 repository = "https://github.com/servo/core-foundation-rs"
-version = "0.4.5"
+version = "0.4.6"
 authors = ["The Servo Project Developers"]
 license = "MIT / Apache-2.0"
 build = "build.rs"

core-foundation/Cargo.toml

@@ -3,13 +3,13 @@ name = "core-foundation"
 description = "Bindings to Core Foundation for OS X"
 homepage = "https://github.com/servo/core-foundation-rs"
 repository = "https://github.com/servo/core-foundation-rs"
-version = "0.4.5"
+version = "0.4.6"
 authors = ["The Servo Project Developers"]
 license = "MIT / Apache-2.0"
 
 [dependencies.core-foundation-sys]
 path = "../core-foundation-sys"
-version = "0.4.5"
+version = "0.4.6"
 
 [dependencies]
 libc = "0.2"

core-foundation/src/array.rs

@@ -94,8 +94,13 @@ impl<T> CFArray<T> {
         }
     }
 
+    #[deprecated(note = "please use `as_untyped` instead")]
     pub fn to_untyped(self) -> CFArray {
-        CFArray(self.0, PhantomData)
+        unsafe { CFArray::wrap_under_get_rule(self.0) }
+    }
+
+    pub fn as_untyped(&self) -> CFArray {
+        unsafe { CFArray::wrap_under_get_rule(self.0) }
     }
 
     /// Iterates over the elements of this `CFArray`.
@@ -150,53 +155,81 @@ impl<'a, T: FromVoid> IntoIterator for &'a CFArray<T> {
     }
 }
 
-#[test]
-fn should_box_and_unbox() {
-    use number::CFNumber;
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::mem;
 
-    let n0 = CFNumber::from(0);
-    let n1 = CFNumber::from(1);
-    let n2 = CFNumber::from(2);
-    let n3 = CFNumber::from(3);
-    let n4 = CFNumber::from(4);
-    let n5 = CFNumber::from(5);
+    #[test]
+    fn to_untyped_correct_retain_count() {
+        let array = CFArray::<CFType>::from_CFTypes(&[]);
+        assert_eq!(array.retain_count(), 1);
 
-    let arr = CFArray::from_CFTypes(&[
-        n0.as_CFType(),
-        n1.as_CFType(),
-        n2.as_CFType(),
-        n3.as_CFType(),
-        n4.as_CFType(),
-        n5.as_CFType(),
-    ]);
+        let untyped_array = array.to_untyped();
+        assert_eq!(untyped_array.retain_count(), 1);
+    }
 
-    assert!(arr.get_all_values() == &[n0.as_CFTypeRef(),
-                                      n1.as_CFTypeRef(),
-                                      n2.as_CFTypeRef(),
-                                      n3.as_CFTypeRef(),
-                                      n4.as_CFTypeRef(),
-                                      n5.as_CFTypeRef()]);
+    #[test]
+    fn as_untyped_correct_retain_count() {
+        let array = CFArray::<CFType>::from_CFTypes(&[]);
+        assert_eq!(array.retain_count(), 1);
 
-    unsafe {
-        let mut sum = 0;
+        let untyped_array = array.as_untyped();
+        assert_eq!(array.retain_count(), 2);
+        assert_eq!(untyped_array.retain_count(), 2);
 
-        let mut iter = arr.iter();
-        assert_eq!(iter.len(), 6);
-        assert!(iter.next().is_some());
-        assert_eq!(iter.len(), 5);
+        mem::drop(array);
+        assert_eq!(untyped_array.retain_count(), 1);
+    }
 
-        for elem in iter {
-            let number: CFNumber = TCFType::wrap_under_get_rule(mem::transmute(elem));
-            sum += number.to_i64().unwrap()
+    #[test]
+    fn should_box_and_unbox() {
+        use number::CFNumber;
+
+        let n0 = CFNumber::from(0);
+        let n1 = CFNumber::from(1);
+        let n2 = CFNumber::from(2);
+        let n3 = CFNumber::from(3);
+        let n4 = CFNumber::from(4);
+        let n5 = CFNumber::from(5);
+
+        let arr = CFArray::from_CFTypes(&[
+            n0.as_CFType(),
+            n1.as_CFType(),
+            n2.as_CFType(),
+            n3.as_CFType(),
+            n4.as_CFType(),
+            n5.as_CFType(),
+        ]);
+
+        assert!(arr.get_all_values() == &[n0.as_CFTypeRef(),
+                                        n1.as_CFTypeRef(),
+                                        n2.as_CFTypeRef(),
+                                        n3.as_CFTypeRef(),
+                                        n4.as_CFTypeRef(),
+                                        n5.as_CFTypeRef()]);
+
+        unsafe {
+            let mut sum = 0;
+
+            let mut iter = arr.iter();
+            assert_eq!(iter.len(), 6);
+            assert!(iter.next().is_some());
+            assert_eq!(iter.len(), 5);
+
+            for elem in iter {
+                let number: CFNumber = TCFType::wrap_under_get_rule(mem::transmute(elem));
+                sum += number.to_i64().unwrap()
+            }
+
+            assert!(sum == 15);
+
+            for elem in arr.iter() {
+                let number: CFNumber = TCFType::wrap_under_get_rule(mem::transmute(elem));
+                sum += number.to_i64().unwrap()
+            }
+
+            assert!(sum == 30);
         }
-
-        assert!(sum == 15);
-
-        for elem in arr.iter() {
-            let number: CFNumber = TCFType::wrap_under_get_rule(mem::transmute(elem));
-            sum += number.to_i64().unwrap()
-        }
-
-        assert!(sum == 30);
     }
 }

core-foundation/src/base.rs

@@ -178,10 +178,20 @@ impl TCFType<CFTypeRef> for CFType {
         // FIXME(pcwalton): Is this right?
         0
     }
+}
 
-    #[inline]
-    fn instance_of<OtherConcreteTypeRef,OtherCFType:TCFType<OtherConcreteTypeRef>>(&self) -> bool {
-        // Since this is the root of the type hierarchy, we always answer yes.
-        true
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use boolean::CFBoolean;
+
+    #[test]
+    fn cftype_instance_of() {
+        let string = CFString::from_static_string("foo");
+        let cftype = string.as_CFType();
+
+        assert!(cftype.instance_of::<_, CFString>());
+        assert!(!cftype.instance_of::<_, CFBoolean>());
     }
 }