Merge branch 'master' into mv/brillig-opcode-docs

Author: vezenovm

acvm-repo/acvm/src/compiler/transformers/csat.rs

@@ -10,9 +10,9 @@ use indexmap::IndexMap;
 pub const MIN_EXPRESSION_WIDTH: usize = 3;
 
 /// A transformer which processes any [`Expression`]s to break them up such that they
-/// fit within the [`ProofSystemCompiler`][crate::ProofSystemCompiler]'s width.
+/// fit within the backend's width.
 ///
-/// This transformer is only used when targeting the [`Bounded`][crate::ExpressionWidth::Bounded] configuration.
+/// This transformer is only used when targeting the [`Bounded`][acir::circuit::ExpressionWidth::Bounded] configuration.
 ///
 /// This is done by creating intermediate variables to hold partial calculations and then combining them
 /// to calculate the original expression.

acvm-repo/acvm/src/pwg/brillig.rs

@@ -35,7 +35,7 @@ pub struct BrilligSolver<'b, F, B: BlackBoxFunctionSolver<F>> {
 }
 
 impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
-    /// Assigns the zero value to all outputs of the given [`Brillig`] bytecode.
+    /// Assigns the zero value to all outputs of a given [brillig call][acir::circuit::opcodes::Opcode::BrilligCall].
     pub(super) fn zero_out_brillig_outputs(
         initial_witness: &mut WitnessMap<F>,
         outputs: &[BrilligOutputs],

compiler/noirc_arena/src/lib.rs

@@ -9,22 +9,11 @@ use std::fmt;
 pub struct Index(usize);
 
 impl Index {
-    #[cfg(test)]
-    pub fn test_new(index: usize) -> Index {
-        Self(index)
-    }
-
     /// Return a dummy index (max value internally).
     /// This should be avoided over `Option<Index>` if possible.
     pub fn dummy() -> Self {
         Self(usize::MAX)
     }
-
-    /// Return the zeroed index. This is unsafe since we don't know
-    /// if this is a valid index for any particular map yet.
-    pub fn unsafe_zeroed() -> Self {
-        Self(0)
-    }
 }
 
 impl fmt::Display for Index {

compiler/noirc_evaluator/src/acir/mod.rs

@@ -2827,7 +2827,7 @@ impl<'a> Context<'a> {
         Ok(())
     }
 
-    /// Convert a Vec<AcirVar> into a Vec<AcirValue> using the given result ids.
+    /// Convert a `Vec<AcirVar>` into a `Vec<AcirValue>` using the given result ids.
     /// If the type of a result id is an array, several acir vars are collected into
     /// a single AcirValue::Array of the same length.
     /// If the type of a result id is a slice, the slice length must precede it and we can

compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs

@@ -260,7 +260,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
         self.deallocate_register(index_at_end_of_array);
     }
 
-    /// Converts a BrilligArray (pointer to [RC, ...items]) to a HeapArray (pointer to [items])
+    /// Converts a BrilligArray (pointer to `[RC, ...items]`) to a HeapArray (pointer to `[items]`)
     pub(crate) fn codegen_brillig_array_to_heap_array(&mut self, array: BrilligArray) -> HeapArray {
         let heap_array = HeapArray { pointer: self.allocate_register(), size: array.size };
         self.codegen_usize_op(array.pointer, heap_array.pointer, BrilligBinaryOp::Add, 1);

compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs

@@ -25,7 +25,7 @@ use super::{
 impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<F, Registers> {
     /// Processes a binary instruction according `operation`.
     ///
-    /// This method will compute lhs <operation> rhs
+    /// This method will compute lhs `<operation>` rhs
     /// and store the result in the `result` register.
     pub(crate) fn binary_instruction(
         &mut self,

compiler/noirc_evaluator/src/ssa/ir/dfg.rs

@@ -32,7 +32,7 @@ use serde_with::serde_as;
 #[serde_as]
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
 pub(crate) struct DataFlowGraph {
-    /// Runtime of the [Function] that owns this [DataFlowGraph].
+    /// Runtime of the [function][super::function::Function] that owns this [DataFlowGraph].
     /// This might change during the `runtime_separation` pass where
     /// ACIR functions are cloned as Brillig functions.
     runtime: RuntimeType,

compiler/noirc_evaluator/src/ssa/ir/map.rs

@@ -9,7 +9,7 @@ use thiserror::Error;
 
 /// A unique ID corresponding to a value of type T.
 /// This type can be used to retrieve a value of type T from
-/// either a DenseMap<T> or SparseMap<T>.
+/// either a `DenseMap<T>` or `SparseMap<T>`.
 ///
 /// Note that there is nothing in an Id binding it to a particular
 /// DenseMap or SparseMap. If an Id was created to correspond to one
@@ -123,7 +123,7 @@ pub(crate) enum IdDisplayFromStrErr {
     InvalidId(String),
 }
 
-/// The implementation of display and FromStr allows serializing and deserializing an Id<T> to a string.
+/// The implementation of display and FromStr allows serializing and deserializing an `Id<T>` to a string.
 /// This is useful when used as key in a map that has to be serialized to JSON/TOML.
 impl FromStr for Id<super::basic_block::BasicBlock> {
     type Err = IdDisplayFromStrErr;

compiler/noirc_evaluator/src/ssa/ir/types.rs

@@ -132,7 +132,7 @@ impl Type {
         Type::unsigned(8)
     }
 
-    /// Creates the str<N> type, of the given length N
+    /// Creates the `str<N>` type, of the given length N
     pub(crate) fn str(length: u32) -> Type {
         Type::Array(Arc::new(vec![Type::char()]), length)
     }

compiler/noirc_evaluator/src/ssa/ir/value.rs

@@ -15,7 +15,7 @@ use super::{
 pub(crate) type ValueId = Id<Value>;
 
 /// Value is the most basic type allowed in the IR.
-/// Transition Note: A Id<Value> is similar to `NodeId` in our previous IR.
+/// Transition Note: A `Id<Value>` is similar to `NodeId` in our previous IR.
 #[derive(Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize)]
 pub(crate) enum Value {
     /// This value was created due to an instruction

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs

@@ -13,9 +13,8 @@
 //! without the need for multiple passes.
 //!
 //! Other passes perform a certain amount of constant folding automatically as they insert instructions
-//! into the [`DataFlowGraph`] but this pass can become needed if [`DataFlowGraph::set_value`] or
-//! [`DataFlowGraph::set_value_from_id`] are used on a value which enables instructions dependent on the value to
-//! now be simplified.
+//! into the [`DataFlowGraph`] but this pass can become needed if [`DataFlowGraph::set_value_from_id`]
+//! is used on a value which enables instructions dependent on the value to now be simplified.
 //!
 //! This is the only pass which removes duplicated pure [`Instruction`]s however and so is needed when
 //! different blocks are merged, i.e. after the [`flatten_cfg`][super::flatten_cfg] pass.

compiler/noirc_evaluator/src/ssa/opt/inlining/inline_info.rs

@@ -41,7 +41,7 @@ pub(crate) type InlineInfos = BTreeMap<FunctionId, InlineInfo>;
 ///  - main
 ///  - Any Brillig function called from Acir
 ///  - Some Brillig functions depending on aggressiveness and some metrics
-///  - Any Acir functions with a [fold inline type][InlineType::Fold],
+///  - Any Acir functions with a [fold inline type][noirc_frontend::monomorphization::ast::InlineType],
 ///
 /// The returned `InlineInfos` won't have every function in it, only the ones which the algorithm visited.
 pub(crate) fn compute_inline_infos(

compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs

@@ -2,12 +2,13 @@
 //! instructions such that they cover the minimum number of instructions possible.
 //!
 //! The pass works as follows:
-//! - Insert instructions until an [Instruction::EnableSideEffectsIf] is encountered, save this [InstructionId].
+//! - Insert instructions until an [Instruction::EnableSideEffectsIf] is encountered, save this [InstructionId][crate::ssa::ir::instruction::InstructionId].
 //! - Continue inserting instructions until either
-//!     - Another [Instruction::EnableSideEffectsIf] is encountered, if so then drop the previous [InstructionId] in favour
+//!     - Another [Instruction::EnableSideEffectsIf] is encountered, if so then drop the previous [InstructionId][crate::ssa::ir::instruction::InstructionId] in favour
 //!       of this one.
 //!     - An [Instruction] with side-effects is encountered, if so then insert the currently saved [Instruction::EnableSideEffectsIf]
 //!       before the [Instruction]. Continue inserting instructions until the next [Instruction::EnableSideEffectsIf] is encountered.
+//!
 use std::collections::HashSet;
 
 use acvm::{FieldElement, acir::AcirField};

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

@@ -597,7 +597,7 @@ impl<'a> FunctionContext<'a> {
     /// Inserts a call instruction at the end of the current block and returns the results
     /// of the call.
     ///
-    /// Compared to self.builder.insert_call, this version will reshape the returned Vec<ValueId>
+    /// Compared to self.builder.insert_call, this version will reshape the returned `Vec<ValueId>`
     /// back into a Values tree of the proper shape.
     pub(super) fn insert_call(
         &mut self,
@@ -728,10 +728,12 @@ impl<'a> FunctionContext<'a> {
     /// Method: First `extract_current_value` must recurse on the lvalue to extract the current
     ///         value contained:
     ///
+    /// ```text
     /// v0 = foo.bar                 ; allocate instruction for bar
     /// v1 = load v0                 ; loading the bar array
     /// v2 = add i1, baz_index       ; field offset for index i1, field baz
     /// v3 = array_get v1, index v2  ; foo.bar[i1].baz
+    /// ```
     ///
     /// Method (part 2): Then, `assign_new_value` will recurse in the opposite direction to
     ///                  construct the larger value as needed until we can `store` to the nearest
@@ -790,7 +792,7 @@ impl<'a> FunctionContext<'a> {
     }
 
     /// Compile the given `array[index]` expression as a reference.
-    /// This will return a triple of (array, index, lvalue_ref, Option<length>) where the lvalue_ref records the
+    /// This will return a triple of (array, index, lvalue_ref, `Option<length>`) where the lvalue_ref records the
     /// structure of the lvalue expression for use by `assign_new_value`.
     /// The optional length is for indexing slices rather than arrays since slices
     /// are represented as a tuple in the form: (length, slice contents).

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

@@ -98,7 +98,7 @@ impl<T> Tree<T> {
 
     /// Calls the given function on each leaf node, mapping this tree into a new one.
     ///
-    /// Because the given function returns a Tree<U> rather than a U, it is possible
+    /// Because the given function returns a `Tree<U>` rather than a U, it is possible
     /// to use this function to turn Leaf nodes into either other Leaf nodes or even Branch nodes.
     pub(super) fn map<U>(self, mut f: impl FnMut(T) -> Tree<U>) -> Tree<U> {
         self.map_helper(&mut f)
@@ -173,7 +173,7 @@ impl Tree<Type> {
 }
 
 impl Tree<Value> {
-    /// Flattens and evaluates this Tree<Value> into a list of ir values
+    /// Flattens and evaluates this `Tree<Value>` into a list of ir values
     /// for return statements, branching instructions, or function parameters.
     pub(super) fn into_value_list(self, ctx: &mut FunctionContext) -> Vec<IrValueId> {
         vecmap(self.flatten(), |value| value.eval(ctx))

compiler/noirc_frontend/src/ast/statement.rs

@@ -104,14 +104,21 @@ impl StatementKind {
             ParserError::with_reason(ParserErrorReason::MissingSeparatingSemi, location);
 
         match self {
-            StatementKind::Let(_)
-            | StatementKind::Assign(_)
+            StatementKind::Let(_) => {
+                // To match rust, a let statement always requires a semicolon, even at the end of a block
+                if semi.is_none() {
+                    let reason = ParserErrorReason::MissingSemicolonAfterLet;
+                    emit_error(ParserError::with_reason(reason, location));
+                }
+                self
+            }
+            StatementKind::Assign(_)
             | StatementKind::Semi(_)
             | StatementKind::Break
             | StatementKind::Continue
             | StatementKind::Error => {
-                // To match rust, statements always require a semicolon, even at the end of a block
-                if semi.is_none() {
+                // These statements can omit the semicolon if they are the last statement in a block
+                if !last_statement_in_block && semi.is_none() {
                     emit_error(missing_semicolon);
                 }
                 self
@@ -791,13 +798,15 @@ impl ForRange {
     /// Create a 'for' expression taking care of desugaring a 'for e in array' loop
     /// into the following if needed:
     ///
+    /// ```text
     /// {
     ///     let fresh1 = array;
     ///     for fresh2 in 0 .. std::array::len(fresh1) {
     ///         let elem = fresh1[fresh2];
     ///         ...
     ///     }
     /// }
+    /// ````
     pub(crate) fn into_for(
         self,
         identifier: Ident,

compiler/noirc_frontend/src/elaborator/enums.rs

@@ -794,8 +794,8 @@ impl Elaborator<'_> {
 
     /// Compiles the rows of a match expression, outputting a decision tree for the match.
     ///
-    /// This is an adaptation of https://github.com/yorickpeterse/pattern-matching-in-rust/tree/main/jacobs2021
-    /// which is an implementation of https://julesjacobs.com/notes/patternmatching/patternmatching.pdf
+    /// This is an adaptation of <https://github.com/yorickpeterse/pattern-matching-in-rust/tree/main/jacobs2021>
+    /// which is an implementation of <https://julesjacobs.com/notes/patternmatching/patternmatching.pdf>
     pub(super) fn elaborate_match_rows(
         &mut self,
         rows: Vec<Row>,

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -1544,12 +1544,12 @@ impl<'context> Elaborator<'context> {
 
     pub fn get_module(&self, module: ModuleId) -> &ModuleData {
         let message = "A crate should always be present for a given crate id";
-        &self.def_maps.get(&module.krate).expect(message).modules[module.local_id.0]
+        &self.def_maps.get(&module.krate).expect(message)[module.local_id]
     }
 
     fn get_module_mut(def_maps: &mut DefMaps, module: ModuleId) -> &mut ModuleData {
         let message = "A crate should always be present for a given crate id";
-        &mut def_maps.get_mut(&module.krate).expect(message).modules[module.local_id.0]
+        &mut def_maps.get_mut(&module.krate).expect(message)[module.local_id]
     }
 
     fn declare_methods_on_struct(

compiler/noirc_frontend/src/elaborator/patterns.rs

@@ -599,9 +599,11 @@ impl Elaborator<'_> {
 
     /// Solve any generics that are part of the path before the function, for example:
     ///
+    /// ```rust
     /// foo::Bar::<i32>::baz
     ///           ^^^^^
     ///         solve these
+    /// ```
     fn resolve_item_turbofish(&mut self, item: PathResolutionItem) -> Vec<Type> {
         match item {
             PathResolutionItem::Method(struct_id, Some(generics), _func_id) => {

compiler/noirc_frontend/src/elaborator/traits.rs

@@ -260,9 +260,9 @@ impl Elaborator<'_> {
 /// `impl<C> Foo<D> for Bar<E> { fn foo<F>(...); } `
 ///
 /// We have to substitute:
-/// - Self for Bar<E>
-/// - A for D
-/// - B for F
+/// - `Self` for `Bar<E>`
+/// - `A` for `D`
+/// - `B` for `F`
 ///
 /// Before we can type check. Finally, we must also check that the unification
 /// result does not introduce any new bindings. This can happen if the impl

compiler/noirc_frontend/src/graph/mod.rs

@@ -116,8 +116,8 @@ pub struct CrateGraph {
 impl CrateGraph {
     /// Tries to find the requested crate in the current one's dependencies,
     /// otherwise walks down the crate dependency graph from crate_id until we reach it.
-    /// This is needed in case a library (lib1) re-export a structure defined in another library (lib2)
-    /// In that case, we will get [lib1,lib2] when looking for a struct defined in lib2,
+    /// This is needed in case a library (`lib1`) re-export a structure defined in another library (`lib2`)
+    /// In that case, we will get `[lib1,lib2]` when looking for a struct defined in lib2,
     /// re-exported by lib1 and used by the main crate.
     /// Returns the path from crate_id to target_crate_id
     pub(crate) fn find_dependencies(

compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs

@@ -471,7 +471,7 @@ fn type_def_as_type(
     Ok(Value::Type(Type::DataType(type_def_rc, generics)))
 }
 
-/// fn generics(self) -> [(Type, Option<Type>)]
+/// fn generics(self) -> [(Type, `Option<Type>`)]
 fn type_def_generics(
     interner: &NodeInterner,
     arguments: Vec<(Value, Location)>,
@@ -3002,7 +3002,7 @@ pub(crate) fn option(option_type: Type, value: Option<Value>, location: Location
     Value::Struct(fields, option_type)
 }
 
-/// Given a type, assert that it's an Option<T> and return the Type for T
+/// Given a type, assert that it's an `Option<T>` and return the Type for T
 pub(crate) fn extract_option_generic_type(typ: Type) -> Type {
     let Type::DataType(struct_type, mut generics) = typ else {
         panic!("Expected type to be a struct");

compiler/noirc_frontend/src/hir/comptime/tests.rs

@@ -1,10 +1,9 @@
 #![cfg(test)]
 
-use std::collections::{BTreeMap, HashMap};
+use std::collections::HashMap;
 use std::path::PathBuf;
 
 use fm::{FileId, FileManager};
-use noirc_arena::Index;
 use noirc_errors::Location;
 
 use super::Interpreter;
@@ -13,7 +12,7 @@ use super::value::Value;
 use crate::elaborator::{Elaborator, ElaboratorOptions};
 use crate::hir::def_collector::dc_crate::{CompilationError, DefCollector};
 use crate::hir::def_collector::dc_mod::collect_defs;
-use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData};
+use crate::hir::def_map::{CrateDefMap, ModuleData};
 use crate::hir::{Context, ParsedFiles};
 use crate::node_interner::FuncId;
 use crate::parse_program;
@@ -27,19 +26,15 @@ pub(crate) fn with_interpreter<T>(
 ) -> T {
     let file = FileId::default();
 
-    // Can't use Index::test_new here for some reason, even with #[cfg(test)].
-    let module_id = LocalModuleId(Index::unsafe_zeroed());
-    let mut modules = noirc_arena::Arena::default();
     let location = Location::new(Default::default(), file);
-    let root = LocalModuleId(modules.insert(ModuleData::new(
+    let root_module = ModuleData::new(
         None,
         location,
         Vec::new(),
         Vec::new(),
         false, // is contract
         false, // is struct
-    )));
-    assert_eq!(root, module_id);
+    );
 
     let file_manager = FileManager::new(&PathBuf::new());
     let parsed_files = ParsedFiles::new();
@@ -52,10 +47,11 @@ pub(crate) fn with_interpreter<T>(
     assert_eq!(errors.len(), 0);
     let ast = module.into_sorted();
 
-    let def_map = CrateDefMap { root: module_id, modules, krate, extern_prelude: BTreeMap::new() };
+    let def_map = CrateDefMap::new(krate, root_module);
+    let root_module_id = def_map.root();
     let mut collector = DefCollector::new(def_map);
 
-    collect_defs(&mut collector, ast, FileId::dummy(), module_id, krate, &mut context);
+    collect_defs(&mut collector, ast, FileId::dummy(), root_module_id, krate, &mut context);
     context.def_maps.insert(krate, collector.def_map);
 
     let main = context.get_main_function(&krate).expect("Expected 'main' function");