Skip to content
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

Downcast for just Send? #14

Open
TheNeikos opened this issue Aug 30, 2023 · 4 comments · May be fixed by #16
Open

Downcast for just Send? #14

TheNeikos opened this issue Aug 30, 2023 · 4 comments · May be fixed by #16

Comments

@TheNeikos
Copy link
Contributor

Currently one loses the Send bound when using into_any for a Box<dyn Foo> where Foo: Send.

I understand that this might complicate the macro though...

@marcianx
Copy link
Owner

I'm curious as to your use-case. I imagine that this doesn't prevent you from downcasting to a concrete type that implements Send. Is the need that you just want to use into_any for the purpose of storing Box<dyn Any + Send>?


In the meantime, depending on your need, above, it seems that this just requires adding an intermediate:

pub trait DowncastSend : Downcast + Send {
    fn into_any_send(self: Box<Self>) -> Box<dyn Any + Send>;
}

impl<T: Any + Send> Downcast for T {
    fn into_any_send(self: Box<Self>) -> Box<dyn Any + Send> { self }
}

and possibly expanding:

pub trait DowncastSync: DowncastSend + Sync {
    fn into_any_sync(self: Box<Self>) -> Box<dyn Any + Send + Sync>;
    ...
}

so that DowncastSync can also inherit the functionality and also support Sync + Sync. I'm not sure how risky the latter would be in terms of causing breakages since it would be adding more methods and a new base trait to DowncastSync. As long as nobody is implementing it manually, it should be fine, but I might have to make a major version bump to be safe.

There doesn't seem to be a need to add a corresponding downcast_send() since, as I noted in my question above, the absence of this functionality doesn't seem to affect downcasting, am I right?

@TheNeikos
Copy link
Contributor Author

I can't link to it as it is not OSS, but I basically have some types that I may move around and then call with &mut so they don't have to be Sync, but definitely Send.

I did end up adding exactly what you wrote but thought that such an inclusion could be worthwhile for the library (and would finish the trifecta of T, T + Send, T + Send + Sync :D)

wrt to downcast: It is implemented for Box<Any {,{+ Send,{,+ Sync}}>, and I am using it, so it should be fine: https://doc.rust-lang.org/std/any/trait.Any.html#impl-Box%3Cdyn+Any+%2B+Send,+A%3E

I am not sure it is necessary to have an intermediary. While it is true that DowncastSync implies DowncastSend, they can be added as needed to the supertraits of the traits one wants to downcast from. It is certainly more elegant though, but that major version bump is always a bit meh.

You could try adding it and running it over cargo-public-api to see what it has to say!

@marcianx
Copy link
Owner

I am not sure it is necessary to have an intermediary. While it is true that DowncastSync implies DowncastSend, they can be added as needed to the supertraits of the traits one wants to downcast from.

Can you help me understand what you're suggesting? Where were you suggesting I should add into_any_send without the intermediary DowncastSend? Into DowncastSync itself? So then we would only support the Send conversion for Send + Sync types? Or did you mean something else?

@TheNeikos
Copy link
Contributor Author

So I was thinking you could just add a trait DowncastSend: Downcast + Send and if people only need Any + Send they add that, and if they also want Sync they add the usual DowncastSync (which already implies Send + Sync).

Lemme make a PR

@TheNeikos TheNeikos linked a pull request Aug 31, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants