add extract combinator to Encore
#271
Conversation
|
the segfaults are not related to the parallel combinators. They are related to a mainstream bug in #274. This PR can be reviewed and merged |
kikofernandez
force-pushed
the
extract-combinator
branch
from
8a6e9bd to
d1a38be
Compare
|
rebased so that it can potentially be merged! |
| in | ||
| print |arr| | ||
| }| | ||
|
|
There was a problem hiding this comment.
return->returns- Maybe say that the runtime will try to choose the one that gets better performance (or are there guarantees?)
- Use
:rather than::to denote the type (in the comment) as this is the Encore syntax - How about an example that prints the contents of the array instead of just the length?
for n in arr print n
There was a problem hiding this comment.
yes.
- Maybe say that the runtime will try to choose the one that gets better performance (or are there guarantees?)
Yes, you are right. Currently it chooses the get operation by default but I envision something more advanced soon
kikofernandez
force-pushed
the
extract-combinator
branch
from
d1a38be to
189c917
Compare
| get test.test_int_and_dec(); | ||
| get test.test_from_string_to_int(); | ||
| get test.test_switch_test(); | ||
| get test.test_passive_object(); |
There was a problem hiding this comment.
What are these tests for? They don't seem to have anything to do with extract.
There was a problem hiding this comment.
these tests are testing that they produce the right value before using the parallel combinators.
testing the tests...
|
The tests are mixing snake_case and camelCase. There's no standard for Encore programs, but at least we should be consistent within the same file. |
| encore_arg_t data; | ||
| struct list_t* next; | ||
| } list_t; | ||
|
|
There was a problem hiding this comment.
If we ever want to reuse the list, it will probably make sense to use a separate struct for the list header (with a pointer to the first node). This makes the list more imperative (no need to save intermediate results when pushing) and makes it possible to differentiate between "no list" (list is NULL) and "empty list" (first-pointer is NULL).
There was a problem hiding this comment.
it could be useful but, at the moment, I would not do a refactoring on something that might or might not happen. if there's a need, we can re-consider
|
My comments are mostly minor! If these are fixed (or rebutted) I think this can be merged. |
kikofernandez
force-pushed
the
extract-combinator
branch
from
189c917 to
b52d5ce
Compare
|
@kikofernandez Are you done reacting to comments? In that case I will merge this later today. |
|
yes, I think so |
add the `extract` combinator to encore. this commit also refactors the tests for the parallel combinators into its own test folder with tests for each of the combinators.
kikofernandez
force-pushed
the
extract-combinator
branch
from
b52d5ce to
219b1b4
Compare
|
added |
|
Good last change. Merging now! |
add `extract` combinator to Encore
extractcombinator to encoreextractcombinator