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

refactor[venom]: make venom repr parseable #4402

Merged
merged 19 commits into from
Dec 19, 2024

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Dec 16, 2024

What I did

How I did it

How to verify it

Commit message

make the `__repr__()` implementations for venom data structures
(`IRContext`, `IRFunction, `IRBasicBlock`) emit strings which will
round-trip through the parser.

for labels generated in the frontend which are not necessarily
valid identifiers (e.g. `"internal 5 foo()"`), these are represented as
escaped strings. the expedient way to implement this was to simply use
`json.loads` / `json.dumps`; there did not seem to be any convenient
stdlib or lark function to do this. since this adds grammar complexity,
the other method that was considered was to map all labels to valid
identifiers in `ir_node_to_venom.py`. but this approach seems easier
than cleaning up all the non-identifier labels generated by the
frontend; plus being able to have arbitrary strings in labels seems like
it will come in handy during debugging some time.

a couple other grammar updates/fixes:
- update instruction order in the text format for `phi` and `invoke`
- ensure instructions are terminated with newline (otherwise they can
  continue slurping tokens from the next line).
- allow signed ints inside `CONST` nodes as `IRLiteral` accepts negative
  numbers

misc/refactor:
- remove a dead function (`str_short()`).
- remove a dead branch in `ir_node_to_venom.py`
- when optimization level is set to `CODESIZE`, the roundtrip test
  is set to xfail, as the data section contains bytestrings (which do
  not parse yet).

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

make the repr implementations for venom data structures (IRContext,
IRFunction, IRBasicBlock) emit strings which will round-trip through the
parser.

remove a dead function (`str_short()`)
@charles-cooper charles-cooper marked this pull request as ready for review December 17, 2024 23:44
@charles-cooper charles-cooper added this to the v0.4.1 milestone Dec 18, 2024
%import common.CNAME
%import common.DIGIT
%import common.LETTER
%import common.WS
%import common.INT
%import common.SIGNED_INT
%import common.ESCAPED_STRING
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if it would be better to just change up some of the labels instead of parsing escaped string (I dont know how many cases are there I stumbled so far on case with the space which could be replaced by underscore)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i could do it both ways honestly, but this seemed nicer for debugging (you can imagine the frontend generating a sentence or description of where it came from for a label)

@charles-cooper charles-cooper merged commit eee31e7 into vyperlang:master Dec 19, 2024
157 checks passed
@charles-cooper charles-cooper deleted the refactor/bb-output branch December 19, 2024 18:01
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 this pull request may close these issues.

3 participants