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

Add extract expressions #118

Merged
merged 3 commits into from
Sep 23, 2021
Merged

Add extract expressions #118

merged 3 commits into from
Sep 23, 2021

Conversation

jasonpaulos
Copy link
Contributor

@jasonpaulos jasonpaulos commented Sep 22, 2021

Adds expressions for using new TEAL 5 extract opcodes.

@jasonpaulos jasonpaulos marked this pull request as ready for review September 23, 2021 00:08
@jasonpaulos jasonpaulos self-assigned this Sep 23, 2021
Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

The code looks good to me, just had two questions:

  1. Will there be a way to force the substring or extract TEAL op code using Pyteal (i.e. the ones that take immediates)? I guess there isn't much of a usecase right now, but those ops do save a couple bytes (assuming they are uint8) if I recall correctly.
  2. There is a somewhat hidden shortcut in extract where if you specify the length to be 0 as an immediate, it extracts until the end of the string. I think this is linked to point 1 and wondering if it should be supported.

@jasonpaulos
Copy link
Contributor Author

jasonpaulos commented Sep 23, 2021

I agree it would be nice if there was a way to generate the substring and extract opcodes, since they are more efficiently encoded.

Normally if an operation can accept either a stack value or an immediate, PyTeal allows Union[int, Expr] and internally chooses the correct opcode based on the arguments. However, with these I think a better approach would be to have some sort of optimization that checks if the input expressions can be transformed into integers, and if that's the case and the integers aren't too large, then the immediate form of the opcode is used. Furthermore, in most cases it will probably be more efficient to use extract over substring, so the optimization could also make this happen when it's beneficial.

For extract the length arg being 0 indicates the end of the string, but for extract3 there is no equivalent to this, correct? I.e. if I pass in 0 as the length arg, I will get a 0 length substring. If this is the case, that's a constraint we'll need to keep in mind when writing an optimization for this.

Since there's a fair bit of complexity here, I think it would be better to address this in a separate issue.

edit: I made an issue: #119

@algochoi algochoi self-requested a review September 23, 2021 16:43
Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

I think that sounds good -- Yes the 0 length trick will only work with the immediates extract opcode rather than the extract3 op code.

Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

@jasonpaulos jasonpaulos merged commit be1a89b into master Sep 23, 2021
@jasonpaulos jasonpaulos deleted the more-teal5-features branch September 23, 2021 18:26
@jasonpaulos jasonpaulos mentioned this pull request Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants