Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd CFPropertyList and methods for casting to subclasses #131
Conversation
|
This looks sensible to me! |
| /// // Cast it down again. | ||
| /// assert!(propertylist.downcast::<_, CFString>().unwrap().to_string() == "FooBar"); | ||
| /// ``` | ||
| pub fn downcast<Raw, T: CFPropertyListSubClass<Raw>>(&self) -> Result<T, ()> { |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| @@ -56,8 +58,142 @@ pub fn create_data(property_list: *const c_void, format: CFPropertyListFormat) - | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| /// Trait for all subclasses of CFPropertyList. | |||
| pub trait CFPropertyListSubClass<Raw>: TCFType<*const Raw> { | |||
This comment has been minimized.
This comment has been minimized.
jdm
Nov 24, 2017
Member
We should be able to do pub trait CFPropertyListSubClass: TCFType<*const Self> { and avoid some redundancy in the implementations.
This comment has been minimized.
This comment has been minimized.
faern
Nov 24, 2017
Author
Contributor
Getting rid of the generic would be awesome. But I don't understand exactly how that would work. Self here is CFString or similar.
This comment has been minimized.
This comment has been minimized.
faern
Nov 24, 2017
Author
Contributor
Separate from this I'm working on trying to make the ConcreteTypeRef in TCFType a associated type instead of a generic. When/if that works out this can be greatly simplified.
This comment has been minimized.
This comment has been minimized.
jdm
Nov 24, 2017
Member
impl CFPropertyListSubClass<::data::__CFData> for ::data::CFData duplicates the type in two places. Using the snippet I provided, the resulting impl of impl CFPropertyListSubClass for ::data::CFData should work just fine: https://play.rust-lang.org/?gist=b89fc9fe403069fd843f6054897f21db&version=stable
This comment has been minimized.
This comment has been minimized.
faern
Nov 24, 2017
Author
Contributor
Yes, but impl U<*const u32> for u32 {} does not hold for us.
Our CFString does not impl TCFType<*const CFString>, it implements it for TCFType<*const __CFString>.
This comment has been minimized.
This comment has been minimized.
faern
Nov 24, 2017
Author
Contributor
The existance of both CFStringRef and __CFString makes it complicated in more than one situation for me. I can't cast between pointers in the downcast method because the compiler does not know that CFStringRef is a raw pointer. It is, and all *Ref are *const _, but the compiler does not know. Which forces me to reference the underlying *const __CF<Foo> types in order to show the compiler the casts are fine. Maybe it's possible to work around it in some way, but I have not found one yet.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
faern
Nov 24, 2017
Author
Contributor
Easy thing to do. Lots of types with very similar names here :)
|
@bors-servo: r+ |
|
|
Add CFPropertyList and methods for casting to subclasses https://developer.apple.com/documentation/corefoundation/cfpropertylist-rif specifies the following types as valid subclasess of a property list: CFData, CFString, CFArray, CFDictionary, CFDate, CFBoolean, and CFNumber. So I add a trait, `CFPropertyListSubClass`, and implement it for the valid types. Then add the `CFPropertyList` type and make it work as much as possible as the other `TCFType`s. However, it can't implement that trait, since it can't implement the static method `type_id`. The main point of adding this was to then get to the convenience methods `to_CFPropertyList` and `downcast` that allows safe casting between the subclasses and the superclass. This is handy in calls such as `SCDynamicStoreSetValue` and `SCDynamicStoreCopyValue` where the methods either accept or return a `CFPropertyListRef`. With this addition it becomes easy to write safe wrappers around those functions and make them accept proper, constrained types. Disclaimer: I'm not yet super familiar with CF. So I can only hope this is safe and sound, but I think it is. <!-- 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/131) <!-- Reviewable:end -->
|
|
faern commentedNov 24, 2017
•
edited
https://developer.apple.com/documentation/corefoundation/cfpropertylist-rif specifies the following types as valid subclasess of a property list:
CFData, CFString, CFArray, CFDictionary, CFDate, CFBoolean, and CFNumber.
So I add a trait,
CFPropertyListSubClass, and implement it for the valid types. Then add theCFPropertyListtype and make it work as much as possible as the otherTCFTypes. However, it can't implement that trait, since it can't implement the static methodtype_id.The main point of adding this was to then get to the convenience methods
to_CFPropertyListanddowncastthat allows safe casting between the subclasses and the superclass. This is handy in calls such asSCDynamicStoreSetValueandSCDynamicStoreCopyValuewhere the methods either accept or return aCFPropertyListRef. With this addition it becomes easy to write safe wrappers around those functions and make them accept proper, constrained types.Disclaimer: I'm not yet super familiar with CF. So I can only hope this is safe and sound, but I think it is.
This change is