[v0.9] Breaking: Treat Some like any newtype variant#413
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #413 +/- ##
==========================================
- Coverage 91.25% 89.93% -1.32%
==========================================
Files 56 56
Lines 6542 6668 +126
==========================================
+ Hits 5970 5997 +27
- Misses 572 671 +99 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
62b7571 to
9b94603
Compare
torkleyy
left a comment
There was a problem hiding this comment.
I think the change makes sense, but I'm wondering if it will break anything in practice. In any case this change needs to be in a breaking version release. Maybe we should merge it when we are about to release other breaking changes.
Good point! I agree that we can keep around this change until we’re ready for 0.9 as it’s quite small and won’t be difficult to rebase then |
|
#465 shows that this might actually be a bad idea, as |
|
#465 now includes and supersedes this PR |
* Early prototyping with a typed arbitrary fuzzer (ser only so far) and reading in the corpus * Also fuzz the ser::PrettyConfig (identation-excluded) * Start implementing the arbitrary typed data deserialising fuzzing * Fix None inside stack of implicit Some-s * Detect problematic Some inside deserialize_any with unwrap_variant_newtypes * Fix clippy::useless_conversion lint * Another alternative: allow newtype variant unwrapping in deserialize_any, also for Some + Fix check_struct_type lookahead * Fix PartialOrd impls for Map and Float * Implement arbitrary tuple struct (static field names slice FIXME) fuzzing deserialisation * Fully fix Float comparison with total_ord * Fix clippy lints * Finished arbitrary struct and enum deserialisation fuzzing * Create CI workflow for benchmarking * Fix corpus download and unzip * Give benchmark the comparison branch name * Restrict the benchmark to unique cases (ty, value, ron) * Add test for the Serialize identifier validation * Add tests for further fuzzer-found bugs * Add the extensive CHANGELOG entry * Add the test and changelog entry from the subsumed #413 * Add an early return + more tests for the expensive newtype or tuple check for unwrapping newtype variants
How should
Somebe treated in the presence of theunwrap_variant_newtypesextension? With that extension,NewtypeVariant(Some(a: 42, b: 24))already worked, butSome(a: 42, b: 24)did not. In my opinionSomeis just a newtype variant and should parse consistently here. @torkleyy what are your thoughts?CHANGELOG.md