Adds support for qualified refs to $ion, $ion_encoding
#830
Conversation
* Adds support for `module_name::encoding_name::parameter` syntax * Adds support for `(module_name::macro_name ...)` syntax The only modules currently supported are `$ion` (the system module) and `$ion_encoding` the active module. This patch also starts uses `Rc` to store macro references in the body of a compiled macro. This allows the macro to continue referring to its dependent even if the dependent is no longer directly addressable.
| @@ -277,6 +277,15 @@ pub enum AnyEExpArgGroupKind<'top> { | |||
| Binary_1_1(BinaryEExpArgGroup<'top>), | |||
| } | |||
|
|
|||
| impl<'top> AnyEExpArgGroupKind<'top> { | |||
| fn encoding(&self) -> &ParameterEncoding { | |||
There was a problem hiding this comment.
🗺️ Adding this method and then calling it below was a workaround for a lifetime constraint.
| AnyEExpArgGroupKind::Binary_1_1(g) => g.encoding(), | ||
| } | ||
| fn encoding(&self) -> &ParameterEncoding { | ||
| self.kind.encoding() |
There was a problem hiding this comment.
🗺️ The callsite mentioned above.
| /// Returns the number of bytes used to encode this value's opcode. If this value was serialized | ||
| /// using a tagless encoding, returns `0`. | ||
| pub fn opcode_length(&self) -> usize { | ||
| match self.encoding { | ||
| BinaryValueEncoding::Tagged => 1, | ||
| _ => 0, | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Rather that setting/storing a number of opcode bytes on every value, we can derive whether an opcode byte was present. Fixes #805.
| @@ -41,7 +41,7 @@ impl EncodedHeader for Header { | |||
| /// without re-parsing its header information each time. | |||
| #[derive(Clone, Copy, Debug, PartialEq)] | |||
| pub(crate) struct EncodedValue<HeaderType: EncodedHeader> { | |||
| pub(crate) encoding: ParameterEncoding, | |||
| pub(crate) encoding: BinaryValueEncoding, | |||
There was a problem hiding this comment.
🗺️ Previously, ParameterEncoding was a Copy type:
ion-rust/src/lazy/expanded/template.rs
Lines 82 to 88 in 6f04a8e
Starting with this PR (which stubs out macro-shaped parameter support), parameters needed the ability to hold a reference to a Macro that would last as long as necessary. To achieve this, a new variant was added: MacroShaped(Rc<Macro>).
This meant that ParameterEncoding was no longer Copy. However, the only place it was used that required it to be Copy was the EncodedValue struct, which was using it as a proxy for "encoding backing this binary value". Since binary values can only be backed by value encodings (not macros), it made sense to introduce a new type for that use case.
| fn encoding(&self) -> ParameterEncoding { | ||
| fn encoding(&self) -> &ParameterEncoding { |
There was a problem hiding this comment.
🗺️ ParameterEncoding is no longer Copy, so accessors hand out a reference instead of a copy.
| let invoked_macro = self | ||
| .context | ||
| .macro_table() | ||
| .macro_at_address(body_invocation.invoked_macro_address) | ||
| .unwrap(); | ||
| let invoked_macro = body_invocation.invoked_macro.as_ref(); |
There was a problem hiding this comment.
🗺️ There were several places where a macro address would need to be resolved into a macro reference. These are no longer needed because the macro definitions hold an Rc.
| pub fn host_macro_ref(&self) -> MacroRef<'top> { | ||
| self.context() | ||
| .macro_table() | ||
| .macro_at_address(self.host_template_address) | ||
| .unwrap() | ||
| pub fn host_macro_ref(&self) -> &'top Macro { | ||
| self.host_template.macro_ref |
There was a problem hiding this comment.
🗺️ Here and below: more address lookups and unwrapping obviated by holding an actual reference.
| /// A binary reader that only reads each value that it visits upon request (that is: lazily). | ||
| /// An Ion reader that only reads each value that it visits upon request (that is: lazily). |
There was a problem hiding this comment.
🗺️ An incidental doc comment change to address #829.
| let mut macro_table = MacroTable::new(); | ||
| let mut macro_table = MacroTable::empty(); |
There was a problem hiding this comment.
🗺️ When reading a (macro_table ...) clause, the reader no longer pre-populates the macro table with system macros.
|
|
||
| $ion_encoding::( | ||
| (macro_table | ||
| $ion_encoding // Re-export the active encoding module's macros |
There was a problem hiding this comment.
In this particular case, you could also write (export double), right?
Rcto store references to dependent macros. This allows the macro to continue referring to its dependent even if the latter is no longer directly addressable.module_name::encoding_name::parameter_namesyntax and(module_name::macro_name ...)syntax. The only modules currently supported are$ion(the system module) and$ion_encoding(the active encoding module).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.