-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
Make some of the Content api public #741
Comments
We can get rid of the |
I think the remaining tasks before we can make this public are:
Not all of these require obvious breaking changes to Content but I wouldn't want to limit our solution to any of these by committing to a flawed API. |
One nuance here is that it is helpful to have slightly different Content enums when serializing vs when deserializing. |
Let's continue to track this but from the opposite direction -- I filed arcnmx/serde-value#16 to absorb serde-value into the public API of Serde when they are ready for it. I think it will be more productive to polish and iterate on serde-value publicly than to try to get our own implementation right on the first try. |
Hi, any updates on this? I can help with this issue |
serde and serde_yaml generate terrible error messages for untagged enums. We can fix this with manual deserialization. Or at least improve it. serde-rs/serde#773 serde-rs/serde#741 dtolnay/serde-yaml#201
Fixes #16922. The error messages in the `ffi` module are somewhat cryptic when passing functions that have invalid `parameters` or `result` type strings. While the generated serializer for the `ForeignFunction` struct correctly outputs a correct and verbose message, the user sees a far less helpful `data did not match any variant` message instead. The underlying cause appears to be the fallback message in the auto-derived deserializer for untagged enums [1] generated as a result of `ForeignSymbol` being marked as `#[serde(untagged)]` [2]. Passing an unexpected value for `NativeType` causes it to error out while attempting to deserialize both enum variants -- once because it's not a match for the `ForeignStatic` variant, and once because the `ForeignFunction` deserializer rejects the invalid type for the parameters/return type. This is currently open as [serde #773](serde-rs/serde#773), and not a trivial exercise to fix generically. [1] https://github.com/serde-rs/serde/blob/v0.9.7/serde_derive/src/de.rs#L730 [2] https://github.com/denoland/deno/blob/main/ext/ffi/dlfcn.rs#L102 [3] serde-rs/serde#773 Note that the auto-generated deserializer for untagged enums uses a private API to buffer deserializer content that we don't have access to. Instead, we can make use of the `serde_value` crate to buffer the values. This can likely be removed once the official buffering API lands (see [4] and [5]). In addition, this crate pulls in `serde_json` as a cheap way to test that the deserializer works properly. [4] serde-rs/serde#741 [5] serde-rs/serde#2348
@dtolnay Has the stance changed on that? The currently private |
The fact that no one has made a better serde-value crate is some indication that the use case is not that important. Do you have insight into why that hasn't happened? Regarding my stance, no change since #741 (comment). |
Its really only useful for untagged enums so it is probably niche but when you hit the problem there is not good solution except using the private api which I have seen used around since its not really private and people are lazy. |
Follow-up to #739. After we get a chance to iterate on the implementation of the
untagged
andtag
attributes, we can start to polish and expose pieces of the implementation because they are definitely useful in other use cases.The text was updated successfully, but these errors were encountered: