-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Allow return in tailrec position #14067
Conversation
91053e9
to
a349775
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't create any issues now.
However, to be complete, you'll have to actually recurse in all trees all the time. See this comment:
https://github.com/lampepfl/dotty/blob/a34977551e9e13090e6b5201f7d91879464f991c/compiler/src/dotty/tools/dotc/transform/TailRec.scala#L29-L31
Now any nested tree could contain a recursive call, which was not the case before.
Also, this will need more tests before it's ready to merge. ;)
67d8995
to
4368847
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
There might be concerns about the performance impact, since now checking needs to be done everywhere all the time. So perhaps ask the performance bot?
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/14067/ to see the changes. Benchmarks is based on merging with master (eb75a1a) |
@sjrd could we merge this or do we need more changes or measurements? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The benchmark results look good to me. It's good to go as far as I'm concerned.
Revert "Allow return in tailrec position" #14067
Revert "Allow return in tailrec position" #14067
Implement lampepfl/dotty-feature-requests#261.
Joint work with @ghostbuster91 in the framework of the issue spree.