Add tail recursion to IR #5228
Closed
Add tail recursion to IR #5228
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisvittal
force-pushed
the
loops
branch
2 times, most recently
from
3807286
to
8eb913b
Compare
|
Spun the wheel got Patrick, would also like @cseed, @tpoterba, @catoverdrive to look if they have time. Thanks! |
|
@patrick-schultz ping |
patrick-schultz
requested changes
Feb 11, 2019
| @typecheck(recur_exprs=expr_any) | ||
| def make_loop(*recur_exprs): | ||
| if len(recur_exprs) != len(exprs): | ||
| raise TypeError('loop and recursion must have the same number of arguments') |
There was a problem hiding this comment.
I would probably make this "Recursive call in loop has wrong number of arguments", and below something like "Type error in recursive call:"
| Returns | ||
| ------- | ||
| :class:`.Expression` | ||
| Result of the loop with `exprs` as inital loop values. |
|
|
||
| for expr in exprs: | ||
| uid = Env.get_uid() | ||
| args.append(construct_variable(uid, expr._type, expr._indices, expr._aggregations)) |
There was a problem hiding this comment.
I find args confusing. How about loop_vars?
This includes
* Python classes
* Parsing
* Interpret
TODOs:
* Emit
* C++ Emit
Also fix some type inferences in Loop and Recur This iteration currently requires `loop` to take its return type as an argument.
|
This fundamentally needs to be rewritten now. I'll be able to fix all the merge conflicts, but it's probably better as a new PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is still work to do here, but it is now complete enough that InterpretSuite can be run properly on a minimal example.
Current TODOs:
Loopand matchingRecurLoopand matchingRecur