-
Notifications
You must be signed in to change notification settings - Fork 36
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
Adds support for qualified refs to $ion
, $ion_encoding
#830
Changes from 3 commits
809dbfe
fabb7fe
6a53680
139c369
a80ec3d
626cbfa
8ecd1c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -277,6 +277,15 @@ pub enum AnyEExpArgGroupKind<'top> { | |
Binary_1_1(BinaryEExpArgGroup<'top>), | ||
} | ||
|
||
impl<'top> AnyEExpArgGroupKind<'top> { | ||
fn encoding(&self) -> &ParameterEncoding { | ||
match self { | ||
AnyEExpArgGroupKind::Text_1_1(g) => g.encoding(), | ||
AnyEExpArgGroupKind::Binary_1_1(g) => g.encoding(), | ||
} | ||
} | ||
} | ||
|
||
impl<'top> HasRange for AnyEExpArgGroup<'top> { | ||
fn range(&self) -> Range<usize> { | ||
match self.kind { | ||
|
@@ -353,11 +362,8 @@ impl<'top> Iterator for AnyEExpArgGroupIterator<'top> { | |
impl<'top> EExpressionArgGroup<'top, AnyEncoding> for AnyEExpArgGroup<'top> { | ||
type Iterator = AnyEExpArgGroupIterator<'top>; | ||
|
||
fn encoding(&self) -> ParameterEncoding { | ||
match self.kind { | ||
AnyEExpArgGroupKind::Text_1_1(g) => g.encoding(), | ||
AnyEExpArgGroupKind::Binary_1_1(g) => g.encoding(), | ||
} | ||
fn encoding(&self) -> &ParameterEncoding { | ||
self.kind.encoding() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ The callsite mentioned above. |
||
} | ||
|
||
fn resolve(self, context: EncodingContextRef<'top>) -> ArgGroup<'top, AnyEncoding> { | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,6 @@ | ||||||||||||||||
use crate::lazy::binary::raw::type_descriptor::Header; | ||||||||||||||||
use crate::lazy::binary::raw::v1_1::immutable_buffer::AnnotationsEncoding; | ||||||||||||||||
use crate::lazy::expanded::template::ParameterEncoding; | ||||||||||||||||
use crate::lazy::binary::raw::v1_1::value::BinaryValueEncoding; | ||||||||||||||||
use crate::IonType; | ||||||||||||||||
use std::ops::Range; | ||||||||||||||||
|
||||||||||||||||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ Previously, 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 This meant that |
||||||||||||||||
// If the compiler decides that a value is too large to be moved/copied with inline code, | ||||||||||||||||
// it will relocate the value using memcpy instead. This can be quite slow by comparison. | ||||||||||||||||
// | ||||||||||||||||
|
@@ -90,8 +90,6 @@ pub(crate) struct EncodedValue<HeaderType: EncodedHeader> { | |||||||||||||||
pub annotations_encoding: AnnotationsEncoding, | ||||||||||||||||
// The offset of the type descriptor byte within the overall input stream. | ||||||||||||||||
pub header_offset: usize, | ||||||||||||||||
// If this value was written with a tagless encoding, this will be 0. Otherwise, it's 1. | ||||||||||||||||
pub opcode_length: u8, | ||||||||||||||||
// The number of bytes used to encode the optional length VarUInt following the header byte. | ||||||||||||||||
pub length_length: u8, | ||||||||||||||||
// The number of bytes used to encode the value itself, not including the header byte | ||||||||||||||||
|
@@ -130,6 +128,15 @@ impl<HeaderType: EncodedHeader> EncodedValue<HeaderType> { | |||||||||||||||
start..end | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
/// 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, | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
Comment on lines
+131
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather that setting/storing a number of opcode bytes on every value, we can derive whether an opcode byte was present. Fixes #805. |
||||||||||||||||
/// Returns the number of bytes used to encode this value's data. | ||||||||||||||||
/// If the value can fit in the type descriptor byte (e.g. `true`, `false`, `null`, `0`), | ||||||||||||||||
/// this function will return 0. | ||||||||||||||||
|
@@ -262,14 +269,14 @@ mod tests { | |||||||||||||||
use crate::lazy::binary::encoded_value::EncodedValue; | ||||||||||||||||
use crate::lazy::binary::raw::type_descriptor::Header; | ||||||||||||||||
use crate::lazy::binary::raw::v1_1::immutable_buffer::AnnotationsEncoding; | ||||||||||||||||
use crate::lazy::expanded::template::ParameterEncoding; | ||||||||||||||||
use crate::lazy::binary::raw::v1_1::value::BinaryValueEncoding; | ||||||||||||||||
use crate::{IonResult, IonType}; | ||||||||||||||||
|
||||||||||||||||
#[test] | ||||||||||||||||
fn accessors() -> IonResult<()> { | ||||||||||||||||
// 3-byte String with 1-byte annotation | ||||||||||||||||
let value = EncodedValue { | ||||||||||||||||
encoding: ParameterEncoding::Tagged, | ||||||||||||||||
encoding: BinaryValueEncoding::Tagged, | ||||||||||||||||
header: Header { | ||||||||||||||||
ion_type: IonType::String, | ||||||||||||||||
ion_type_code: IonTypeCode::String, | ||||||||||||||||
|
@@ -279,7 +286,6 @@ mod tests { | |||||||||||||||
annotations_sequence_length: 1, | ||||||||||||||||
annotations_encoding: AnnotationsEncoding::SymbolAddress, | ||||||||||||||||
header_offset: 200, | ||||||||||||||||
opcode_length: 1, | ||||||||||||||||
length_length: 0, | ||||||||||||||||
value_body_length: 3, | ||||||||||||||||
total_length: 7, | ||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -306,6 +306,9 @@ impl<'top> Iterator for BinaryEExpArgsInputIter<'top> { | |
EExpArg::new(parameter, EExpArgExpr::ValueLiteral(value_ref)), | ||
remaining, | ||
) | ||
} | ||
ParameterEncoding::MacroShaped(_macro_ref) => { | ||
todo!("macro-shaped parameter encoding") | ||
} // TODO: The other tagless encodings | ||
}, | ||
// If it's an argument group... | ||
|
@@ -448,7 +451,7 @@ impl<'top> IntoIterator for BinaryEExpArgGroup<'top> { | |
impl<'top> EExpressionArgGroup<'top, BinaryEncoding_1_1> for BinaryEExpArgGroup<'top> { | ||
type Iterator = BinaryEExpArgGroupIterator<'top>; | ||
|
||
fn encoding(&self) -> ParameterEncoding { | ||
fn encoding(&self) -> &ParameterEncoding { | ||
Comment on lines
-451
to
+454
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ |
||
self.parameter.encoding() | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ use crate::lazy::binary::raw::v1_1::e_expression::{ | |
}; | ||
use crate::lazy::binary::raw::v1_1::r#struct::LazyRawBinaryFieldName_1_1; | ||
use crate::lazy::binary::raw::v1_1::value::{ | ||
DelimitedContents, LazyRawBinaryValue_1_1, LazyRawBinaryVersionMarker_1_1, | ||
BinaryValueEncoding, DelimitedContents, LazyRawBinaryValue_1_1, LazyRawBinaryVersionMarker_1_1, | ||
}; | ||
use crate::lazy::binary::raw::v1_1::{Header, LengthType, Opcode, OpcodeType, ION_1_1_OPCODES}; | ||
use crate::lazy::decoder::{LazyRawFieldExpr, LazyRawValueExpr, RawValueExpr}; | ||
|
@@ -21,7 +21,6 @@ use crate::lazy::encoder::binary::v1_1::flex_int::FlexInt; | |
use crate::lazy::encoder::binary::v1_1::flex_sym::{FlexSym, FlexSymValue}; | ||
use crate::lazy::encoder::binary::v1_1::flex_uint::FlexUInt; | ||
use crate::lazy::expanded::macro_table::MacroRef; | ||
use crate::lazy::expanded::template::ParameterEncoding; | ||
use crate::lazy::expanded::EncodingContextRef; | ||
use crate::lazy::text::raw::v1_1::arg_group::EExpArgExpr; | ||
use crate::lazy::text::raw::v1_1::reader::MacroAddress; | ||
|
@@ -663,15 +662,13 @@ impl<'a> ImmutableBuffer<'a> { | |
} | ||
|
||
let encoded_value = EncodedValue { | ||
encoding: ParameterEncoding::Tagged, | ||
encoding: BinaryValueEncoding::Tagged, | ||
header, | ||
// If applicable, these are populated by the caller: `read_annotated_value()` | ||
annotations_header_length: 0, | ||
annotations_sequence_length: 0, | ||
annotations_encoding: AnnotationsEncoding::SymbolAddress, | ||
header_offset, | ||
// This is a tagged value, so its opcode length is always 1 | ||
opcode_length: 1, | ||
length_length, | ||
value_body_length, | ||
total_length, | ||
|
@@ -1544,7 +1541,8 @@ mod tests { | |
|
||
// Construct an encoding directive that defines this number of macros. Each macro will expand | ||
// to its own address. | ||
let mut macro_definitions = String::from("$ion_encoding::(\n (macro_table\n"); | ||
let mut macro_definitions = | ||
String::from("$ion_encoding::(\n (macro_table $ion_encoding\n"); | ||
Comment on lines
-1547
to
+1545
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ This PR modified the encoding directives to create a new macro table each time while also supporting re-exports of |
||
for address in MacroTable::FIRST_USER_MACRO_ID..MAX_TEST_MACRO_ADDRESS { | ||
writeln!(macro_definitions, " (macro m{address} () {address})")?; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -337,6 +337,7 @@ mod tests { | |
$ion_encoding::( | ||
(symbol_table $ion_encoding) | ||
(macro_table | ||
$ion_encoding | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ Macro table append. |
||
(macro greet (name) (make_string "hello, " name)) | ||
) | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,7 +69,7 @@ impl<E: Encoding, Output: Write> Writer<E, Output> { | |
// TODO: LazyEncoder should define a method to construct a new symtab and/or macro table | ||
let ion_version = E::ion_version(); | ||
let symbol_table = SymbolTable::new(ion_version); | ||
let macro_table = MacroTable::new(); | ||
let macro_table = MacroTable::with_system_macros(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ |
||
let context = WriterContext::new(symbol_table, macro_table); | ||
let mut writer = Writer { | ||
context, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗺️ Adding this method and then calling it below was a workaround for a lifetime constraint.