Repository navigation
Adds some utilities for Ion 1.0 BytecodeGenerators - #1124
Conversation
|
Looks like I have a spotbugs warning to deal with. |
| private fun initOperationKindForType(state: Int): Int { | ||
| return when (state) { | ||
| in 0x00..0x0E -> OperationKind.UNSET | ||
| 0x0F -> OperationKind.NULL | ||
| 0x10, 0x11, 0x1F -> OperationKind.BOOL | ||
| in 0x20..0x2F -> OperationKind.INT | ||
| in 0x31..0x3F -> OperationKind.INT | ||
| in 0x40..0x4F -> OperationKind.FLOAT | ||
| in 0x50..0x5F -> OperationKind.DECIMAL | ||
| in 0x60..0x6F -> OperationKind.TIMESTAMP | ||
| in 0x70..0x7F -> OperationKind.SYMBOL | ||
| in 0x80..0x8F -> OperationKind.STRING | ||
| in 0x90..0x9F -> OperationKind.CLOB | ||
| in 0xA0..0xAF -> OperationKind.BLOB | ||
| in 0xB0..0xBF -> OperationKind.LIST | ||
| in 0xC0..0xCF -> OperationKind.SEXP | ||
| 0xD0, in 0xD2..0xDF -> OperationKind.STRUCT | ||
| 0xE0 -> OperationKind.IVM | ||
| in 0xE1..0xEE -> OperationKind.ANNOTATIONS | ||
| // Everything else: 12..1E, 30, D1, EF, F0..FF, | ||
| else -> OperationKind.UNSET | ||
| } | ||
| } |
There was a problem hiding this comment.
You are excluding some illegal typeIDs here (int 0 with type code 3, bools that are not true/false/null, null annotation, etc). but there are still some more that you are not excluding:
0x40with low nibble0x01-0x03,0x05-0x07, and0x09-0x0Eare illegal - only L=4 (for FP32), L=8 (FP64), L=0 (0e0) and L=15 (null) are supported for float0x60with low nibble0x00/0x01is illegal - timestamps require at least offset and year0xE0with low nibble0x01-0x02is illegal - annotations require the annot_length field, at least one annotation and the value
| "0x3E, INT", | ||
| "0x3F, INT", | ||
| "0x40, FLOAT", | ||
| "0x4E, FLOAT", |
There was a problem hiding this comment.
Already mentioned this, 0x4E and some others are technically illegal.
| offset | ||
| ) | ||
| } catch (e: IllegalArgumentException) { | ||
| println("Timestamp starting at $position") |
There was a problem hiding this comment.
Remember to remove the println if this was for testing
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## ion11 #1124 +/- ##
========================================
Coverage ? 68.35%
Complexity ? 5868
========================================
Files ? 189
Lines ? 24043
Branches ? 4286
========================================
Hits ? 16435
Misses ? 6302
Partials ? 1306 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // @JvmStatic | ||
| val TYPE_LENGTHS by lazy { IntArray(256) { initTypeLength(it) } } | ||
|
|
||
| // @JvmStatic |
|
|
||
| /** | ||
| * Given a typeId in the range 0x20..0x3F, returns either -1 or 1. | ||
| * This uses some clever bit twiddling to avoid any branching. |
There was a problem hiding this comment.
I'd love to have some quantification of the benefit as an interesting learning. Not blocking.
There was a problem hiding this comment.
I don't have a quantification of the benefit. However, I've been trying to minimize hard-to-predict branching. I'll add a TODO to go and benchmark this vs branching later.
| } | ||
|
|
||
| /** | ||
| * Returns a signed integer up to 7 bytes, with an 1 byte integer signifying how many varuint bytes were used in its encoding. |
There was a problem hiding this comment.
It looks like bad things could happen if the VarInt exceeds 7 bytes, so we probably need to throw if that happens. I know it adds a branch, but I don't know how to avoid it safely. I'd imagine the branch would ~always be predicted correctly.
| // TODO: See if we can have a shared set of reusable buffers for this instead of allocating a copy. | ||
| val bytes = valueBytes.copyOfRange(p, p + coefficientLength) |
There was a problem hiding this comment.
| var length = 2 | ||
| do { | ||
| length++ | ||
| if (length > 7) throw IonException("VarUInt value is too large") |
There was a problem hiding this comment.
I think it would be fine to put this after the loop.
3 participants
Issue #, if available:
None
Description of changes:
Adds some utilities for handling Timestamp and Decimal references in Ion 1.0
BytecodeGenerators, and for converting from Ion 1.0 typeIds to operation kind.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.