These transmutes were going from one type of pointer to another,
and so can be done as casts instead.
Doing so also means that one unsafe block can be removed.
This was found via clippy.
BUG: Add test showing how downcasting to CFArray is unsound
This PR is not intended to me merged directly. It adds a test that causes undefined behavior without using `unsafe {}`. So it's more to show the problem and discuss a solution.
The problem here is that if you have a `CFArray<X>` you can cast it up to a `CFType` and then down to a `CFArray<Y>` for any `Y: TCFType`. It never checks that the elements in the array are of type `Y` or not. At this point `CFArray::iter` will happily give out `&Y` references, while in fact the pointers backing those objects are pointing to `XRef` instances. So doing anything with the `&Y` will likely cause invalid memory access.
How do we solve this?
Ping @jrmuizel
<!-- 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/158)
<!-- Reviewable:end -->
Using some more macro magic we can avoid having to duplicate
our impl macro implementation for generic types. This will help
when we want a version for CFDictionary with two type parameters.
Reduce usage of libc crate.
Switch from using `libc` types to `::std::os::raw`. This is a breaking
change due to the differences in how `c_void` is defined.
`core-foundation-sys` no longer needs the `libc` dependency.
<!-- 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/156)
<!-- Reviewable:end -->
Switch from using `libc` types to `::std::os::raw`. This is a breaking
change due to the differences in how `c_void` is defined.
`core-foundation-sys` no longer needs the `libc` dependency.
This changes get() to return an Option<> and adds a get_unchecked() that doesn't do a bounds check.
This means our bounds check can be inlined into the caller instead of us having to CFArrayGetCount twice
for each element.
Add consuming cheap cast
There are a lot of places where we increment and decrement the retain count when we would not have to. For example, a very common use-case with PropertyLists is to just directly cast them to their correct sub type and then throw the PropertyList away (because there is nothing you can do with the PropertyList instance directly.)
The reason we do this, I guess, is mostly because all `Drop` implementations force a retain count decrement. So we have to keep incrementing it to keep it correct. But if we add a flag to types that can be used to disable the decrement, then we can also get rid of the increment when we virtually just want to cast a pointer between two types.
This PR implements this by adding a boolean flag to all CF structs that is used by the `Drop` impl to determine if it should `CFRelease` the instance or not. Then this flag is modified in appropriate places where we cast to other types and consume ourselves.
I added some benchmarks locally. I did not want to commit them since benchmarking only works on nighty, and I did not feel like adding a feature flag for it etc. Anyhow, the following benchmarks:
```rust
#[bench]
fn bench_before(b: &mut Bencher) {
let string = CFString::from_static_string("Bar");
b.iter(|| unsafe {
string.clone()
.to_CFPropertyList()
.downcast::<_, CFString>()
.unwrap()
.disable_release();
})
}
#[bench]
fn bench_after(b: &mut Bencher) {
let string = CFString::from_static_string("Bar");
b.iter(|| unsafe {
string.clone()
.into_CFPropertyList()
.downcast_into::<_, CFString>()
.unwrap()
.disable_release();
})
}
```
Given the following results:
```
test propertylist::test::bench_after ... bench: 16 ns/iter (+/- 3)
test propertylist::test::bench_before ... bench: 73 ns/iter (+/- 14)
```
The final call to `disable_release()` in the benchmarks are to prevent the type coming out of the `unwrap()` from calling `CFRelease` on drop, and thus avoid measuring that. I want to try to measure only `{to,into}_CFPropertyList`, `downcast` and `downcast_into`. The `clone` is needed because the `into` version requires it and it would then not be fair to not have it on both.
I would say that speeding it up by a factor of ~4 can be well worth it? (Well, that 4 is of course no good measurement of anything, microbenchmarks etc. etc.. The benchmark mostly shows that a lot can be gained from avoiding some retain counting.)
<!-- 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/142)
<!-- Reviewable:end -->
Add CFMutableDictionary type.
Added CFMutableDictionary that presents the mutable dictionary
functions of CF. Tries to present a similar interface as CFDictionary.
Fixes issue #93.
<!-- 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/135)
<!-- Reviewable:end -->
Patrick Walton
