Skip to content

Make untagged enums work#253

Closed
joonazan wants to merge 2 commits into
ron-rs:masterfrom
joonazan:master
Closed

Make untagged enums work#253
joonazan wants to merge 2 commits into
ron-rs:masterfrom
joonazan:master

Conversation

@joonazan

@joonazan joonazan commented Jul 6, 2020

Copy link
Copy Markdown

When deserializing an untagged enum, serde uses deserialize_any. There is ambiguity, as we cannot know if a struct or an enum is expected. That ambiguity is resolved by serde if we just give it a JSON-like data structure.

When we encounter an lone identifier, it becomes a string. An identifier with parens after it becomes a singleton map.

Solves #217

Newtype pain

Id(Val) would be {"Id": "Val"} in JSON, but RON produces a one-element tuple instead. The only way to identify that case that I could think of is checking for commas, which unfortunately makes deserializing somewhat quadratic. It could be linear if this was done as a preprocessing pass.

Is there a way to first read a tuple but then change it if it only has one element?

Failing test case

One of the tests (

fn test_complex() {
) now fails because it doesn't expect a map. I think it should expect a map, but I don't understand that test's purpose very well, so maybe someone else can comment on this?

@kvark kvark requested a review from torkleyy July 7, 2020 03:50

@kvark kvark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this addition!
I'd love to have @torkleyy reviewing this. If not, I'll try to take another pass.
Having a hard time wrapping my head around serde stuff when I'm out of context for a while

Comment thread src/de/mod.rs Outdated
@halvnykterist

Copy link
Copy Markdown

I came upon this issue and tried using the PR branch in my code. It seems to work as intended, but there are two issues.

  • Line/Column information seems to be missing on the errrors.
  • In the case of errors in self-referential enums, a stack overflow (!!) occurs. I've got a minimal example below:
use serde::Deserialize;
use ron;

#[derive(Deserialize)]
#[serde(untagged)]
enum Number {
    Number(i32),
    SelfReferencing(Box<Number>),
}

fn main() {
    let _: Number = ron::de::from_str("This causes a stack overflow").unwrap();
}

@kvark

kvark commented Dec 3, 2020

Copy link
Copy Markdown
Collaborator

@torkleyy could you have a look, please?

@joonazan

joonazan commented Dec 4, 2020

Copy link
Copy Markdown
Author

@halvnykterist This PR should not be merged. Instead, a new version should be written that implements this in a cleaner way.

Maybe there is some way to use serde that I couldn't find. My first post describes the problems I faced.

If not, the best solution would be to feed the input characters into a state machine as they are read in. The current implementation checks the same characters multiple times.

The stack overflow is not ideal but does make sense. After all, there could be an infinite number of nested SelfReferencing I don't think it is related to this patch.

@github-actions

Copy link
Copy Markdown

Issue has had no activity in the last 60 days and is going to be closed in 7 days if no further activity occurs

@github-actions github-actions Bot added the stale label Nov 18, 2021
@github-actions github-actions Bot closed this Nov 25, 2021
juntyr added a commit to juntyr/ron that referenced this pull request Jan 5, 2022
juntyr added a commit to juntyr/ron that referenced this pull request Jan 5, 2022
juntyr added a commit to juntyr/ron that referenced this pull request Sep 18, 2022
@juntyr juntyr mentioned this pull request Sep 18, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants