(PUP-2755) Move iterative functions to new api#2765
(PUP-2755) Move iterative functions to new api#2765zaphod42 merged 17 commits intopuppetlabs:masterfrom
Conversation
|
CLA signed by all contributors. |
These functions are just stubs that raises an error, and they are no longer needed.
This moves all the iterative functions to the 4x function API. Functions are now simpler to read. A shared utility module was created for repeated asserts. While testing it was clear that it is difficult to use Puppet.lookup(:loaders), and the evaluator was changed to get the loaders via the compiler instead. The evaluator tests are changed to make use of the simplified handling of the loaders. The tests for the functions that have moved, were also moved, but are unchanged in what they test.
This adds tests that local and match scope handling is corrent and does not leak variable bindings.
This fixes merge issues that were not automatically done due to files being moved and changed structurally. The support for captures_rest was lost. This is now reinstated in the shared IterativeSupport module and in reduce and slice where the semantics of "serving_size" are slightly different.
There was a problem hiding this comment.
I don't think this is needed
This fixes comments from review. It also removes the special handling of the iterative functions (special case to avoid having their arguments transformed) as this is no longer needed.
There was a problem hiding this comment.
Is it possible to specify the signature for the callable? Something like Callable[Object, 1, 2] I think would do the trick. Then it can probably get rid of the assertion of the serving size and simply work from the defined signature.
Same comment applies to the other functions.
There was a problem hiding this comment.
Turns out that using a signature of Callable[Object, 1, 2] doesn't work because of the support for using captures rest arguments. Specifying |*$x| { ... } as the block results in a signature of Callable[Object] which isn't assignable to Callable[Object, 1, 2]. If we widened Callable[Object, 1, 2] to be able to accept Callable[Object] then it would just end up at Callable[Object], which is the default for required_block_param anyway and all of the checks would need to be put back in.
There was a problem hiding this comment.
Ok, this is a bit more of a discussion than I want to have here. I've opened PUP-2794 to track it. I think we should drop the special support for |*$x|, however.
3 participants
This moves the iterative functions each, map, reduce, filter, and slice to the new function.
A utility module was added for shared behavior (included in the functions).
This also removes the function stubs for old / renamed iterative functions as these stubs have played out
their role, and it seemed pointless to move them to the new API.