-
Notifications
You must be signed in to change notification settings - Fork 230
Reference type as associated value instead of generic #132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
core-foundation/src/dictionary.rs
Outdated
self.contains_key(key.as_concrete_TypeRef() as *const c_void) | ||
pub fn contains_key2<K: TCFType>(&self, key: &K) -> bool { | ||
let type_ref_ptr = &key.as_concrete_TypeRef() as *const _ as *const *const c_void; | ||
self.contains_key(unsafe { *type_ref_ptr }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation had to pay with a bit complexity for the reduced API complexity.
Basically, now I don't know that K::TypeRef
is a *const _
any longer, so I can't just cast the pointer directly. Instead I have to take the pointer to the pointer and then cast that and lastly dereference.
I added a unit test to make sure I got it right.
core-foundation/src/propertylist.rs
Outdated
let ptr = &self.0 as *const _ as *const <T as TCFType>::TypeRef; | ||
unsafe { | ||
let reference: &<T as TCFType>::TypeRef = &*ptr; | ||
Some(T::wrap_under_get_rule(mem::transmute_copy(reference))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation became uglier in order to get rid of the Raw
generic. Using mem::transmute_copy
is not very recommended maybe, but I hope this is used correctly. Here reference
is of type &CFFooRef
, which after the transmute_copy
should be a CFFooRef
. So the transmute_copy
don't change the type, it just helps me take ownership of a dereferenced raw pointer. Which should be safe here unless I'm mistaken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a more elegant solution is possible? I would be eager to learn a better way if there is one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is of course depending on the assumption that all TCFType::TypeRef
s have the same size as *const c_void
. This is true for all the possible subclasses of CFPropertyList
at the moment and probably forever, but nothing is enforcing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I add a second associated type, TCFType::RawTypeRef
(or similar name), that we set to the corresponding __CFFoo
type. Then I can refer to *const Self::RawTypeRef
in places and do proper casts. From some early tinkering around it looks like that can get rid of all these strange solutions.
assert!(propertylist.downcast::<_, CFString>().unwrap().to_string() == "Bar"); | ||
assert!(propertylist.downcast::<_, CFBoolean>().is_none()); | ||
assert!(propertylist.downcast::<CFString>().unwrap().to_string() == "Bar"); | ||
assert!(propertylist.downcast::<CFBoolean>().is_none()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here one can see one of the clear benefits of this PR.
☔ The latest upstream changes (presumably #127) made this pull request unmergeable. Please resolve the merge conflicts. |
ee441c5
to
1377b50
Compare
Now I'm much more satisfied with the solution to this. By adding a trait to the sys crate that provide helper functions for const raw pointers (all CFref-types) I'm now able to know more about |
I have investigated a little bit how much this change would break published crates depending on the 0.4 version of No change required:
Some change required, but very little:
|
@@ -15,7 +15,7 @@ use string::CFStringRef; | |||
#[repr(C)] | |||
pub struct __CFError(c_void); | |||
|
|||
pub type CFErrorRef = *mut __CFError; | |||
pub type CFErrorRef = *const __CFError; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this was *mut
for a reason I have missed. But it was the only CF-ref typedef which was not *const
so my guess is it was a typo. All tests of this crate and all dependant crates I tried work fine after this change.
ca4834f
to
93c9cba
Compare
☔ The latest upstream changes (presumably #139) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice improvement!
It is easy but very fastidious, so I'll not let this PR land without all the others being ready too, including a PR to update them in Servo. |
93c9cba
to
7a26e39
Compare
@nox If it's released as |
The issue arises when we start needing things that are unrelated to your changes, and 0.5.0 didn't land yet in servo/servo, so we must continue to maintain a 0.4 branch to get the unrelated changes in tree. Of course, we should give you a hand and not tell you to do all the bumps. |
@nox I'll be happy to help upgrade the dependencies! But I'm not sure which they are. All the ones I can see are the ones listed under https://crates.io/crates/core-foundation/reverse_dependencies But I suspect that is not all of it. If you list them here I can start submitting PRs to them. Given that you want this feature that is. |
Looking at the 0.2 to 0.3 bump commit gives an idea what to do. |
@nox @jdm What are your thoughts on the name How about just calling it If not, how about getting rid of "Concrete" and calling it |
7a26e39
to
c7d610d
Compare
@jrmuizel I didn't have time to help update servo and all dependencies yet. I still have some hope I will get to that though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like these changes. Sorry that they sat around for so long!
core-foundation/src/base.rs
Outdated
fn instance_of<OtherConcreteTypeRef,OtherCFType:TCFType<OtherConcreteTypeRef>>(&self) -> bool { | ||
self.type_of() == <OtherCFType as TCFType<_>>::type_id() | ||
fn instance_of<OtherCFType: TCFType>(&self) -> bool { | ||
self.type_of() == <OtherCFType as TCFType>::type_id() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to use OtherCFType::type_id()
now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
core-foundation/src/propertylist.rs
Outdated
) -> bool { | ||
self.type_of() == <OtherCFType as TCFType<_>>::type_id() | ||
pub fn instance_of<OtherCFType: TCFType>(&self) -> bool { | ||
self.type_of() == <OtherCFType as TCFType>::type_id() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to do OtherCFType::type_id()
now.
core-foundation/src/propertylist.rs
Outdated
pub fn downcast<T: CFPropertyListSubClass>(&self) -> Option<T> { | ||
if self.instance_of::<T>() { | ||
unsafe { | ||
let subclass_ref = <T as TCFType>::Ref::from_void_ptr(self.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to do T::Ref::from_void_ptr(self.0)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -80,6 +80,23 @@ pub struct CFAllocatorContext { | |||
pub preferredSize: CFAllocatorPreferredSizeCallBack | |||
} | |||
|
|||
/// Trait for all types which are Core Foundation reference types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this commit should be reordered to come before 3688803.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdm My macOS dev environment was borked, so I could not build Servo. Thus I never came around to upgrade the dependencies from Servo down to here. Would be awesome to get it in though, not sure how to fix that dev env really... |
The travis build fails with a serious crash. It might be some bug I introduced, but it might also be something with Travis, since I can't reproduce it locally. Before I dive into this problem, does anyone know if this has show up before and/or is a known problem with this crate, or if it's something I likely introduced? @jdm ? |
@@ -92,11 +92,6 @@ impl<T> CFArray<T> { | |||
} | |||
} | |||
|
|||
#[deprecated(note = "please use `as_untyped` instead")] | |||
pub fn to_untyped(self) -> CFArray { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize now that my previous move to deprecate to_untyped
and add as_untyped
was not exactly making things more correct than before, if one should follow the API guidelines to the letter.
This removed to_untyped
is a consuming convert (owned -> owned (non-Copy types)
), so it should have been named into_untyped
with that signature. So the old name was slightly "wrong".
On the other hand, the new as_untyped
only borrows &self
, but gives out an owned object (borrowed -> owned (non-Copy types)
), so it should be named to_untyped
, and not as_
since that is for conversions borrowed -> borrowed
.
Do you think I should change the name of the method that will be present in the 0.5.0
release? Or is that too much nitpicking and too much changing the API back and forth? If it should be changed this would be the correct signature:
pub fn to_untyped(&self) -> CFArray { ... }
In line with all the new downcast_into
methods, we could also add a consuming one that avoids bumping the retain count:
pub fn into_untyped(self) -> CFArray { ... }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those seem like worthwhile changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check. I'll add a commit for that. I'll push it after my other PR is merged, as this one will need rebasing/manual conflict resolution after that one anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
🔒 Merge conflict |
☔ The latest upstream changes (presumably #145) made this pull request unmergeable. Please resolve the merge conflicts. |
63a43bb
to
3fbdf42
Compare
3fbdf42
to
7e87a9d
Compare
@bors-servo r+ |
📌 Commit 7e87a9d has been approved by |
Reference type as associated value instead of generic Each `CFFoo` has exactly one `CFFooRef` and which one is defined by `CFFoo`, not by its user. Thus the type of reference can be an associated type instead of a generic. As you can see this greatly simplifies things such as calling `instance_of` and `from_CFType_pairs` as well as using `CFPropertyListSubClass`. It basically removes one generic parameter from any method dealing with `TCFType`s in some way. This is a breaking change, so it would probably mean a `0.5.0` release if accepted. I don't know what your stance on breaking changes or version bumps is, so I don't know if you want this or not. But it would simplify the API, both here, but first and foremost in higher level crates using this. <!-- 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/132) <!-- Reviewable:end -->
☀️ Test successful - status-travis |
Finally! \o/ |
Upgrade core foundation This is a PR in a series of PRs originating at servo/core-foundation-rs#132 The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly. TODO before merge: - [x] Merge `core-foundation` PR and publish. - [x] Remove the last commit from this PR, so we depend on `core-foundation` from crates.io. <!-- 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-graphics-rs/110) <!-- Reviewable:end -->
Upgrade core foundation This is a PR in a series of PRs originating at servo/core-foundation-rs#132 The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly TODO before merge: - [x] Merge `core-foundation` PR and publish. - [x] Merge `core-graphics` PR and publish. - [x] Remove the last commit from this PR, so we depend on `core-foundation` from crates.io. <!-- 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-text-rs/75) <!-- Reviewable:end -->
Upgrade core foundation This is a PR in a series of PRs originating at servo/core-foundation-rs#132 The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly. TODO before merge: - [x] Merge `core-graphics` PR and publish. - [x] Remove the last commit from this PR, so we depend on `core-foundation` from crates.io. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/cocoa-rs/181) <!-- Reviewable:end -->
Upgrade core foundation This is a PR in a series of PRs originating at servo/core-foundation-rs#132 The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly. TODO before merge: - [x] Merge `core-foundation` PR and publish. - [x] Remove the last commit from this PR, so we depend on code from crates.io. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/io-surface-rs/60) <!-- Reviewable:end -->
Upgrade core foundation This is a PR in a series of PRs originating at servo/core-foundation-rs#132 The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly. TODO before merge: - [x] Merge `core-foundation`, `core-graphics` and `cocoa` PRs and publish. - [x] Remove the last commit from this PR, so we depend on code from crates.io. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/glutin/142) <!-- Reviewable:end -->
Upgrade core foundation This is a PR in a series of PRs originating at servo/core-foundation-rs#132 The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. Before the merge/publish, a set of PRs making sure the entire dependency graph of Servo is ready for this change will be created. TODO before merge of this PR: - [x] Merge `io-surface` and `servo-glutin` PRs and publish. - [x] Remove the last commit from this PR, so we depend on code from crates.io. I did not read the contributing guidelines for this repo, as the link to it did not work for me. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/skia/148) <!-- Reviewable:end -->
Upgrade core foundation This is a PR in a series of PRs originating at servo/core-foundation-rs#132 The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. Hopefully we can manage to bring Servo and its entire dependency tree up to date as rapidly as possible in combination with this, as to use only the new core-foundation in Servo and all its deps. :) TODO before merge: - [x] Merge `skia`. - [x] Remove the last commit from this PR, so we depend on code from crates.io. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-azure/282) <!-- Reviewable:end -->
Upgrade core foundation This is a PR in a series of PRs originating at servo/core-foundation-rs#132 The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. Hopefully we can manage to bring Servo and its entire dependency tree up to date as rapidly as possible in combination with this, as to use only the new core-foundation in Servo and all its deps. :) TODO before merge: - [x] Merge `core-foundation`, `core-graphics`, `core-text`, `servo-glutin` PRs and publish. - [x] Merge `font-loader` PR and publish. - [x] Remove the last commit from this PR, so we depend on code from crates.io. - [x] Update Cargo.lock again, to not contain urls to temporary feature branhces. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2299) <!-- Reviewable:end -->
Upgrade core foundation <!-- Please describe your changes on the following line: --> This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132 Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time. This PR is **not ready for merge** in its current state. The following must be done first: - [x] Merge servo/core-foundation-rs#132 and publish. - [x] Merge servo/core-graphics-rs#110 and publish. - [x] Merge servo/core-text-rs#75 and publish. - [x] Merge servo/cocoa-rs#181 and publish. - [x] Merge servo/glutin#142 and publish. - [x] Merge servo/io-surface-rs#60 and publish. - [x] Merge servo/skia#148. - [x] Merge servo/rust-azure#282. - [x] Merge servo/webrender#2299. - [x] Merge servo/surfman#118 and publish. - [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`. - [x] Update Cargo.lock again to not point to my feature branches. For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something. Ping @jdm and @nox who have been the ones commenting on the initial PR. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors <!-- Either: --> - [X] These changes do not require tests because they don't change any code, just upgrade dependencies. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19759) <!-- Reviewable:end -->
…e-foundation); r=jdm <!-- Please describe your changes on the following line: --> This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132 Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time. This PR is **not ready for merge** in its current state. The following must be done first: - [x] Merge servo/core-foundation-rs#132 and publish. - [x] Merge servo/core-graphics-rs#110 and publish. - [x] Merge servo/core-text-rs#75 and publish. - [x] Merge servo/cocoa-rs#181 and publish. - [x] Merge servo/glutin#142 and publish. - [x] Merge servo/io-surface-rs#60 and publish. - [x] Merge servo/skia#148. - [x] Merge servo/rust-azure#282. - [x] Merge servo/webrender#2299. - [x] Merge servo/surfman#118 and publish. - [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`. - [x] Update Cargo.lock again to not point to my feature branches. For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something. Ping @jdm and @nox who have been the ones commenting on the initial PR. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors <!-- Either: --> - [X] These changes do not require tests because they don't change any code, just upgrade dependencies. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: e94a25949c924e086e38ef6bdbdc935734415b26 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 4e0c40d1610e09d5ecf823f51d42ede8706751e3
…e-foundation); r=jdm <!-- Please describe your changes on the following line: --> This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132 Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time. This PR is **not ready for merge** in its current state. The following must be done first: - [x] Merge servo/core-foundation-rs#132 and publish. - [x] Merge servo/core-graphics-rs#110 and publish. - [x] Merge servo/core-text-rs#75 and publish. - [x] Merge servo/cocoa-rs#181 and publish. - [x] Merge servo/glutin#142 and publish. - [x] Merge servo/io-surface-rs#60 and publish. - [x] Merge servo/skia#148. - [x] Merge servo/rust-azure#282. - [x] Merge servo/webrender#2299. - [x] Merge servo/surfman#118 and publish. - [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`. - [x] Update Cargo.lock again to not point to my feature branches. For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something. Ping jdm and nox who have been the ones commenting on the initial PR. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors <!-- Either: --> - [X] These changes do not require tests because they don't change any code, just upgrade dependencies. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: e94a25949c924e086e38ef6bdbdc935734415b26 UltraBlame original commit: 931c61b9f7a889329ab73d00414483af47ac3f7e
…e-foundation); r=jdm <!-- Please describe your changes on the following line: --> This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132 Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time. This PR is **not ready for merge** in its current state. The following must be done first: - [x] Merge servo/core-foundation-rs#132 and publish. - [x] Merge servo/core-graphics-rs#110 and publish. - [x] Merge servo/core-text-rs#75 and publish. - [x] Merge servo/cocoa-rs#181 and publish. - [x] Merge servo/glutin#142 and publish. - [x] Merge servo/io-surface-rs#60 and publish. - [x] Merge servo/skia#148. - [x] Merge servo/rust-azure#282. - [x] Merge servo/webrender#2299. - [x] Merge servo/surfman#118 and publish. - [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`. - [x] Update Cargo.lock again to not point to my feature branches. For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something. Ping jdm and nox who have been the ones commenting on the initial PR. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors <!-- Either: --> - [X] These changes do not require tests because they don't change any code, just upgrade dependencies. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: e94a25949c924e086e38ef6bdbdc935734415b26 UltraBlame original commit: 931c61b9f7a889329ab73d00414483af47ac3f7e
…e-foundation); r=jdm <!-- Please describe your changes on the following line: --> This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132 Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time. This PR is **not ready for merge** in its current state. The following must be done first: - [x] Merge servo/core-foundation-rs#132 and publish. - [x] Merge servo/core-graphics-rs#110 and publish. - [x] Merge servo/core-text-rs#75 and publish. - [x] Merge servo/cocoa-rs#181 and publish. - [x] Merge servo/glutin#142 and publish. - [x] Merge servo/io-surface-rs#60 and publish. - [x] Merge servo/skia#148. - [x] Merge servo/rust-azure#282. - [x] Merge servo/webrender#2299. - [x] Merge servo/surfman#118 and publish. - [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`. - [x] Update Cargo.lock again to not point to my feature branches. For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something. Ping jdm and nox who have been the ones commenting on the initial PR. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors <!-- Either: --> - [X] These changes do not require tests because they don't change any code, just upgrade dependencies. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: e94a25949c924e086e38ef6bdbdc935734415b26 UltraBlame original commit: 931c61b9f7a889329ab73d00414483af47ac3f7e
Each
CFFoo
has exactly oneCFFooRef
and which one is defined byCFFoo
, not by its user. Thus the type of reference can be an associated type instead of a generic.As you can see this greatly simplifies things such as calling
instance_of
andfrom_CFType_pairs
as well as usingCFPropertyListSubClass
. It basically removes one generic parameter from any method dealing withTCFType
s in some way.This is a breaking change, so it would probably mean a
0.5.0
release if accepted. I don't know what your stance on breaking changes or version bumps is, so I don't know if you want this or not. But it would simplify the API, both here, but first and foremost in higher level crates using this.This change is