Handle unknowns completely

[jabillings]

This PR is adapted from the previous one.

The main new addition it makes to this existing s/read-un-as-sq branch is treating unknown elements* as bytes not strings. It also contains changes from a main branch rebase.

*outside of the implicit syntax+undefined length case

[suyashkumar]

overall lgtm to merge, and will be good to compare the newly merge branch to main, hoping that the changes should diff cleanly. Thank you!

[suyashkumar]

just a note/thought to myself...wondering if we need to revisit the point of having VRKind construct. There's new behavior now, with VRUnknown, the value could actually be represented in two different containers: as a sequence or as bytes (which is new, at least given the behaviors of other items in the list). And anyway, it's not like we need VRKind to unpack the value stored in the container (we could use ValueType for that). So basically the VRKind construct at the moment just serves to "group" some VRs together (e.g. OW and OB) into the type of value we unpack it as in our program. But in reality, we don't need this and could just switch on the VRs directly in readValue or possibly group raw VRs by ValueType we wish to represent them as internally (but again, this doesn't work for UN, because we need other information to determine if we'll store it as bytesValue or sequenceValue).

Anyway, just some quick off the top of my head thoughts for me to revisit later.

[suyashkumar]

Oh hey, there's some build failures due to a duplicate switch/case as well.

[jabillings]

Huh, I'm just not seeing the duplicate switch/case. I only see the tag.VRUnkown type once in this switch case (and the entire file, for that matter).

[suyashkumar]

Hey! Going to close and re-open this to re-run the checks, and start looking into what's up with the build here.

[suyashkumar]

Seems like updating the go version helps with this? See
#242
If you merge jabillings#1 into your branch that should help!

[suyashkumar]

For some reasons the checks are not triggering--looking into this more.

[suyashkumar]

Could be the issue that benchstat isn't found in the GitHub actions environment for some reason. Maybe they've changed the way they handle paths with respect to GOBIN. Going to see if fixing that does anything.

[suyashkumar]

Also, I see you're wanting to merge this into s/read-un-as-sq--that seems reasonable, I can merge this into there and then polish it up as needed before merging into main (I'll list you as a co-author of course). I'll take it from here! Thank you for the contribution!

[atreyamj]

Hi Folks,
I am new to the community, first off - thank you for all the contributions - I was wondering if this PR was already merged to main and if the repo lib handles UN tags?

Thank You!