Skip to content

Commit 5b1c896

Browse files
authored
fix(ssa): RC correctness issue (#6134)
# Description ## Problem\* Resolves #6123 ## Summary\* Still a draft as I want to test it a bit more and am not fully sure if it is the best solution. ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
1 parent 684b6cc commit 5b1c896

4 files changed

Lines changed: 54 additions & 1 deletion

File tree

compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,10 @@ impl<'a> FunctionContext<'a> {
732732
let element_types = Self::convert_type(element_type);
733733
values.map_both(element_types, |value, element_type| {
734734
let reference = value.eval_reference();
735+
// Reference counting in brillig relies on us incrementing reference
736+
// counts when arrays/slices are constructed or indexed.
737+
// Thus, if we dereference an lvalue which happens to be array/slice we should increment its reference counter.
738+
self.builder.increment_array_reference_count(reference);
735739
self.builder.insert_load(reference, element_type).into()
736740
})
737741
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[package]
2+
name = "brillig_rc_regression_6123"
3+
type = "bin"
4+
authors = [""]
5+
compiler_version = ">=0.34.0"
6+
7+
[dependencies]
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
struct Builder {
2+
note_hashes: BoundedVec<Field, 2>,
3+
nullifiers: BoundedVec<Field, 2>,
4+
}
5+
6+
impl Builder {
7+
fn append_note_hashes_with_logs(&mut self, num_note_hashes: u32) {
8+
let index_offset = self.note_hashes.len();
9+
for i in 0..self.note_hashes.max_len() {
10+
if i < num_note_hashes {
11+
self.add_new_note_hash((index_offset + i) as Field);
12+
}
13+
}
14+
}
15+
16+
fn add_new_note_hash(&mut self, value: Field) {
17+
self.note_hashes.push(value);
18+
}
19+
}
20+
21+
fn swap_items<T, let N: u32>(vec: &mut BoundedVec<T, N>, from_index: u32, to_index: u32) {
22+
let tmp = vec.storage[from_index];
23+
vec.storage[from_index] = vec.storage[to_index];
24+
vec.storage[to_index] = tmp;
25+
}
26+
27+
unconstrained fn main() {
28+
let mut builder = Builder { note_hashes: BoundedVec::new(), nullifiers: BoundedVec::new() };
29+
30+
builder.append_note_hashes_with_logs(2);
31+
builder.nullifiers.storage[1] = 27;
32+
// Get ordered items before shuffling.
33+
let note_hashes = builder.note_hashes.storage;
34+
let original_first_note_hash = note_hashes[0];
35+
// Shuffle.
36+
swap_items(&mut builder.note_hashes, 1, 0);
37+
38+
for i in 0..1 {
39+
assert_eq(note_hashes[i], original_first_note_hash);
40+
}
41+
}

tooling/debugger/ignored-tests.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ is_unconstrained
44
macros
55
references
66
regression_4709
7-
reference_only_used_as_alias
7+
reference_only_used_as_alias
8+
brillig_rc_regression_6123

0 commit comments

Comments
 (0)