-
Notifications
You must be signed in to change notification settings - Fork 442
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
[sc-525] Float combinators not extracting some terms #245
[sc-525] Float combinators not extracting some terms #245
Conversation
This pull request has been linked to Shortcut Story #525: float_combinators not extracting some terms. |
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.
Can you add a test for recursiveness/combinators in other positions? Like in different application positions, in adt matches and num matches, both in arms and as argument. In lists, tuples, sups, lets, dups, etc.
Also some positions where it should not get extracted, like the body of a lambda (eg foo = @a @b @c (foo a)
)
Also, update the documentation for this pass to explain the new logic, including the exceptions where we don't float.
term_info.merge_scope(arg_scope); | ||
|
||
detach | ||
/// A term is a supercombinator if it is a lambda abstraction without free variables. |
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 is not quite the definition of a supercombinator. This is just a lambda abstraction that is also a combinator, which is all we need here in this case. To be a supercombinator we require that it is of the form
@x1 @x2 ... @xN (Arg1 Arg2 ... ArgM)
Where Arg1-M are also supercombinators, constants or one of the x1-N.
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.
Remember to update the documentation
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.
Great job
No description provided.