Adds binary 1.1 read support for symbol address annotations #814
Conversation
| self.read_value(opcode) | ||
| .map(|(v, after)| (Some(v), after)) | ||
| self.read_value(opcode).map(|(v, after)| (Some(v), after)) |
There was a problem hiding this comment.
🗺️ cargo fmt re-flowed some things in this file even though they hadn't changed in this commit. You can ignore this and the one below (lines 630-654).
| // TODO: This implementation actively reads the annotations, which isn't necessary. | ||
| // At this phase of parsing we can just identify the buffer slice that contains | ||
| // the annotations and remember their encoding; later on, the annotations iterator | ||
| // can actually do the reading. That optimization would be impactful for FlexSyms | ||
| // that represent inline text. |
There was a problem hiding this comment.
🗺️ I did this optimization for skipping FlexUInt symbol addresses, but doing it for FlexSym would be much more involved. I introduced two methods below to encapsulate that logic:
consume_flex_uint(skips without reading)consume_flex_sym(skips by reading)
| let sequence = EncodedAnnotations { | ||
| encoding: AnnotationsEncoding::FlexSym, | ||
| header_length: 1, // 0xE7 | ||
| sequence_length: u16::try_from(flex_sym.size_in_bytes()).map_err(|_| { | ||
| IonError::decoding_error( | ||
| "the maximum supported annotations sequence length is 65KB.", | ||
| ) | ||
| })?, | ||
| }; |
There was a problem hiding this comment.
🗺️ Previously I was constructing an EncodedAnnotations struct in each match arm. I lifted out out to the end of the method, which I think helped readability.
| let sequence = EncodedAnnotations { | ||
| encoding: AnnotationsEncoding::FlexSym, | ||
| header_length, | ||
| sequence_length, | ||
| }; |
There was a problem hiding this comment.
🗺️ Here's the instantiation of the EncodedAnnotations.
| // TODO: As an optimization, see if we can avoid actually reading the flex_sym. | ||
| let (flex_sym, remaining) = self.read_flex_sym()?; | ||
| if let FlexSymValue::Opcode(opcode) = flex_sym.value() { | ||
| todo!("FlexSym escapes in annotation sequences; opcode: {opcode:?}"); |
There was a problem hiding this comment.
🗺️ The skipper doesn't (yet) attempt to interpret the opcode and skip whatever it indicates.
| } | ||
|
|
||
| /// Skips beyond a `FlexUInt` at the head of the buffer, returning the remaining slice. | ||
| fn consume_flex_uint(self) -> IonResult<Self> { |
There was a problem hiding this comment.
🗺️ Skips a FlexUInt without actually reading it.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.