Skip to content

Commit

Permalink
fix: array get/set only has side effects with constant indices
Browse files Browse the repository at this point in the history
  • Loading branch information
TomAFrench committed Feb 1, 2024
1 parent 446a51d commit 7f0189c
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
17 changes: 14 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,20 @@ impl Instruction {
| Store { .. }
| EnableSideEffects { .. }
| IncrementRc { .. }
| RangeCheck { .. }
| ArrayGet { .. }
| ArraySet { .. } => true,
| RangeCheck { .. } => true,

ArrayGet { array, index } | ArraySet { array, index, .. } => {
// This is janky but we only use the side effects flag to check that we're not accessing past the end of the array

Check warning on line 291 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (janky)
// unconditionally to provide a compiler error.
let array_len = if let Value::Array { array, .. } = &dfg[*array] {
array.len() as u128
} else {
return false;
};
let index_const = dfg.get_numeric_constant(*index);

index_const.map_or(false, |index| index.to_u128() >= array_len)
}

// Some `Intrinsic`s have side effects so we must check what kind of `Call` this is.
Call { func, .. } => match dfg[*func] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl Context {
// before we insert this new instruction.
if instruction.has_side_effects(&function.dfg) {
if let Some(enable_side_effects_instruction_id) =
last_side_effects_enabled_instruction
last_side_effects_enabled_instruction.take()
{
new_instructions.push(enable_side_effects_instruction_id);
}
Expand Down

0 comments on commit 7f0189c

Please sign in to comment.