Remove Type::Error
#1590
Remove Type::Error
#1590
Conversation
|
Chatting a little with Ben, we feel that if we want to take this at all, taking it before 0.24 is probably the lest painful thing for people maintaining external bindings, and my intention is to put a PR up for at least go and the external kotlin ones which addresses almost all of the pain we put them through. I believe that the next patch (allowing interfaces as errors) will be far less intrusive than this - at least until they actually want to support that capability. So putting it up for review now. |
| @@ -451,8 +458,25 @@ pub mod filters { | |||
| Ok(oracle().enum_variant_name(nm)) | |||
| } | |||
|
|
|||
| /// Some of the above filters have different versions to help when the type | |||
| /// is used as an error. | |||
| pub fn error_type_name(as_type: &impl AsType) -> Result<String, askama::Error> { | |||
There was a problem hiding this comment.
This is about the only part of this that I don't think is great! It's OK, but not great, The fact all the filters now use AsType means I can't magically turn an enum into "something else type-ish". This was actually quite a bit cleaner when it was a CodeType or CodeTypeDispatch, but removing CodeTypeDispatch was such a big win everywhere else that I think we made the correct tradeoff.
There was a problem hiding this comment.
I don't really mind this, but one option would be to keep the Type::Error variant, but generalize it as Type::Error(Box<Type>). I'm not sure if that's a net positive or not, but it would remove the need for these extra filter functions.
There was a problem hiding this comment.
I think that would be a bit of a negative - it would still be a level of abstraction that hurts almost everywhere else IMO, and I think it would mean we'd need to force that level of abstraction - eg, we'd need to insist that, the function like fn throws() -> Option<Type> always was forced to return Type::Error again, which I think isn't ideal. To put it another way - Type::Error is a concept only the bindings need to care about, so having it as a first-class type in bindgen seems wrong.
TBH, I think the above really points more to a weakness in the CodeOracle setup than in uniffi_bindgen::interface, and that while AsType is great as the glue for CodeType, it might not be the correct thing to actually expose in the filters.
End goal is to allow other types to be used as errors, but I figured the first and hopefully least controversial step would be to remove
Type::Errorentirely.Still needs some thought re the bindings, but I think this is getting into good shape.