Skip to content

Commit 6bd5532

Browse files
authored
Text reader implementation cleanup (#763)
* Simplifies `EncodedTextValue` layout and shrinks several types * Environment allocation uses a capacity hint * Types now hold a ref (8 bytes) instead of the EncodingContext (24 bytes) * Removes superfluous `MatchedRawTextValue` type * Shrinks TemplateVariableReference's signature index from usize to u16
1 parent 158bff7 commit 6bd5532

20 files changed

+559
-792
lines changed

src/lazy/binary/raw/v1_1/value.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ impl<'top> LazyRawBinaryValue_1_1<'top> {
175175
fn value_body(&self) -> IonResult<&'top [u8]> {
176176
let value_total_length = self.encoded_value.total_length();
177177
if self.input.len() < value_total_length {
178-
eprintln!("[value_body] Incomplete {:?}", self);
179178
return IonResult::incomplete(
180179
"only part of the requested value is available in the buffer",
181180
self.input.offset(),

src/lazy/binary/raw/value.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ impl<'a, 'top> EncodedBinaryValueData_1_0<'a, 'top> {
231231
pub fn trailing_length_span(&self) -> Span<'top> {
232232
let stream_range = self.trailing_length_range();
233233
let offset = self.value.input.offset();
234-
let local_range = stream_range.start - offset .. stream_range.end - offset;
234+
let local_range = stream_range.start - offset..stream_range.end - offset;
235235
let bytes = &self.value.input.bytes()[local_range];
236236
Span::with_offset(stream_range.start, bytes)
237237
}
@@ -252,7 +252,7 @@ impl<'a, 'top> EncodedBinaryValueData_1_0<'a, 'top> {
252252
pub fn body_span(&self) -> Span<'top> {
253253
let stream_range = self.body_range();
254254
let offset = self.value.input.offset();
255-
let local_range = stream_range.start - offset .. stream_range.end - offset;
255+
let local_range = stream_range.start - offset..stream_range.end - offset;
256256
let bytes = &self.span().bytes()[local_range];
257257
Span::with_offset(stream_range.start, bytes)
258258
}

src/lazy/decoder.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ impl<'top, D: LazyDecoder> HasRange for LazyRawFieldExpr<'top, D> {
269269
// function while also preventing users from seeing or depending on it.
270270
pub(crate) mod private {
271271
use crate::lazy::expanded::r#struct::UnexpandedField;
272-
use crate::lazy::expanded::EncodingContext;
272+
use crate::lazy::expanded::EncodingContextRef;
273273
use crate::IonResult;
274274

275275
use super::{LazyDecoder, LazyRawFieldExpr, LazyRawStruct};
@@ -286,15 +286,21 @@ pub(crate) mod private {
286286
/// expansion process.
287287
fn unexpanded_fields(
288288
&self,
289-
context: EncodingContext<'top>,
289+
context: EncodingContextRef<'top>,
290290
) -> RawStructUnexpandedFieldsIterator<'top, D>;
291291
}
292292

293293
pub struct RawStructUnexpandedFieldsIterator<'top, D: LazyDecoder> {
294-
context: EncodingContext<'top>,
294+
context: EncodingContextRef<'top>,
295295
raw_fields: <D::Struct<'top> as LazyRawStruct<'top, D>>::Iterator,
296296
}
297297

298+
impl<'top, D: LazyDecoder> RawStructUnexpandedFieldsIterator<'top, D> {
299+
pub fn context(&self) -> EncodingContextRef<'top> {
300+
self.context
301+
}
302+
}
303+
298304
impl<'top, D: LazyDecoder> Iterator for RawStructUnexpandedFieldsIterator<'top, D> {
299305
type Item = IonResult<UnexpandedField<'top, D>>;
300306

@@ -320,7 +326,7 @@ pub(crate) mod private {
320326
{
321327
fn unexpanded_fields(
322328
&self,
323-
context: EncodingContext<'top>,
329+
context: EncodingContextRef<'top>,
324330
) -> RawStructUnexpandedFieldsIterator<'top, D> {
325331
let raw_fields = <Self as LazyRawStruct<'top, D>>::iter(self);
326332
RawStructUnexpandedFieldsIterator {

src/lazy/encoding.rs

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use crate::lazy::text::raw::v1_1::reader::{
2929
};
3030
use crate::lazy::text::value::{
3131
LazyRawTextValue, LazyRawTextValue_1_0, LazyRawTextValue_1_1, LazyRawTextVersionMarker_1_0,
32-
LazyRawTextVersionMarker_1_1, MatchedRawTextValue, RawTextAnnotationsIterator,
32+
LazyRawTextVersionMarker_1_1, RawTextAnnotationsIterator,
3333
};
3434
use crate::{TextKind, WriteConfig};
3535

@@ -100,26 +100,16 @@ pub trait BinaryEncoding<'top>: Encoding + LazyDecoder {}
100100

101101
/// Marker trait for text encodings.
102102
pub trait TextEncoding<'top>:
103-
Encoding + LazyDecoder<AnnotationsIterator<'top> = RawTextAnnotationsIterator<'top>>
103+
Encoding
104+
+ LazyDecoder<
105+
AnnotationsIterator<'top> = RawTextAnnotationsIterator<'top>,
106+
Value<'top> = LazyRawTextValue<'top, Self>,
107+
>
104108
{
105-
fn value_from_matched(
106-
matched: MatchedRawTextValue<'top, Self>,
107-
) -> <Self as LazyDecoder>::Value<'top>;
108-
}
109-
impl<'top> TextEncoding<'top> for TextEncoding_1_0 {
110-
fn value_from_matched(
111-
matched: MatchedRawTextValue<'_, Self>,
112-
) -> <Self as LazyDecoder>::Value<'_> {
113-
LazyRawTextValue_1_0::from(matched)
114-
}
115-
}
116-
impl<'top> TextEncoding<'top> for TextEncoding_1_1 {
117-
fn value_from_matched(
118-
matched: MatchedRawTextValue<'_, Self>,
119-
) -> <Self as LazyDecoder>::Value<'_> {
120-
LazyRawTextValue_1_1::from(matched)
121-
}
109+
// No methods, just a marker
122110
}
111+
impl<'top> TextEncoding<'top> for TextEncoding_1_0 {}
112+
impl<'top> TextEncoding<'top> for TextEncoding_1_1 {}
123113

124114
/// Marker trait for encodings that support macros.
125115
pub trait EncodingWithMacroSupport {}
@@ -192,7 +182,6 @@ impl LazyDecoder for BinaryEncoding_1_1 {
192182
// the implementation will conflict with the core `impl<T> From<T> for T` implementation.
193183
pub trait RawValueLiteral {}
194184

195-
impl<'top, E: TextEncoding<'top>> RawValueLiteral for MatchedRawTextValue<'top, E> {}
196185
impl<'top, E: TextEncoding<'top>> RawValueLiteral for LazyRawTextValue<'top, E> {}
197186
impl<'top> RawValueLiteral for LazyRawBinaryValue_1_0<'top> {}
198187
impl<'top> RawValueLiteral for LazyRawBinaryValue_1_1<'top> {}

src/lazy/expanded/compiler.rs

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::lazy::expanded::template::{
99
TemplateBodyMacroInvocation, TemplateBodyValueExpr, TemplateMacro, TemplateStructIndex,
1010
TemplateValue,
1111
};
12-
use crate::lazy::expanded::EncodingContext;
12+
use crate::lazy::expanded::EncodingContextRef;
1313
use crate::lazy::r#struct::LazyStruct;
1414
use crate::lazy::reader::LazyTextReader_1_1;
1515
use crate::lazy::sequence::{LazyList, LazySExp};
@@ -61,7 +61,7 @@ impl TemplateCompiler {
6161
/// to the template without interpretation. `(quote ...)` does not appear in the compiled
6262
/// template as there is nothing more for it to do at expansion time.
6363
pub fn compile_from_text(
64-
context: EncodingContext,
64+
context: EncodingContextRef,
6565
expression: &str,
6666
) -> IonResult<TemplateMacro> {
6767
// TODO: This is a rudimentary implementation that panics instead of performing thorough
@@ -137,7 +137,7 @@ impl TemplateCompiler {
137137
///
138138
/// If `is_quoted` is true, nested symbols and s-expressions will not be interpreted.
139139
fn compile_value<'top, D: LazyDecoder>(
140-
context: EncodingContext<'top>,
140+
context: EncodingContextRef<'top>,
141141
signature: &MacroSignature,
142142
definition: &mut TemplateBody,
143143
is_quoted: bool,
@@ -210,7 +210,7 @@ impl TemplateCompiler {
210210

211211
/// Helper method for visiting all of the child expressions in a list.
212212
fn compile_list<'top, D: LazyDecoder>(
213-
context: EncodingContext<'top>,
213+
context: EncodingContextRef<'top>,
214214
signature: &MacroSignature,
215215
definition: &mut TemplateBody,
216216
is_quoted: bool,
@@ -238,7 +238,7 @@ impl TemplateCompiler {
238238

239239
/// Helper method for visiting all of the child expressions in a sexp.
240240
fn compile_sexp<'top, D: LazyDecoder>(
241-
context: EncodingContext<'top>,
241+
context: EncodingContextRef<'top>,
242242
signature: &MacroSignature,
243243
definition: &mut TemplateBody,
244244
is_quoted: bool,
@@ -272,7 +272,7 @@ impl TemplateCompiler {
272272
/// Adds a `lazy_sexp` that has been determined to represent a macro invocation to the
273273
/// TemplateBody.
274274
fn compile_macro<'top, D: LazyDecoder>(
275-
context: EncodingContext<'top>,
275+
context: EncodingContextRef<'top>,
276276
signature: &MacroSignature,
277277
definition: &mut TemplateBody,
278278
lazy_sexp: LazySExp<'top, D>,
@@ -311,7 +311,7 @@ impl TemplateCompiler {
311311
/// Given a `LazyValue` that represents a macro ID (name or address), attempts to resolve the
312312
/// ID to a macro address.
313313
fn name_and_address_from_id_expr<'top, D: LazyDecoder>(
314-
context: EncodingContext<'top>,
314+
context: EncodingContextRef<'top>,
315315
id_expr: Option<IonResult<LazyValue<'top, D>>>,
316316
) -> IonResult<(Option<String>, usize)> {
317317
match id_expr {
@@ -352,7 +352,7 @@ impl TemplateCompiler {
352352
/// without interpretation. `lazy_sexp` itself is the `quote` macro, and does not get added
353353
/// to the template body as there is nothing more for it to do at evaluation time.
354354
fn compile_quoted_elements<'top, D: LazyDecoder>(
355-
context: EncodingContext<'top>,
355+
context: EncodingContextRef<'top>,
356356
signature: &MacroSignature,
357357
definition: &mut TemplateBody,
358358
lazy_sexp: LazySExp<'top, D>,
@@ -375,7 +375,7 @@ impl TemplateCompiler {
375375

376376
/// Adds `lazy_sexp` to the template body without interpretation.
377377
fn compile_quoted_sexp<'top, D: LazyDecoder>(
378-
context: EncodingContext<'top>,
378+
context: EncodingContextRef<'top>,
379379
signature: &MacroSignature,
380380
definition: &mut TemplateBody,
381381
annotations_range: Range<usize>,
@@ -419,7 +419,7 @@ impl TemplateCompiler {
419419

420420
/// Recursively adds all of the expressions in `lazy_struct` to the `TemplateBody`.
421421
fn compile_struct<'top, D: LazyDecoder>(
422-
context: EncodingContext<'top>,
422+
context: EncodingContextRef<'top>,
423423
signature: &MacroSignature,
424424
definition: &mut TemplateBody,
425425
is_quoted: bool,
@@ -476,7 +476,7 @@ impl TemplateCompiler {
476476
/// Resolves `variable` to a parameter in the macro signature and adds a corresponding
477477
/// `TemplateExpansionStep` to the `TemplateBody`.
478478
fn compile_variable_reference(
479-
_context: EncodingContext,
479+
_context: EncodingContextRef,
480480
signature: &MacroSignature,
481481
definition: &mut TemplateBody,
482482
annotations_range: Range<usize>,
@@ -497,7 +497,10 @@ impl TemplateCompiler {
497497
.ok_or_else(|| {
498498
IonError::decoding_error(format!("variable '{name}' is not recognized"))
499499
})?;
500-
definition.push_variable(signature_index);
500+
if signature_index > u16::MAX as usize {
501+
return IonResult::decoding_error("this implementation supports up to 65K parameters");
502+
}
503+
definition.push_variable(signature_index as u16);
501504
Ok(())
502505
}
503506
}
@@ -558,7 +561,7 @@ mod tests {
558561
definition,
559562
index,
560563
TemplateBodyValueExpr::Variable(TemplateBodyVariableReference::new(
561-
expected_signature_index,
564+
expected_signature_index as u16,
562565
)),
563566
)
564567
}
@@ -630,7 +633,7 @@ mod tests {
630633

631634
let expression = "(macro foo () 42)";
632635

633-
let template = TemplateCompiler::compile_from_text(context, expression)?;
636+
let template = TemplateCompiler::compile_from_text(context.get_ref(), expression)?;
634637
assert_eq!(template.name(), "foo");
635638
assert_eq!(template.signature().parameters().len(), 0);
636639
expect_value(&template, 0, TemplateValue::Int(42.into()))?;
@@ -644,7 +647,7 @@ mod tests {
644647

645648
let expression = "(macro foo () [1, 2, 3])";
646649

647-
let template = TemplateCompiler::compile_from_text(context, expression)?;
650+
let template = TemplateCompiler::compile_from_text(context.get_ref(), expression)?;
648651
assert_eq!(template.name(), "foo");
649652
assert_eq!(template.signature().parameters().len(), 0);
650653
expect_value(&template, 0, TemplateValue::List(ExprRange::new(1..4)))?;
@@ -661,7 +664,7 @@ mod tests {
661664

662665
let expression = r#"(macro foo () (values 42 "hello" false))"#;
663666

664-
let template = TemplateCompiler::compile_from_text(context, expression)?;
667+
let template = TemplateCompiler::compile_from_text(context.get_ref(), expression)?;
665668
assert_eq!(template.name(), "foo");
666669
assert_eq!(template.signature().parameters().len(), 0);
667670
expect_macro(
@@ -683,7 +686,7 @@ mod tests {
683686

684687
let expression = "(macro foo (x y z) [100, [200, a::b::300], x, {y: [true, false, z]}])";
685688

686-
let template = TemplateCompiler::compile_from_text(context, expression)?;
689+
let template = TemplateCompiler::compile_from_text(context.get_ref(), expression)?;
687690
expect_value(&template, 0, TemplateValue::List(ExprRange::new(1..12)))?;
688691
expect_value(&template, 1, TemplateValue::Int(Int::from(100)))?;
689692
expect_value(&template, 2, TemplateValue::List(ExprRange::new(3..5)))?;
@@ -713,7 +716,7 @@ mod tests {
713716

714717
let expression = "(macro identity (x) x)";
715718

716-
let template = TemplateCompiler::compile_from_text(context, expression)?;
719+
let template = TemplateCompiler::compile_from_text(context.get_ref(), expression)?;
717720
assert_eq!(template.name(), "identity");
718721
assert_eq!(template.signature().parameters().len(), 1);
719722
expect_variable(&template, 0, 0)?;
@@ -736,7 +739,7 @@ mod tests {
736739
(values x))))
737740
"#;
738741

739-
let template = TemplateCompiler::compile_from_text(context, expression)?;
742+
let template = TemplateCompiler::compile_from_text(context.get_ref(), expression)?;
740743
assert_eq!(template.name(), "foo");
741744
assert_eq!(template.signature().parameters().len(), 1);
742745
// Outer `values`

src/lazy/expanded/e_expression.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,23 @@ use crate::lazy::decoder::{LazyDecoder, LazyRawValueExpr};
55
use crate::lazy::encoding::TextEncoding_1_1;
66
use crate::lazy::expanded::macro_evaluator::{MacroExpr, RawEExpression, ValueExpr};
77
use crate::lazy::expanded::macro_table::MacroRef;
8-
use crate::lazy::expanded::{EncodingContext, LazyExpandedValue};
8+
use crate::lazy::expanded::{EncodingContextRef, LazyExpandedValue};
99
use crate::lazy::text::raw::v1_1::reader::MacroIdRef;
1010
use crate::IonResult;
1111
use std::fmt::{Debug, Formatter};
1212

1313
/// An e-expression (in Ion format `D`) that has been resolved in the current encoding context.
1414
#[derive(Copy, Clone)]
1515
pub struct EExpression<'top, D: LazyDecoder> {
16-
pub(crate) context: EncodingContext<'top>,
16+
pub(crate) context: EncodingContextRef<'top>,
1717
pub(crate) raw_invocation: D::EExp<'top>,
1818
pub(crate) invoked_macro: MacroRef<'top>,
1919
}
2020

2121
impl<'top, D: LazyDecoder> EExpression<'top, D> {
22+
pub fn context(&self) -> EncodingContextRef<'top> {
23+
self.context
24+
}
2225
pub fn raw_invocation(&self) -> D::EExp<'top> {
2326
self.raw_invocation
2427
}
@@ -35,7 +38,7 @@ impl<'top, D: LazyDecoder> Debug for EExpression<'top, D> {
3538

3639
impl<'top, D: LazyDecoder> EExpression<'top, D> {
3740
pub fn new(
38-
context: EncodingContext<'top>,
41+
context: EncodingContextRef<'top>,
3942
raw_invocation: D::EExp<'top>,
4043
invoked_macro: MacroRef<'top>,
4144
) -> Self {
@@ -67,7 +70,7 @@ impl<'top, D: LazyDecoder> From<EExpression<'top, D>> for MacroExpr<'top, D> {
6770
}
6871

6972
pub struct EExpressionArgsIterator<'top, D: LazyDecoder> {
70-
context: EncodingContext<'top>,
73+
context: EncodingContextRef<'top>,
7174
raw_args: <D::EExp<'top> as RawEExpression<'top, D>>::RawArgumentsIterator<'top>,
7275
}
7376

0 commit comments

Comments
 (0)