-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Array comprehension #613
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #613 +/- ##
==========================================
+ Coverage 92.48% 92.75% +0.26%
==========================================
Files 66 66
Lines 7549 7658 +109
==========================================
+ Hits 6982 7103 +121
+ Misses 567 555 -12 ☔ View full report in Codecov by Sentry. |
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.
👍
guppylang/definition/custom.py
Outdated
|
||
expr, res_ty = self.synthesize(args) | ||
subst, _ = check_type_against(res_ty, ty, self.node) | ||
return expr, subst |
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.
not clear to me why this change is related here
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 was added in #610. Github showed the diff because of a merge conflict after the base branch changed, sorry 😅
guppylang/prelude/builtins.py
Outdated
@@ -559,6 +561,27 @@ def __len__(self: array[L, n]) -> int: ... | |||
def __new__(): ... | |||
|
|||
|
|||
@guppy.extend_type(sized_iter_type_def) | |||
class SizedIter: | |||
"""A wrapper around an iterator type `T` promising that the iterator will yield |
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.
"""A wrapper around an iterator type `T` promising that the iterator will yield | |
"""A wrapper around an iterator type `L` promising that the iterator will yield |
?
guppylang/prelude/builtins.py
Outdated
@guppy.custom(NoopCompiler()) | ||
def unwrap_iter(self: "SizedIter[L, n]" @ owned) -> L: | ||
"""Extracts the actual iterator.""" | ||
|
||
@guppy.custom(NoopCompiler()) | ||
def __iter__(self: "SizedIter[L, n]" @ owned) -> L: | ||
"""Extracts the actual iterator.""" |
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.
what's the difference between these? (is there one?)
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.
It's the same, I thought we might want to have a non-dunder method to get the wrapped iterator?
Refactor the list comprehension logic so we can also use it for array comprehensions
* Refactor compilation of list comprehensions so we can use the same logic for array comprehensions. The only difference is that lists call `push` to add the next element to the accumulator whereas arrays use `set` and update a counter. This is now implemented via a `build_update` hook provided to `_compile_generators` * Store checked `Globals` inside the compilation context so we can invoke Guppy methods by name while building Hugr
if node.elt_ty.linear | ||
else node.elt_ty.to_hugr() | ||
) | ||
# See https://github.com/CQCL/guppylang/issues/629 |
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.
not sure about the relevance of this link?
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 to explain why we use Option
for both linear and classical arrays
Adds array comprehensions of the form
array(i for i in range(10))
.This is the base PR that adds new AST nodes for array comprehensions and general generator expressions. Checking and lowering logic follows in subsequent PRs targetting this branch:
Closes #612