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

Support the 'method' pseudo op #50

Merged
merged 3 commits into from
Mar 24, 2023
Merged

Support the 'method' pseudo op #50

merged 3 commits into from
Mar 24, 2023

Conversation

fergalwalsh
Copy link
Collaborator

This PR provides a mechanism for pseudo ops to be supported in Tealish. So far only method is added.
I'll repeat a comment from the code here:

Note: Other pseudo ops like addr and base32 are not included here because their syntax isn't parseable by Tealish currently.
e.g addr(RIKLQ5HEVXAOAWYSW2LGQFYGWVO4J6LIAQQ72ZRULHZ4KS5NRPCCKYPCUU) is not parseable because the address isn't quoted.

In Teal this is written as

addr RIKLQ5HEVXAOAWYSW2LGQFYGWVO4J6LIAQQ72ZRULHZ4KS5NRPCCKYPCUU

No quotes! I'm not sure we'll ever support those directly as making the parser accept unquoted strings seems like a messy thing to do. We may just add some explicit helper functions to Tealish for these if necessary.

So in summary the only supported pseudo-op will be method for now.

@fergalwalsh fergalwalsh mentioned this pull request Jan 27, 2023
@barnjamin
Copy link
Contributor

barnjamin commented Jan 29, 2023

For addr type, maybe something like in #60 where the 0x prefix is used to denote what follows is hex bytes.

edit:
followed up with an attempt at this #61

@gokselcoban
Copy link
Contributor

It works well as method("name(uint64,uint64"), thank you.

On the other hand method pseudo-op doesn't work with const as expected.

It generates method CONST_NAME instead of method CONST_VALUE lines.

@fergalwalsh
Copy link
Collaborator Author

On the other hand method pseudo-op doesn't work with const as expected.

Thanks for reporting this. It's messy to solve at the moment but should be easier after an in-progress refactor so I'll leave as is for now. I added an expected failure test to track this for now.

@fergalwalsh fergalwalsh merged commit 8d56928 into main Mar 24, 2023
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.

None yet

3 participants