Repository navigation
Rust improvements #516
Description
- Fix invalid JSON in header comments
- Optionally emit top-level marshaling functions
- Improve header comments
- Only show how to deserialize JSON (no need to show going back to string)
- Don't use extra empty lines between statements
- Use actual top-level type names in sample
- Optionally omit redundant rename attributes when property name and JSON name match (dense mode)
Activity
phrohdoh commentedon Feb 13, 2018
The invalid JSON mentioned is here and should be
let json = r#"{"answer": 42}"#;.Dense option for Rust
dvdsgl commentedon Feb 13, 2018
@phrohdoh would you like to submit a PR to improve the header comments? There are many quick wins there.
Ryman commentedon Feb 17, 2018
To add:
serde_jsonbut that part actually requiresserde_derive. It would work if they copied all of the code though so maybe that's okay, but I imagine some people may not know that they need to importserde_derivein their library code. Playpen example of it compiling with the changeI'd submit a patch for the first point myself but when I cloned the repo and tried running tests without modification they took way too long on my machine (with only the rust fixtures). Unsure if that's a problem with my configuration or not but thought it's worth mentioning.
dvdsgl commentedon Feb 17, 2018
@Ryman
FIXTURE=rust npm testwill generate and test a couple hundred Rust programs. Do individual tests take longer than expected or the entire test suite or both?schani commentedon Feb 17, 2018
@dvdsgl Is it possible that we're downloading and re-compiling all Rust dependencies for each test case?
Ryman commentedon Feb 17, 2018
It was the entire rust test suit afaik, the first 4 or so tests ran... relatively quick then 20 or so more would take many many minutes more, I don't have access to my laptop where I can run it right now to check though. It definitely felt like it was compiling more than necessary.
dvdsgl commentedon Feb 17, 2018
@Ryman the test slowness is fixed now
Ryman commentedon Feb 23, 2018
@dvdsgl Hey, sorry for the delay, I looked into the compile times a little and some of your generated types, when passed to serde are creating 40k+ lines of code (post macro expansion, try using
cargo-expandif you want to see) with 30k of them being in a single method which rustc chokes badly on intranslation(you can see this on a nightly compiler by settingRUSTFLAGS=-Ztime-passes. I had hoped to look into improving the codegen for serde and it seems to be a known problem on the issue tracker but I haven't been successful this week.I'd advise that you skip some of your tests such as this one unless you enable some "extra-tests" feature or something which CI would enable, it should make local development nicer :) Or maybe it could just be an error to try to quicktype pathalogical inputs? What do you think?
dvdsgl commentedon Feb 23, 2018
@Ryman yes, thank you for the analysis and we agree that we need to split up some of our test data that needlessly require target languages to emit absurdly large types: #559
schani commentedon Aug 1, 2018
Closing this. But please keep submitting Rust PRs!