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

Implement proper fix for "Broken deserialization of Lists with offsets" (#248) #250

Closed
Tracked by #238
chmp opened this issue Oct 23, 2024 · 2 comments
Closed
Tracked by #238

Comments

@chmp
Copy link
Owner

chmp commented Oct 23, 2024

The current fix for #248 requires skips the elements by deserializing any skipped values. Figure out an alternative that avoids any unnecessary work. Ideas:

  • Add skip to the interface of the deserializers and implement a solution by shifting around the offsets
  • Implement random access deserialization
    • This will also allow to deserialize enums with holes or lists with null values with data
    • This would also allow to support lists with non-monotically increasing offsets (although such lists could be ill formed)

Potential design of RandomAccessDeserializer

trait RandomAccessDeserializer {
  /// Get the number of items
  fn len(&self) -> usize;
  /// Deserialize the `i`-th item of this deserializer into the given type
  fn get<'de, T: Deserialize<'de>>(&'de self, i: usize) -> Result<T>;
}

struct Deserializer {
  inner: StructDeserializer,
  next: usize,
  len: usize,
}

// impl Deserializer analogous to OuterSequenceDeserializer

Alternative design

Context:

  • the deserialize API fully consumes the Deserializer, no option to hand in additional info
  • some APIs require iterator style access (e.g., SeqAccess, MapAccess)
    • but the corresponding impl knows the number of elements, e.g., it can always use a non-iterate design
  • current impl uses stacked iterators

Extend serde interface to take an index, add wrapper type around a deserializer and index and calls the deserializer with the given index.

@chmp chmp changed the title Implement proper fix for #248 Implement proper fix for "Broken deserialization of Lists with offsets" (#248) Oct 26, 2024
@chmp chmp mentioned this issue Jan 19, 2025
14 tasks
@chmp chmp mentioned this issue Feb 1, 2025
3 tasks
@chmp
Copy link
Owner Author

chmp commented Feb 1, 2025

Work started in #260

@chmp
Copy link
Owner Author

chmp commented Feb 6, 2025

Implemented with #260

@chmp chmp closed this as completed Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant