From 66244b6e5b505f692c7e9a41bdc061c77fd1284d Mon Sep 17 00:00:00 2001
From: Tom French <15848336+TomAFrench@users.noreply.github.com>
Date: Tue, 9 Jul 2024 12:59:06 +0100
Subject: [PATCH] fix: prevent `no_predicates` from removing predicates in
 calling function (#5452)

# Description

## Problem\*

Resolves #5435

## Summary\*

This PR caches the current `side_effects_enabled` value on the
`PerFunctionContext` and restores it after inlining another function.
This ensures that any `EnableSideEffects` instructions from the inlined
function do not leak out into the calling function.

## 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.
---
 .../noirc_evaluator/src/ssa/opt/inlining.rs    | 18 ++++++++++++++++++
 .../regression_5435/Nargo.toml                 |  7 +++++++
 .../regression_5435/Prover.toml                |  2 ++
 .../regression_5435/src/main.nr                | 18 ++++++++++++++++++
 4 files changed, 45 insertions(+)
 create mode 100644 test_programs/execution_success/regression_5435/Nargo.toml
 create mode 100644 test_programs/execution_success/regression_5435/Prover.toml
 create mode 100644 test_programs/execution_success/regression_5435/src/main.nr

diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs
index c30bc884535..09802713363 100644
--- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs
+++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs
@@ -475,6 +475,8 @@ impl<'function> PerFunctionContext<'function> {
     /// Inline each instruction in the given block into the function being inlined into.
     /// This may recurse if it finds another function to inline if a call instruction is within this block.
     fn inline_block_instructions(&mut self, ssa: &Ssa, block_id: BasicBlockId) {
+        let mut side_effects_enabled: Option<ValueId> = None;
+
         let block = &self.source_function.dfg[block_id];
         for id in block.instructions() {
             match &self.source_function.dfg[*id] {
@@ -482,12 +484,28 @@ impl<'function> PerFunctionContext<'function> {
                     Some(func_id) => {
                         if self.should_inline_call(ssa, func_id) {
                             self.inline_function(ssa, *id, func_id, arguments);
+
+                            // This is only relevant during handling functions with `InlineType::NoPredicates` as these
+                            // can pollute the function they're being inlined into with `Instruction::EnabledSideEffects`,
+                            // resulting in predicates not being applied properly.
+                            //
+                            // Note that this doesn't cover the case in which there exists an `Instruction::EnabledSideEffects`
+                            // within the function being inlined whilst the source function has not encountered one yet.
+                            // In practice this isn't an issue as the last `Instruction::EnabledSideEffects` in the
+                            // function being inlined will be to turn off predicates rather than to create one.
+                            if let Some(condition) = side_effects_enabled {
+                                self.context.builder.insert_enable_side_effects_if(condition);
+                            }
                         } else {
                             self.push_instruction(*id);
                         }
                     }
                     None => self.push_instruction(*id),
                 },
+                Instruction::EnableSideEffects { condition } => {
+                    side_effects_enabled = Some(self.translate_value(*condition));
+                    self.push_instruction(*id);
+                }
                 _ => self.push_instruction(*id),
             }
         }
diff --git a/test_programs/execution_success/regression_5435/Nargo.toml b/test_programs/execution_success/regression_5435/Nargo.toml
new file mode 100644
index 00000000000..6affc423b7a
--- /dev/null
+++ b/test_programs/execution_success/regression_5435/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "regression_5435"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.31.0"
+
+[dependencies]
diff --git a/test_programs/execution_success/regression_5435/Prover.toml b/test_programs/execution_success/regression_5435/Prover.toml
new file mode 100644
index 00000000000..f3e2bbe32e7
--- /dev/null
+++ b/test_programs/execution_success/regression_5435/Prover.toml
@@ -0,0 +1,2 @@
+input = "0"
+enable = false
diff --git a/test_programs/execution_success/regression_5435/src/main.nr b/test_programs/execution_success/regression_5435/src/main.nr
new file mode 100644
index 00000000000..65f13c5b201
--- /dev/null
+++ b/test_programs/execution_success/regression_5435/src/main.nr
@@ -0,0 +1,18 @@
+fn main(input: Field, enable: bool) {
+    if enable {
+        let hash = no_predicate_function(input);
+        // `EnableSideEffects` instruction from above instruction leaks out and removes the predicate from this call,
+        // resulting in execution failure.
+        fail(hash);
+    }
+}
+
+#[no_predicates]
+fn no_predicate_function(enable: Field) -> Field {
+    // if-statement ensures that an `EnableSideEffects` instruction is emitted.
+    if enable == 0 { 1 } else { 0 }
+}
+
+unconstrained fn fail(_: Field) {
+    assert(false);
+}