Skip to content

refactor[venom]: make venom repr parseable#4402

Merged
charles-cooper merged 19 commits intovyperlang:masterfrom
charles-cooper:refactor/bb-output
Dec 19, 2024
Merged

refactor[venom]: make venom repr parseable#4402
charles-cooper merged 19 commits intovyperlang:masterfrom
charles-cooper:refactor/bb-output

Conversation

@charles-cooper
Copy link
Copy Markdown
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.WS
%import common.INT
%import common.SIGNED_INT
%import common.ESCAPED_STRING
Copy link
Copy Markdown
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
Copy Markdown
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
@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