fix!: Change array result ops signature to return array result #888
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #888 +/- ##
==========================================
+ Coverage 80.66% 80.69% +0.02%
==========================================
Files 88 88
Lines 10711 10731 +20
Branches 10437 10457 +20
==========================================
+ Hits 8640 8659 +19
Misses 1492 1492
- Partials 579 580 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I don't know the downstream implications of making breaking changes to tket2, but this seems necessary and I'm assuming other people (e.g. Seyon) know this is required. There's already a breaking change in main anyways #854
So LGTM!
| PolyFuncType::new( | ||
| [vec![TypeParam::String], self.type_params()].concat(), | ||
| Signature::new(self.arg_type(), vec![self.arg_type()]), | ||
| ) |
There was a problem hiding this comment.
Remove the vec![] for consistency (handled by Into).
Consider also just defining let output = if ... else ...; and then moving the shared code for the returned value after the conditional. Keeps the branched code shorter and removes duplication.
| ) | |
| Signature::new(self.arg_type(), self.arg_type()), |
There was a problem hiding this comment.
wrong line, this edit is for 168
| [vec![TypeParam::String], self.type_params()].concat(), | ||
| Signature::new(self.arg_type(), type_row![]), | ||
| if self.is_array_result_op() { | ||
| // Arrays need to be returned to preserve no-implicit copy guarantees. |
There was a problem hiding this comment.
There is not really a "no-implicit copy guarantee" in tket2 -- that's a guppy thing, right? I'm not sure how to phrase this comment in a way that makes sense for tket2. Maybe something along the lines of "Do not consume input arrays to allow linearity"?
tatiana-s
changed the title
Needed for CQCL/guppylang#981
BREAKING CHANGE:
ResultOpDef::ArrBool,ResultOpDef::ArrInt,ResultOpDef::ArrUIntandResultOpDef::ArrF64signatures now return array results