Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify sorts and review parsing in KMIR-AST #449

Open
jberthold opened this issue Feb 10, 2025 · 1 comment · Fixed by #450
Open

Simplify sorts and review parsing in KMIR-AST #449

jberthold opened this issue Feb 10, 2025 · 1 comment · Fixed by #450
Assignees

Comments

@jberthold
Copy link
Member

A few improvements to make working with the KMIR-AST::Pgm easier.

  • The AllocBytes sort in body.md contains a sequence of optional byte values. This seems to be an unnecessary complication.
  • The ProvenanceMap is currently parsed as a simple Int list but could be more structured.
  • A few other FIXME notes from prior work on the python parser can be removed now
@jberthold jberthold self-assigned this Feb 10, 2025
@jberthold
Copy link
Member Author

jberthold commented Feb 10, 2025

As it turned out from the integration tests we have, the bytes in constant operands may not always be populated in stable-mir, i.e. there can be null values in the byte sequence.
From inspection of the tests that fail on this, it appears as though the nulls' intention is to reserve space/pad memory for an optional value that is None, the None being represented as [0,0,0,0,0,0,0,0] and a sequence of 8 nulls following as padding for the size of the type that could be here.
Parsing the bytes values will be adapted to use zero for any null values.

automergerpr-permission-manager bot pushed a commit that referenced this issue Feb 10, 2025
* Removes the `AllocBytes` sort from `Allocation`, replacing it by
`MIRBytes`
* implements parser for `group(mir-bytes)`: an array of either `int` or
`null` is parsed into a byte string token (`b"..."`)
* parses `ProvenanceMapEntry` into a data structure rather than a list
of two `Int`
* removes a few other erratic `FIXME` comments 

Fixes #449

---------

Co-authored-by: devops <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant