Add subroutine support #99

jasonpaulos · 2021-08-19T18:26:53Z

This PR implements subroutines in PyTeal. Subroutines may be declared as functions with the Subroutine function decorator, like so:

@Subroutine(TealType.uint64)
def isEven(i: Expr) -> Expr:
    return i % Int(2) == Int(0)

program = Seq([
    Assert(isEven(Int(10))),
    Approve(),
])
tealSrc = compileTeal(program, mode=Mode.Application, version=4)

Subroutine functions may accept any number of arguments, but the arguments must not be keyword-only Python arguments, must not have default values, and must not be variable-length (e.g. *args). The argument in the Subroutine decorator function is the return type of the subroutine, so in this example the subroutine must return an expression that evaluates to TealType.uint64. Subroutine functions are executed only once during the compilation process in order to extract their AST.

Since there can now be multiple "functions" in a program, I changed how the Return expression works. Now, Return means return from the current subroutine (retsub op), or if the main routine is executing, end the program with the return op. However, if you'd like to exit the program from within a subroutine, I've introduced Approve() and Reject() expressions which always evaluate to the ops int 1; return and int 0; return, respectively.

In the compiler, each subroutine is compiled independently, then they are all combined into a single series of TealComponents with the flattenSubroutines function. I had to make changes to the way scratch slots and labels are resolved in order to make this work.

Closes #71

jasonpaulos · 2021-08-25T16:47:36Z

pyteal/ast/scratch.py

    def __str__(self):
        return "slot#{}".format(self.id)

-    def __eq__(self, other):


I had to remove this so that ScratchSlot(1) != ScratchSlot(1), which is required in order to detect duplicate requested slot IDs when I add all slots to a set.

Sounds fine to me -- I think I previously checked for the id of the object e.g. id(slot1) == id(slot2) for detecting duplicates, but with this change it seems like we can remove this check

jasonpaulos · 2021-08-25T16:51:22Z

pyteal/ir/labelref.py

@@ -0,0 +1,20 @@
+class LabelReference:


This class is basically a pass-by-reference string. It was added so that flattenSubroutines can easily prefix all the labels in a given subroutine in order to differentiate them.

algochoi

It looking good, I really like the recursive test coverage as well. Just minor comments/questions.

algochoi · 2021-08-25T21:54:22Z

pyteal/compiler/flatten.py

+    # from the main routine, and "subX_" (the subroutine label) to the
+    # ones from each subroutine
+
+    mainRoutine = subroutineMapping[None]


This is an interesting trick, using None as a key for a dict.

pyteal/ast/subroutine.py

pyteal/compiler/scratchslots.py

algochoi · 2021-08-25T23:34:54Z

pyteal/ast/scratch.py

    def __str__(self):
        return "slot#{}".format(self.id)

-    def __eq__(self, other):


Sounds fine to me -- I think I previously checked for the id of the object e.g. id(slot1) == id(slot2) for detecting duplicates, but with this change it seems like we can remove this check

pyteal/ast/if_.py

pyteal/ast/return_.py

examples/signature/atomic_swap.teal

pyteal/ast/binaryexpr.py

pyteal/ast/cond.py

pyteal/ast/return_.py

pyteal/ast/subroutine.py

pyteal/compiler/compiler.py

shiqizng · 2021-08-26T15:14:31Z

pyteal/compiler/compiler.py

+    ] = dict()
+    subroutineGraph: Dict[SubroutineDefinition, Set[SubroutineDefinition]] = dict()
+    subroutineBlocks: Dict[Optional[SubroutineDefinition], TealBlock] = dict()
+    compileSubroutine(


maybe this should be named compile() since it's not a compiler specific to subroutines only?

I have mostly been treating the main routine as a special subroutine which gets invoked first (and doesn't have a SubroutineDefinition object). So I think compileSubroutine is an ok name. Would compileRoutine be better?

The only reason I want to avoid compile() is because that's already a builtin Python function: https://docs.python.org/3/library/functions.html#compile

in that case, compileSubroutine is better.

shiqizng · 2021-08-26T15:21:44Z

Jason, can you please elaborate on what has_return() do? It's not always clear to me why it's True/False in some cases. For example, in If(), why is it False when there's no else branch? if self.elseBranch is None: return False

jasonpaulos · 2021-08-27T17:28:44Z

Jason, can you please elaborate what has_return() do? It's not always clear to me why it's True/False in some cases. For example, in If(), why is it False when there's no else branch? if self.elseBranch is None: return False

Yes, I have updated the docstring to be more specific, and added comments to each complex class which overrides it. The docstring is now says "Check if this expression always returns from the current subroutine or program."

That check is needed in order to insert retsub and return ops at the end of a subroutine/main routine if they are not already present. See here:

pyteal/pyteal/compiler/compiler.py

Line 124 in d28ada2

if not ast.has_return():

I thought this was a better solution than requiring every subroutine and main routine to end with a Return statement.

ahangsu

LGTM

ahangsu · 2021-08-26T17:43:03Z

pyteal/compiler/compiler.py

+        self.continueBlocksStack[-1].append(block)
+
+    def exitLoop(self) -> Tuple[List[TealSimpleBlock], List[TealSimpleBlock]]:
+        return (self.breakBlocksStack.pop(), self.continueBlocksStack.pop())


nit: can remove redundant parentheses, i.e., return self.breakBlocksStack.pop(), self.continueBlocksStack.pop() should work.

ahangsu · 2021-08-26T19:43:07Z

pyteal/ast/return_.py

+        if self.value is not None:
+            args.append(self.value)
+
+        return TealBlock.FromOp(options, TealOp(self, op), *args)


nit: can be done as a one-liner:

return TealBlock.FromOp(options, TealOp(self, op), *([] if self.value is None else [self.value]) )

True, but I think I'll stick with the current code since it's a little easier to see what's happening

pyteal/compiler/compiler.py

algochoi

Changes look good to me 👍

ahangsu

LGTM

jasonpaulos added 15 commits August 19, 2021 11:22

Subroutine implementation

c61beaa

Merge branch 'master' into jason/subroutines

1956301

Fix test errors

4543785

Merge branch 'master' into jason/subroutines

334f8e9

Minor improvements

fd86f27

Add AST tests

380d87b

Merge branch 'master' into jason/subroutines

81ef79d

Update new loop expressions and tests

25d9000

Fix mypy & format

2259e3e

Fix OrderedDict type issue

2910587

Fix issue in other file

43e04c2

Add more tests and documentation

b82b61b

Fix validateSlots

445a154

Format code

536192d

Update test names

0aa6432

jasonpaulos marked this pull request as ready for review August 25, 2021 16:19

jasonpaulos commented Aug 25, 2021

View reviewed changes

jasonpaulos requested review from shiqizng, algochoi and ahangsu August 25, 2021 16:51

algochoi reviewed Aug 25, 2021

View reviewed changes

shiqizng reviewed Aug 26, 2021

View reviewed changes

jasonpaulos added 2 commits August 27, 2021 10:19

Respond to feedback

d28ada2

Fix format

3a54057

ahangsu reviewed Aug 27, 2021

View reviewed changes

algochoi self-requested a review August 27, 2021 18:37

algochoi approved these changes Aug 27, 2021

View reviewed changes

ahangsu self-requested a review August 27, 2021 18:39

ahangsu approved these changes Aug 27, 2021

View reviewed changes

jasonpaulos merged commit d922247 into master Aug 27, 2021

jasonpaulos deleted the jason/subroutines branch August 27, 2021 22:24

jasonpaulos mentioned this pull request Aug 31, 2021

v0.8.0 #105

Merged

tzaffi mentioned this pull request Apr 13, 2022

Flake & Make #273

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add subroutine support #99

Add subroutine support #99

jasonpaulos commented Aug 19, 2021 •

edited

Loading

jasonpaulos Aug 25, 2021

algochoi Aug 25, 2021

jasonpaulos Aug 25, 2021 •

edited

Loading

algochoi left a comment

algochoi Aug 25, 2021

algochoi Aug 25, 2021

shiqizng Aug 26, 2021

jasonpaulos Aug 27, 2021

shiqizng Aug 27, 2021

shiqizng commented Aug 26, 2021 •

edited

Loading

jasonpaulos commented Aug 27, 2021 •

edited

Loading

ahangsu left a comment

ahangsu Aug 26, 2021

ahangsu Aug 26, 2021

jasonpaulos Aug 27, 2021

algochoi left a comment

ahangsu left a comment

Add subroutine support #99

Add subroutine support #99

Conversation

jasonpaulos commented Aug 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonpaulos Aug 25, 2021 • edited Loading

Choose a reason for hiding this comment

algochoi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shiqizng commented Aug 26, 2021 • edited Loading

jasonpaulos commented Aug 27, 2021 • edited Loading

ahangsu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

algochoi left a comment

Choose a reason for hiding this comment

ahangsu left a comment

Choose a reason for hiding this comment

jasonpaulos commented Aug 19, 2021 •

edited

Loading

jasonpaulos Aug 25, 2021 •

edited

Loading

shiqizng commented Aug 26, 2021 •

edited

Loading

jasonpaulos commented Aug 27, 2021 •

edited

Loading