Skip to content

Commit 8fc59f3

Browse files
authored
refactor!: consistent inout order in borrow array (#2621)
BorrowArray::{borrow, is_borrowed} have had their return types switched such that the array itself is first, matching the input signature. This is more consistent with other ops that both take and return a type. BREAKING CHANGE: BorrowArray::{borrow, is_borrowed} return types have been swapped such that the array is first.
1 parent 228ee58 commit 8fc59f3

File tree

4 files changed

+51
-45
lines changed

4 files changed

+51
-45
lines changed

hugr-core/src/std_extensions/collections/borrow_array.rs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ pub const BORROW_ARRAY_VALUENAME: TypeName = TypeName::new_inline("borrow_array"
4545
/// Reported unique name of the extension
4646
pub const EXTENSION_ID: ExtensionId = ExtensionId::new_unchecked("collections.borrow_arr");
4747
/// Extension version.
48-
pub const VERSION: semver::Version = semver::Version::new(0, 1, 2);
48+
pub const VERSION: semver::Version = semver::Version::new(0, 2, 0);
4949

5050
/// A linear, unsafe, fixed-length collection of values.
5151
///
@@ -153,7 +153,7 @@ impl BArrayUnsafeOpDef {
153153
match self {
154154
Self::borrow => PolyFuncTypeRV::new(
155155
params,
156-
FuncValueType::new(vec![array_ty.clone(), usize_t], vec![elem_ty_var, array_ty]),
156+
FuncValueType::new(vec![array_ty.clone(), usize_t], vec![array_ty, elem_ty_var]),
157157
),
158158
Self::r#return => PolyFuncTypeRV::new(
159159
params,
@@ -172,7 +172,7 @@ impl BArrayUnsafeOpDef {
172172
params,
173173
FuncValueType::new(
174174
vec![array_ty.clone(), usize_t],
175-
vec![crate::extension::prelude::bool_t(), array_ty],
175+
vec![array_ty, crate::extension::prelude::bool_t()],
176176
),
177177
),
178178
}
@@ -644,6 +644,12 @@ pub trait BArrayOpBuilder: GenericArrayOpBuilder {
644644
/// * `input` - The wire representing the array.
645645
/// * `index` - The wire representing the index to get.
646646
///
647+
/// # Returns
648+
///
649+
/// A tuple containing:
650+
/// * The wire representing the updated array with the element marked as borrowed.
651+
/// * The wire representing the borrowed element at the specified index.
652+
///
647653
/// # Errors
648654
///
649655
/// Returns an error if building the operation fails.
@@ -655,10 +661,10 @@ pub trait BArrayOpBuilder: GenericArrayOpBuilder {
655661
index: Wire,
656662
) -> Result<(Wire, Wire), BuildError> {
657663
let op = BArrayUnsafeOpDef::borrow.instantiate(&[size.into(), elem_ty.into()])?;
658-
let [out, arr] = self
664+
let [arr, out] = self
659665
.add_dataflow_op(op.to_extension_op().unwrap(), vec![input, index])?
660666
.outputs_arr();
661-
Ok((out, arr))
667+
Ok((arr, out))
662668
}
663669

664670
/// Adds a borrow array put operation to the dataflow graph.
@@ -746,8 +752,8 @@ pub trait BArrayOpBuilder: GenericArrayOpBuilder {
746752
/// # Returns
747753
///
748754
/// A tuple containing:
749-
/// * The wire representing the boolean result (true if borrowed).
750755
/// * The wire representing the updated array.
756+
/// * The wire representing the boolean result (true if borrowed).
751757
fn add_is_borrowed(
752758
&mut self,
753759
elem_ty: Type,
@@ -756,10 +762,10 @@ pub trait BArrayOpBuilder: GenericArrayOpBuilder {
756762
index: Wire,
757763
) -> Result<(Wire, Wire), BuildError> {
758764
let op = BArrayUnsafeOpDef::is_borrowed.instantiate(&[size.into(), elem_ty.into()])?;
759-
let [is_borrowed, arr] = self
765+
let [arr, is_borrowed] = self
760766
.add_dataflow_op(op.to_extension_op().unwrap(), vec![input, index])?
761767
.outputs_arr();
762-
Ok((is_borrowed, arr))
768+
Ok((arr, is_borrowed))
763769
}
764770
}
765771

@@ -799,7 +805,7 @@ mod test {
799805
let idx1 = builder.add_load_value(ConstUsize::new(11));
800806
let idx2 = builder.add_load_value(ConstUsize::new(11));
801807
let [arr] = builder.input_wires_arr();
802-
let (el, arr_with_take) = builder
808+
let (arr_with_take, el) = builder
803809
.add_borrow_array_borrow(elem_ty.clone(), size, arr, idx1)
804810
.unwrap();
805811
let arr_with_put = builder
@@ -819,7 +825,7 @@ mod test {
819825
DFGBuilder::new(Signature::new(vec![arr_ty.clone()], vec![qb_t()])).unwrap();
820826
let idx = builder.add_load_value(ConstUsize::new(0));
821827
let [arr] = builder.input_wires_arr();
822-
let (el, arr_with_borrowed) = builder
828+
let (arr_with_borrowed, el) = builder
823829
.add_borrow_array_borrow(elem_ty.clone(), size, arr, idx)
824830
.unwrap();
825831
builder
@@ -857,10 +863,10 @@ mod test {
857863
let idx = builder.add_load_value(ConstUsize::new(2));
858864
let [arr] = builder.input_wires_arr();
859865
// Borrow the element at index 2
860-
let (qb, arr_with_borrowed) = builder
866+
let (arr_with_borrowed, qb) = builder
861867
.add_borrow_array_borrow(elem_ty.clone(), size, arr, idx)
862868
.unwrap();
863-
let (_is_borrowed, arr_after_check) = builder
869+
let (arr_after_check, _is_borrowed) = builder
864870
.add_is_borrowed(elem_ty.clone(), size, arr_with_borrowed, idx)
865871
.unwrap();
866872
builder

hugr-llvm/src/extension/collections/borrow_array.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,7 +1579,7 @@ pub fn emit_barray_unsafe_op<'c, H: HugrView<Node = Node>>(
15791579
let elem_addr =
15801580
unsafe { builder.build_in_bounds_gep(elems_ptr, &[offset_index_v], "")? };
15811581
let elem_v = builder.build_load(elem_addr, "")?;
1582-
outputs.finish(builder, [elem_v, array_v])
1582+
outputs.finish(builder, [array_v, elem_v])
15831583
}
15841584
BArrayUnsafeOpDef::r#return => {
15851585
let [array_v, index_v, elem_v] = inputs
@@ -1631,7 +1631,7 @@ pub fn emit_barray_unsafe_op<'c, H: HugrView<Node = Node>>(
16311631
let offset_index_v = ctx.builder().build_int_add(index_v, offset, "")?;
16321632
// let bit = build_is_borrowed_check(ctx, mask_ptr, offset_index_v)?;
16331633
let bit = build_is_borrowed_bit(ctx, mask_ptr, offset_index_v)?;
1634-
outputs.finish(ctx.builder(), [bit.into(), array_v])
1634+
outputs.finish(ctx.builder(), [array_v, bit.into()])
16351635
}
16361636
_ => todo!(),
16371637
}
@@ -2555,7 +2555,7 @@ mod test {
25552555
let mut r = builder.add_load_value(ConstInt::new_u(6, 0).unwrap());
25562556
for &i in indices {
25572557
let i = builder.add_load_value(ConstUsize::new(i));
2558-
let (val, arr) = builder
2558+
let (arr, val) = builder
25592559
.add_borrow_array_borrow(int_ty.clone(), size, array, i)
25602560
.unwrap();
25612561
r = builder.add_iadd(6, r, val).unwrap();
@@ -2570,7 +2570,7 @@ mod test {
25702570
}
25712571
for &i in indices {
25722572
let i = builder.add_load_value(ConstUsize::new(i));
2573-
let (val, arr) = builder
2573+
let (arr, val) = builder
25742574
.add_borrow_array_borrow(int_ty.clone(), size, array, i)
25752575
.unwrap();
25762576
r = builder.add_iadd(6, r, val).unwrap();
@@ -2624,7 +2624,7 @@ mod test {
26242624
let mut r = builder.add_load_value(ConstInt::new_u(6, 0).unwrap());
26252625
for i in 0..size {
26262626
let i = builder.add_load_value(ConstUsize::new(i));
2627-
let (val, arr) = builder
2627+
let (arr, val) = builder
26282628
.add_borrow_array_borrow(int_ty.clone(), size, array, i)
26292629
.unwrap();
26302630
r = builder.add_iadd(6, r, val).unwrap();
@@ -2729,7 +2729,7 @@ mod test {
27292729
let mut r = builder.add_load_value(ConstInt::new_u(6, 0).unwrap());
27302730
for i in 0..size {
27312731
let i = builder.add_load_value(ConstUsize::new(i));
2732-
let (val, arr) = builder
2732+
let (arr, val) = builder
27332733
.add_borrow_array_borrow(int_ty.clone(), size, barray, i)
27342734
.unwrap();
27352735
r = builder.add_iadd(6, r, val).unwrap();
@@ -2786,13 +2786,13 @@ mod test {
27862786
array = builder
27872787
.add_borrow_array_borrow(int_ty.clone(), size, array, i1)
27882788
.unwrap()
2789-
.1;
2789+
.0;
27902790
array = build_pops(&mut builder, int_ty.clone(), size, array, num_pops);
27912791
size -= num_pops;
27922792
array = builder
27932793
.add_borrow_array_borrow(int_ty.clone(), size, array, i2)
27942794
.unwrap()
2795-
.1;
2795+
.0;
27962796
builder
27972797
.add_borrow_array_discard(int_ty.clone(), size, array)
27982798
.unwrap();
@@ -2951,7 +2951,7 @@ mod test {
29512951
);
29522952
let barray = builder.add_load_value(barray);
29532953
let idx = builder.add_load_value(ConstUsize::new(0));
2954-
let (_, barray) = builder
2954+
let (barray, _) = builder
29552955
.add_borrow_array_borrow(int_ty.clone(), size, barray, idx)
29562956
.unwrap();
29572957
let array = builder
@@ -2995,7 +2995,7 @@ mod test {
29952995
);
29962996
let barray = builder.add_load_value(barray);
29972997
let idx1 = builder.add_load_value(ConstUsize::new(1));
2998-
let (_, barray) = builder
2998+
let (barray, _) = builder
29992999
.add_borrow_array_borrow(int_ty.clone(), size, barray, idx1)
30003000
.unwrap();
30013001

@@ -3004,7 +3004,7 @@ mod test {
30043004
[idx0, idx1]
30053005
.iter()
30063006
.fold((barray, Vec::new()), |(arr, mut bools), idx| {
3007-
let (b, arr) = builder
3007+
let (arr, b) = builder
30083008
.add_is_borrowed(int_ty.clone(), size, arr, *idx)
30093009
.unwrap();
30103010
bools.push(b);

hugr-py/src/hugr/std/_json_defs/collections/borrow_arr.json

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "0.1.2",
2+
"version": "0.2.0",
33
"name": "collections.borrow_arr",
44
"types": {
55
"borrow_array": {
@@ -69,11 +69,6 @@
6969
}
7070
],
7171
"output": [
72-
{
73-
"t": "V",
74-
"i": 1,
75-
"b": "A"
76-
},
7772
{
7873
"t": "Opaque",
7974
"extension": "collections.borrow_arr",
@@ -97,6 +92,11 @@
9792
}
9893
],
9994
"bound": "A"
95+
},
96+
{
97+
"t": "V",
98+
"i": 1,
99+
"b": "A"
100100
}
101101
]
102102
}
@@ -539,11 +539,6 @@
539539
}
540540
],
541541
"output": [
542-
{
543-
"t": "Sum",
544-
"s": "Unit",
545-
"size": 2
546-
},
547542
{
548543
"t": "Opaque",
549544
"extension": "collections.borrow_arr",
@@ -567,6 +562,11 @@
567562
}
568563
],
569564
"bound": "A"
565+
},
566+
{
567+
"t": "Sum",
568+
"s": "Unit",
569+
"size": 2
570570
}
571571
]
572572
}

specification/std_extensions/collections/borrow_arr.json

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "0.1.2",
2+
"version": "0.2.0",
33
"name": "collections.borrow_arr",
44
"types": {
55
"borrow_array": {
@@ -69,11 +69,6 @@
6969
}
7070
],
7171
"output": [
72-
{
73-
"t": "V",
74-
"i": 1,
75-
"b": "A"
76-
},
7772
{
7873
"t": "Opaque",
7974
"extension": "collections.borrow_arr",
@@ -97,6 +92,11 @@
9792
}
9893
],
9994
"bound": "A"
95+
},
96+
{
97+
"t": "V",
98+
"i": 1,
99+
"b": "A"
100100
}
101101
]
102102
}
@@ -539,11 +539,6 @@
539539
}
540540
],
541541
"output": [
542-
{
543-
"t": "Sum",
544-
"s": "Unit",
545-
"size": 2
546-
},
547542
{
548543
"t": "Opaque",
549544
"extension": "collections.borrow_arr",
@@ -567,6 +562,11 @@
567562
}
568563
],
569564
"bound": "A"
565+
},
566+
{
567+
"t": "Sum",
568+
"s": "Unit",
569+
"size": 2
570570
}
571571
]
572572
}

0 commit comments

Comments
 (0)