feat: Type check array subscripts #420
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/arrays #420 +/- ##
===============================================
- Coverage 92.48% 92.46% -0.02%
===============================================
Files 48 48
Lines 5178 5232 +54
===============================================
+ Hits 4789 4838 +49
- Misses 389 394 +5 ☔ View full report in Codecov by Sentry. |
mark-koch
force-pushed
the
array/checking
branch
from
c3081a3 to
db6698b
Compare
| 23: def test(c: MyImmutableContainer) -> MyImmutableContainer: | ||
| 24: foo(c[0]) | ||
| ^^^^ | ||
| GuppyTypeError: Expression of type `MyImmutableContainer` is not assignable since it does not implement the `__setitem__` method |
There was a problem hiding this comment.
The error message seems confusing here as the effect of c[0] is not an assignment. To me it reads more like __getitem__. Isn't the error really because foo consumes an inout and doesn't return it back?
Assuming that is allowed if __setitem__ is defined for the container, perhaps the error messaging needs to be improved here. Something about foo requiring an assignment that's not implemented by the container.
There was a problem hiding this comment.
The problem is that once the inout value has been given back by foo, we cannot assign it back to c[0] since c doesn't implement __setitem__.
But you're right, the current message is misleading. I'll update it 👍
There was a problem hiding this comment.
Ah, that explains the reassignment, certainly better error message will help here.
This is the feature branch for array indexing. Tracked by #253. The goal is to allow array indexing in `@inout` positions that ignores linearity constraints in the indices: ```python qs: array[qubit, 42] cx(qs[0], qs[1]) # Ok cx(qs[0], qs[0]) # Compiles, but will panic at runtime q = qs[0] # Error: Indexing only allowed in inout position ``` This is achieved by lowering `array[qubit]` to `array[qubit | None]` and making `array.__getitem__` and `array.__setitem__` swap in `None` whenever a qubit is taken out or put back in. The functions `array.__getitem__` and `array.__setitem__` take the array as `@inout`, so it suffices to apply the following desugaring: ```python cx(qs[expr1], qs[expr2]) # Desugars to idx1 = expr1 idx2 = expr2 tmp1 = qs.__getitem__(idx1) tmp2 = qs.__getitem__(idx2) cx(tmp1, tmp2) qs.__setitem__(tmp1, idx1) qs.__setitem__(tmp2, idx2) ``` This also means that arrays containing structs might behave slightly unexpectedly: ```python @guppy.struct class QubitPair: q1: qubit q2: qubit ps: array[QubitPair, 42] cx(ps[0].q1, ps[0].q2) # Panics at runtime :( # Since it desugars to idx1 = 0 idx2 = 0 tmp1 = ps.__getitem__(idx1) tmp2 = ps.__getitem__(idx2) # Panic: Struct at index 0 has already been replaced with None cx(tmp1.q1, tmp2.q2) ... ``` To solve this, we need to change the Hugr lowering of arrays to structually replace any occurrence of `qubit` with `qubit | None`, i.e. instead of doing `array[QubitPair | None]`, we would need to do `array[tuple[qubit | None, qubit | None]]`. I'll leave this to a future PR in case we are interested in that. The following feature PRs target this branch: * #420 * #421 * #422 * #447
Closes #416. See #415 for context.
SubscriptAccessplace that will be used to track array subscripts during linearity checking__setitem__method