From af1eec6446fd49acf8fea02484b88f3d0283b340 Mon Sep 17 00:00:00 2001
From: Maxim Vezenov <mvezenov@gmail.com>
Date: Wed, 18 Dec 2024 09:34:51 -0500
Subject: [PATCH] fix: Remove all non determinism (#6852)

---
 .../src/brillig/brillig_ir/artifact.rs              |  8 ++++----
 compiler/noirc_evaluator/src/ssa/opt/unrolling.rs   |  6 ++++--
 compiler/noirc_frontend/src/ast/statement.rs        | 13 ++++++-------
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs
index be4a6b84bc..4a8e1cdcad 100644
--- a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs
+++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs
@@ -27,7 +27,7 @@ pub(crate) struct GeneratedBrillig<F> {
     pub(crate) locations: BTreeMap<OpcodeLocation, CallStack>,
     pub(crate) error_types: BTreeMap<ErrorSelector, ErrorType>,
     pub(crate) name: String,
-    pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
+    pub(crate) procedure_locations: BTreeMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
 }
 
 #[derive(Default, Debug, Clone)]
@@ -61,13 +61,13 @@ pub(crate) struct BrilligArtifact<F> {
     /// This is created as artifacts are linked together and allows us to determine
     /// which opcodes originate from reusable procedures.s
     /// The range is inclusive for both start and end opcode locations.
-    pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
+    pub(crate) procedure_locations: BTreeMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
 }
 
 /// A pointer to a location in the opcode.
 pub(crate) type OpcodeLocation = usize;
 
-#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
 pub(crate) enum LabelType {
     /// Labels for the entry point bytecode
     Entrypoint,
@@ -97,7 +97,7 @@ impl std::fmt::Display for LabelType {
 ///
 /// It is assumed that an entity will keep a map
 /// of labels to Opcode locations.
-#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
 pub(crate) struct Label {
     pub(crate) label_type: LabelType,
     pub(crate) section: Option<usize>,
diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs
index 2a27223619..ab4256197b 100644
--- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs
+++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs
@@ -18,6 +18,8 @@
 //!
 //! When unrolling ACIR code, we remove reference count instructions because they are
 //! only used by Brillig bytecode.
+use std::collections::BTreeSet;
+
 use acvm::{acir::AcirField, FieldElement};
 use im::HashSet;
 
@@ -117,7 +119,7 @@ pub(super) struct Loop {
     back_edge_start: BasicBlockId,
 
     /// All the blocks contained within the loop, including `header` and `back_edge_start`.
-    pub(super) blocks: HashSet<BasicBlockId>,
+    pub(super) blocks: BTreeSet<BasicBlockId>,
 }
 
 pub(super) struct Loops {
@@ -238,7 +240,7 @@ impl Loop {
         back_edge_start: BasicBlockId,
         cfg: &ControlFlowGraph,
     ) -> Self {
-        let mut blocks = HashSet::default();
+        let mut blocks = BTreeSet::default();
         blocks.insert(header);
 
         let mut insert = |block, stack: &mut Vec<BasicBlockId>| {
diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs
index 0ce21f9f79..bd65fc3337 100644
--- a/compiler/noirc_frontend/src/ast/statement.rs
+++ b/compiler/noirc_frontend/src/ast/statement.rs
@@ -1,5 +1,4 @@
 use std::fmt::Display;
-use std::sync::atomic::{AtomicU32, Ordering};
 
 use acvm::acir::AcirField;
 use acvm::FieldElement;
@@ -841,10 +840,9 @@ impl ForRange {
         block: Expression,
         for_loop_span: Span,
     ) -> Statement {
-        /// Counter used to generate unique names when desugaring
-        /// code in the parser requires the creation of fresh variables.
-        /// The parser is stateless so this is a static global instead.
-        static UNIQUE_NAME_COUNTER: AtomicU32 = AtomicU32::new(0);
+        // Counter used to generate unique names when desugaring
+        // code in the parser requires the creation of fresh variables.
+        let mut unique_name_counter: u32 = 0;
 
         match self {
             ForRange::Range(..) => {
@@ -855,7 +853,8 @@ impl ForRange {
                 let start_range = ExpressionKind::integer(FieldElement::zero());
                 let start_range = Expression::new(start_range, array_span);
 
-                let next_unique_id = UNIQUE_NAME_COUNTER.fetch_add(1, Ordering::Relaxed);
+                let next_unique_id = unique_name_counter;
+                unique_name_counter += 1;
                 let array_name = format!("$i{next_unique_id}");
                 let array_span = array.span;
                 let array_ident = Ident::new(array_name, array_span);
@@ -888,7 +887,7 @@ impl ForRange {
                 }));
                 let end_range = Expression::new(end_range, array_span);
 
-                let next_unique_id = UNIQUE_NAME_COUNTER.fetch_add(1, Ordering::Relaxed);
+                let next_unique_id = unique_name_counter;
                 let index_name = format!("$i{next_unique_id}");
                 let fresh_identifier = Ident::new(index_name.clone(), array_span);