Derive Deserialize on SimpleValue

[rushsteve1]

Currently SimpleValue does not derive the Deserialize trait, meaning it can't be deserialized when within other types.

Altering value.rs and adding the following on line 98:

#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
#[serde(untagged)]
pub enum SimpleValue {

And then commenting out the implementations of Sealed and FromDhall for SimpleValue down below cause the library to compile just fine, but it fails the documentation tests with the following error.

---- src/value.rs - value::SimpleValue (line 31) stdout ----
Test executable failed (terminated by signal).

stderr:

thread 'main' has overflowed its stack
fatal runtime error: stack overflow

Notably this is the same error I had when trying to write my own custom Value type which I copied into #177. I believe it has something to do with the way SimpleValue is recursive, but so is serde_json's Value type.

Sorry for saying #183 was good before fully testing this by the way, I should have actually checked first.

[Nadrieril]

What are you trying to do specifically? Deriving Deserialize on SimpleValue seems wrong: it would treat it as a normal enum, so it would expect inputs like { "Optional": { "Text": "foo" } } or something like that instead of using dhall to parse the input. Is that what you wanted?

[rushsteve1]

The addion of the the #[serde(untagged)] should cause Serde to deserialize based on the type so it wouldn't look like { "Optional": { "Text": "foo" } }. Serde Docs on Enum tagging

The exact use case was something like this example using serde_json where a file is serialized into a HashMap of Strings and Values, and the values are parsed into more concrete types later.

[Nadrieril]

I imagine you want to write:

#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
struct Foo {
    foo: SimpleValue,
    bar: String,
}

and then parse some dhall into this struct, such that whatever is in the foo field gets shoved into the SimpleValue?

[Nadrieril]

Ok I see. Deriving Deserialize still would not work. I'd need a manual impl of Deserialize for SimpleValue, exactly like the one serde_json::Value has. That would conflict with its current manual FromDhall impl, since Deserialize implies FromDhall. Maybe I designed those impls wrong because they would need to change a bit to allow that :/

[rushsteve1]

In the implementation I came up with I had to comment out the manual FromDhall impl to get it to compile.

[rushsteve1]

BIG EDIT because I cannot read nor write apparently, I've been talking about SimpleValue this whole time, but I've been writing SimpleType.

SimpleValue is what I mean.

[Nadrieril]

BIG EDIT because I cannot read nor write apparently, I've been talking about SimpleValue this whole time, but I've been writing SimpleType.

SimpleValue is what I mean.

I gathered yeah.

In the implementation I came up with I had to comment out the manual FromDhall impl to get it to compile.

Yeah, makes sense. I'm trying the manual impl rn, we'll see if it works

[rushsteve1]

Interesting discovery from my testing, the same error can occur in other types of recursive structures, not just SimpleValue:

use serde::Deserialize;

#[derive(Debug, Deserialize)]
#[serde(untagged)]
enum Loop {
    Number(usize),
    MaybeRecur(Box<Option<Loop>>)
}

fn main() {
    // Works
    let _: Loop = serde_dhall::from_str("Some 5").parse().unwrap();

    // Works
    let _: Loop = serde_dhall::from_str("Some (Some 5)").parse().unwrap();

    // Errors with the stack overflow error from above
    let _: Loop = serde_dhall::from_str("{ x = 5 }").parse().unwrap();
}

Note that the last line wouldn't be valid for the type, but doesn't cause the unwrap to fail, it causes the program to stack overflow and panic.

[Nadrieril]

Huh, that's definitely a bug. Can you file it as a separate issue?

[Nadrieril]

I think https://github.com/Nadrieril/dhall-rust/pull/186 should do what you want? Look at the tests I added; if I understood you correctly, they capture what you were looking for.