More enum decoding#740
Conversation
tothtamas28
commented
Oct 13, 2025
tothtamas28
commented
- Decode single-variant enums
- Decode multi-variant enums with direct tag encoding
jberthold
left a comment
jberthold
left a comment
There was a problem hiding this comment.
A few questions but we can merge this and follow up in the next decoding iteration.
| case _: | ||
| raise AssertionError('Undhandled case') |
There was a problem hiding this comment.
Is there another case? (I think not)
There was a problem hiding this comment.
There isn't, I just want to explicitly indicate to the reader who's not familiar with the data model that this is the case.
|
|
||
| assert len(discriminants) == 1, 'Expected a single discriminant for single-variant enum' | ||
| discriminant = discriminants[0] | ||
| assert tag_index == discriminant, 'Assumed tag_index to be the same as the discriminant' |
There was a problem hiding this comment.
🤔 a bit confusing because the tag_index sounds like metadata, while the discriminants vector stores values (the ones used in #switchInt and retrieved by RValue::Discriminant). Or maybe the Single variant shape works differently and tag_index is a misnomer?
There was a problem hiding this comment.
I may be 100% wrong here. For P-Token though, this assertion doesn't fail.
There was a problem hiding this comment.
You could check what happens when you have a non-standard discriminant in a single-variant enum:
enum Testing { Testing{ field: u16 } = 123 , }There was a problem hiding this comment.
I did not try this, but according to (a more recent version of) the docs, index is always 0 for Single(index). I will fix this in the next PR.
| assert tag_field == 0, 'Assumed tag field to be zero' | ||
| assert len(offsets) == 1, 'Assumed offsets to only contain the tag offset' |
There was a problem hiding this comment.
Is it certain that there are no other fields shared between the different variants of an enum?
(I was unable to produce a counter-example in my experiments but the doc.s talk about the possibility of other shared fields...).
We can merge and iterate, though.
There was a problem hiding this comment.
It is far from certain, I only have guesses about this data model. That's why I made all the assertions with the error messages. So far it seems to work for P-Token.
| class IntegerLength(Enum): | ||
| I8 = 1 | ||
| I16 = 2 | ||
| I32 = 4 | ||
| I64 = 8 | ||
| I128 = 16 |
There was a problem hiding this comment.
(nit-pick) We also have IntTy in the same file and quite similar to this one. Do we need both?
There was a problem hiding this comment.
I had the same thought, but the Rust code has two separate enums, and I didn't want to innovate.
(The reason there's two might be that the other one also has a value for isize).
2 participants
