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

Replace Box<dyn Iterator> API for reading annotations #511

Open
zslayton opened this issue Apr 17, 2023 · 0 comments
Open

Replace Box<dyn Iterator> API for reading annotations #511

zslayton opened this issue Apr 17, 2023 · 0 comments
Labels
code-quality Issues of style, consistency, or factoring

Comments

@zslayton
Copy link
Contributor

Currently the IonReader trait includes an annotations() method that returns a Box<dyn Iterator<_>> over the current value's annotations:

/// Returns an iterator that will yield each of the annotations for the current value in order.
/// If there is no current value, returns an empty iterator.
// TODO: When GATs are stabilized, we can change this to a known associated type that's generic
// over the lifetime of &self.
fn annotations<'a>(&'a self) -> Box<dyn Iterator<Item = IonResult<Self::Symbol>> + 'a>;

As the comment notes, we intended to replace this Box<dyn ...> with a generic associated type (GAT), so implementations of IonReader could supply a concrete Iterator implementation. However, we cannot do that for this particular method because we need the IonReader trait to be object safe. For the moment, traits that use GATs are not object safe. See also: rust-lang/rust#81823.

Iterating over a value's annotations shouldn't require the expense of Box<dyn>, so we need to replace this with a concrete implementation that would work for all reader implementations.

@almann had a good suggestion for this--add a method like this to IonReader:

trait IonReader {
    // ...
    fn read_annotation(&mut self) -> IonResult<Option<Self::Symbol>>;
}

This method would require reader implementations to maintain a small amount of state to track which annotations have been read. When reading the value foo::bar::5, calling read_annotation() once would return Ok(Some(foo)), calling it a second time would return Ok(Some(bar)), and calling it a third time would return Ok(None).

Having this method available in all IonReader implementations would allow us to provide a concrete, stack-allocated, statically-dispatched AnnotationsIterator type that worked for all readers.

@zslayton zslayton added the code-quality Issues of style, consistency, or factoring label Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality Issues of style, consistency, or factoring
Projects
None yet
Development

No branches or pull requests

1 participant