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

Make some of the Content api public #741

Closed
dtolnay opened this issue Feb 3, 2017 · 8 comments
Closed

Make some of the Content api public #741

dtolnay opened this issue Feb 3, 2017 · 8 comments

Comments

@dtolnay
Copy link
Member

dtolnay commented Feb 3, 2017

Follow-up to #739. After we get a chance to iterate on the implementation of the untagged and tag attributes, we can start to polish and expose pieces of the implementation because they are definitely useful in other use cases.

@oli-obk
Copy link
Member

oli-obk commented Feb 3, 2017

We can get rid of the E parameter: 6277079

@dtolnay
Copy link
Member Author

dtolnay commented Feb 18, 2017

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.

@dtolnay
Copy link
Member Author

dtolnay commented Feb 25, 2017

One nuance here is that it is helpful to have slightly different Content enums when serializing vs when deserializing.

@dtolnay
Copy link
Member Author

dtolnay commented Aug 29, 2017

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.

@dtolnay dtolnay closed this as completed Aug 29, 2017
@slinkydeveloper
Copy link

slinkydeveloper commented Mar 26, 2020

Hi, any updates on this? I can help with this issue

emk added a commit to faradayio/openapi-interfaces that referenced this issue Sep 23, 2021
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
littledivy pushed a commit to denoland/deno that referenced this issue Feb 15, 2023
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
@Sytten
Copy link

Sytten commented Oct 24, 2024

@dtolnay Has the stance changed on that? The currently private Content and ContentRefDeserializer are very useful. serde-value is not really a replacement since the deserializer takes an owned Value not a ref.
Is it something you are still looking to make public?

@dtolnay
Copy link
Member Author

dtolnay commented Oct 24, 2024

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).

@Sytten
Copy link

Sytten commented Oct 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants