Support 1.1 LST append, binary e-exps in field value position #820
Conversation
| @@ -1601,7 +1601,7 @@ impl<'data> From<LazyRawFieldExpr<'data, BinaryEncoding_1_1>> | |||
| use LazyRawFieldExpr::*; | |||
| match binary_field { | |||
| NameValue(name, value) => NameValue(name.into(), value.into()), | |||
| NameEExp(_, _) => todo!("(name, e-exp) field in binary Ion 1.1"), | |||
| NameEExp(name, eexp) => NameEExp(name.into(), eexp.into()), | |||
There was a problem hiding this comment.
🗺️ Converting a LazyRawFieldExpr<_, v1_1::Binary> to a LazyRawFieldExpr<_, AnyEncoding> for use in the format-agnostic Reader.
| fn peek_value( | ||
| fn peek_value_expr( |
There was a problem hiding this comment.
🗺️ Previously the lazy binary struct could only surface fields with a value literal. This method can now return either a ValueLiteral or an EExp.
| @@ -279,24 +277,26 @@ impl<'top> RawBinaryStructIterator_1_1<'top> { | |||
| return IonResult::incomplete("found field name but no value", after_name.offset()); | |||
| } | |||
|
|
|||
| let (value, after_value) = match Self::peek_value(after_name)? { | |||
There was a problem hiding this comment.
🗺️ All variable renames from here to line 288.
| match E::ion_version() { | ||
| IonVersion::v1_0 => self.write_lst_append()?, | ||
| IonVersion::v1_1 => self.write_encoding_directive()?, | ||
| } |
There was a problem hiding this comment.
🗺️ The Writer now writes either an LST or encoding directive depending on the Ion version of the stream.
| let mut symbol_table = directive.sexp_writer()?; | ||
| symbol_table | ||
| .write_symbol("symbol_table")? | ||
| .write_symbol("$ion_encoding")? |
There was a problem hiding this comment.
🗺️ The directive's (symbol_table ...) operation starts with a reference to the $ion_encoding module to re-export its symbols.
| @@ -349,6 +349,7 @@ impl<Encoding: Decoder, Input: IonInput> ExpandingReader<Encoding, Input> { | |||
| if let Some(new_version) = pending_changes.switch_to_version.take() { | |||
| symbol_table.reset_to_version(new_version); | |||
| pending_changes.has_changes = false; | |||
| pending_changes.is_lst_append = false; | |||
There was a problem hiding this comment.
🗺️ Resetting some state in the early-return cases.
| ValueRef::Symbol(symbol) if symbol == "$ion_encoding" => { | ||
| let active_symtab = operation.expanded().context.symbol_table(); | ||
| for symbol in active_symtab.application_symbols() { | ||
| symbol_table.add_symbol(symbol.clone()); | ||
| } |
There was a problem hiding this comment.
🗺️ If the reader encounters a $ion_encoding in the (macro_table ...), it copies its application symbols over to the table we're constructing. While symbol.clone() is just bumping a reference count, there's room for optimization here.
| } | ||
| } | ||
| ValueRef::Symbol(symbol) => { | ||
| todo!("modules other than $ion_encoding (found symbol '{symbol:?}')") |
There was a problem hiding this comment.
🗺️ Symbols other than $ion_encoding (which would represent other modules with symbols to re-export) are not yet supported.
| // TODO: Update this when the implementation is complete | ||
| const NUM_SYSTEM_SYMBOLS_1_1: usize = 10; |
There was a problem hiding this comment.
🗺️ For now, the Ion 1.1 reader and writer use the Ion 1.0 symbol table. We'll set this to 0 once we have the system symbol encoding documented.
Changes:
Writernow emits an encoding directive instead of a 1.0-style symbol table.(symbol_table $ion_encoding ["sym1", "sym2", "sym3"])syntax as re-exporting the symbols in the current context's table at the head of the new context's table.SymbolTabletype now exposes methods for getting its system symbols and its application symbols separately.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.