Add support for reading system e-expressions #961
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## ion-11-encoding #961 +/- ##
==================================================
Coverage ? 70.16%
Complexity ? 6940
==================================================
Files ? 200
Lines ? 27349
Branches ? 4945
==================================================
Hits ? 19189
Misses ? 6613
Partials ? 1547 ☔ View full report in Codecov by Sentry. |
| @CsvSource({ | ||
| // (:values (:: ) ) 0 1 | ||
| "EF 01 02 01 F0 60 61 01", | ||
| // (:values 0 1) // using length-prefixed expression group |
There was a problem hiding this comment.
| // (:values 0 1) // using length-prefixed expression group | |
| // (:values (:: 0 1)) // using length-prefixed expression group |
Is this right?
There was a problem hiding this comment.
Either one is legal since we can use the implicit rest param for the argument to values. I've only used (:: ) in some places to make it clear that the encoding is an empty expression group or a single-arg expression group.
If you have a preference for always being explicit about expression groups in the tests here, I can change it.
| "EF 01 02 01 F0 60 61 01", | ||
| // (:values 0 1) // using length-prefixed expression group | ||
| "EF 01 02 07 60 61 01", | ||
| // (:values 0 1) // using delimited expression group |
There was a problem hiding this comment.
| // (:values 0 1) // using delimited expression group | |
| // (:values (:: 0 1)) // using delimited expression group |
Is this right?
| "EF 01 02 01 60 61 01 F0", | ||
| // (:values (:: 0) (:values (:: 1)) | ||
| "EF 01 02 03 60 EF 01 02 05 61 01", | ||
| // (:values (:values 0 1)) |
There was a problem hiding this comment.
| // (:values (:values 0 1)) | |
| // (:values (:values 0 1)) |
I guess we could choose not to use the expression group syntax for all of these, even though that's how they're encoded in binary...
Issue #, if available:
#741
Description of changes:
Adds support for reading the System EExpression op-code.
In the process, I discovered two issues that seem like they are not caused by my changes. I tried to create an equivalent test using non-system macros to determine whether these issues already existed, but I ran into some trouble getting it set up. If you agree that it seems like I haven't introduced a problem in this PR, then I'll go ahead and create GH issues to follow up on these bugs.
slowFillValue()when attempting to read arguments from a length-prefixed expression group.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.