feat: Sync from noir (#8378)

Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat(perf): Remove known store values that equal the store address in
mem2reg (noir-lang/noir#5935)
feat: remove blocks which consist of only a jump to another block
(noir-lang/noir#5889)
fix: use element_size() instead of computing it with division
(noir-lang/noir#5939)
feat: Add `StructDefinition::set_fields`
(noir-lang/noir#5931)
feat: Only check array bounds in brillig if index is unsafe
(noir-lang/noir#5938)
chore: error on false constraint
(noir-lang/noir#5890)
feat: warn on unused functions
(noir-lang/noir#5892)
feat: add `fmtstr::contents`
(noir-lang/noir#5928)
fix: collect functions generated by attributes
(noir-lang/noir#5930)
fix: Support debug comptime flag for attributes
(noir-lang/noir#5929)
feat: Allow inserting new structs and into programs from attributes
(noir-lang/noir#5927)
feat: module attributes (noir-lang/noir#5888)
feat: unquote some value as tokens, not as unquote markers
(noir-lang/noir#5924)
feat: check argument count and types on attribute function callback
(noir-lang/noir#5921)
feat: LSP will now suggest private items if they are visible
(noir-lang/noir#5923)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>

.noir-sync-commit

@@ -1 +1 @@
-44cf9a2140bc06b550d4b46966f1637598ac11a7
+b84009ca428a5790acf53a6c027146b706170574

noir/noir-repo/aztec_macros/src/utils/ast_utils.rs

@@ -187,8 +187,8 @@ pub fn check_trait_method_implemented(trait_impl: &NoirTraitImpl, method_name: &
 
 /// Checks if an attribute is a custom attribute with a specific name
 pub fn is_custom_attribute(attr: &SecondaryAttribute, attribute_name: &str) -> bool {
-    if let SecondaryAttribute::Custom(custom_attr) = attr {
-        custom_attr.as_str() == attribute_name
+    if let SecondaryAttribute::Custom(custom_attribute) = attr {
+        custom_attribute.contents.as_str() == attribute_name
     } else {
         false
     }

noir/noir-repo/aztec_macros/src/utils/parse_utils.rs

@@ -53,6 +53,7 @@ fn empty_item(item: &mut Item) {
         ItemKind::Import(use_tree, _) => empty_use_tree(use_tree),
         ItemKind::Struct(noir_struct) => empty_noir_struct(noir_struct),
         ItemKind::TypeAlias(noir_type_alias) => empty_noir_type_alias(noir_type_alias),
+        ItemKind::InnerAttribute(_) => (),
     }
 }
 

noir/noir-repo/compiler/noirc_driver/src/lib.rs

@@ -452,9 +452,13 @@ fn compile_contract_inner(
             .attributes
             .secondary
             .iter()
-            .filter_map(
-                |attr| if let SecondaryAttribute::Custom(tag) = attr { Some(tag) } else { None },
-            )
+            .filter_map(|attr| {
+                if let SecondaryAttribute::Custom(attribute) = attr {
+                    Some(&attribute.contents)
+                } else {
+                    None
+                }
+            })
             .cloned()
             .collect();
 

noir/noir-repo/compiler/noirc_errors/src/position.rs

@@ -103,6 +103,10 @@ impl Span {
         let other_distance = other.end() - other.start();
         self_distance < other_distance
     }
+
+    pub fn shift_by(&self, offset: u32) -> Span {
+        Self::from(self.start() + offset..self.end() + offset)
+    }
 }
 
 impl From<Span> for Range<usize> {

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -648,7 +648,11 @@ impl<'block> BrilligBlock<'block> {
                 };
 
                 let index_variable = self.convert_ssa_single_addr_value(*index, dfg);
-                self.validate_array_index(array_variable, index_variable);
+
+                if !dfg.is_safe_index(*index, *array) {
+                    self.validate_array_index(array_variable, index_variable);
+                }
+
                 self.retrieve_variable_from_array(
                     array_pointer,
                     index_variable,
@@ -667,7 +671,11 @@ impl<'block> BrilligBlock<'block> {
                     result_ids[0],
                     dfg,
                 );
-                self.validate_array_index(source_variable, index_register);
+
+                if !dfg.is_safe_index(*index, *array) {
+                    self.validate_array_index(source_variable, index_register);
+                }
+
                 self.convert_ssa_array_set(
                     source_variable,
                     destination_variable,

noir/noir-repo/compiler/noirc_evaluator/src/errors.rs

@@ -88,6 +88,7 @@ impl From<SsaReport> for FileDiagnostic {
                     InternalBug::IndependentSubgraph { call_stack } => {
                         ("There is no path from the output of this brillig call to either return values or inputs of the circuit, which creates an independent subgraph. This is quite likely a soundness vulnerability".to_string(),call_stack)
                     }
+                    InternalBug::AssertFailed { call_stack } => ("As a result, the compiled circuit is ensured to fail. Other assertions may also fail during execution".to_string(), call_stack)
                 };
                 let call_stack = vecmap(call_stack, |location| location);
                 let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
@@ -111,6 +112,8 @@ pub enum InternalWarning {
 pub enum InternalBug {
     #[error("Input to brillig function is in a separate subgraph to output")]
     IndependentSubgraph { call_stack: CallStack },
+    #[error("Assertion is always false")]
+    AssertFailed { call_stack: CallStack },
 }
 
 #[derive(Debug, PartialEq, Eq, Clone, Error)]

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -2,7 +2,7 @@ use super::big_int::BigIntContext;
 use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX};
 use crate::brillig::brillig_gen::brillig_directive;
 use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
-use crate::errors::{InternalError, RuntimeError, SsaReport};
+use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport};
 use crate::ssa::acir_gen::{AcirDynamicArray, AcirValue};
 use crate::ssa::ir::dfg::CallStack;
 use crate::ssa::ir::types::Type as SsaType;
@@ -126,6 +126,8 @@ pub(crate) struct AcirContext<F: AcirField> {
     big_int_ctx: BigIntContext,
 
     expression_width: ExpressionWidth,
+
+    pub(crate) warnings: Vec<SsaReport>,
 }
 
 impl<F: AcirField> AcirContext<F> {
@@ -518,6 +520,12 @@ impl<F: AcirField> AcirContext<F> {
             self.mark_variables_equivalent(lhs, rhs)?;
             return Ok(());
         }
+        if diff_expr.is_const() {
+            // Constraint is always false
+            self.warnings.push(SsaReport::Bug(InternalBug::AssertFailed {
+                call_stack: self.get_call_stack(),
+            }));
+        }
 
         self.acir_ir.assert_is_zero(diff_expr);
         if let Some(payload) = assert_message {

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -453,6 +453,7 @@ impl<'a> Context<'a> {
         }
 
         warnings.extend(return_warnings);
+        warnings.extend(self.acir_context.warnings.clone());
 
         // Add the warnings from the alter Ssa passes
         Ok(self.acir_context.finish(input_witness, return_witnesses, warnings))

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs

@@ -377,13 +377,12 @@ fn handle_array_get_group(
     next_out_of_bounds_index: &mut Option<usize>,
     possible_index_out_of_bounds_indexes: &mut Vec<usize>,
 ) {
-    let Some(array_length) = function.dfg.try_get_array_length(*array) else {
+    if function.dfg.try_get_array_length(*array).is_none() {
         // Nothing to do for slices
         return;
     };
 
-    let flattened_size = function.dfg.type_of_value(*array).flattened_size();
-    let element_size = flattened_size / array_length;
+    let element_size = function.dfg.type_of_value(*array).element_size();
     if element_size <= 1 {
         // Not a composite type
         return;

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs

@@ -117,6 +117,10 @@ struct PerFunctionContext<'f> {
     /// Track a value's last load across all blocks.
     /// If a value is not used in anymore loads we can remove the last store to that value.
     last_loads: HashMap<ValueId, (InstructionId, BasicBlockId)>,
+
+    /// Track whether the last instruction is an inc_rc/dec_rc instruction.
+    /// If it is we should not remove any known store values.
+    inside_rc_reload: bool,
 }
 
 impl<'f> PerFunctionContext<'f> {
@@ -131,6 +135,7 @@ impl<'f> PerFunctionContext<'f> {
             blocks: BTreeMap::new(),
             instructions_to_remove: BTreeSet::new(),
             last_loads: HashMap::default(),
+            inside_rc_reload: false,
         }
     }
 
@@ -281,6 +286,21 @@ impl<'f> PerFunctionContext<'f> {
                 } else {
                     references.mark_value_used(address, self.inserter.function);
 
+                    let expression = if let Some(expression) = references.expressions.get(&result) {
+                        expression.clone()
+                    } else {
+                        references.expressions.insert(result, Expression::Other(result));
+                        Expression::Other(result)
+                    };
+                    if let Some(aliases) = references.aliases.get_mut(&expression) {
+                        aliases.insert(result);
+                    } else {
+                        references
+                            .aliases
+                            .insert(Expression::Other(result), AliasSet::known(result));
+                    }
+                    references.set_known_value(result, address);
+
                     self.last_loads.insert(address, (instruction, block_id));
                 }
             }
@@ -296,6 +316,14 @@ impl<'f> PerFunctionContext<'f> {
                     self.instructions_to_remove.insert(*last_store);
                 }
 
+                let known_value = references.get_known_value(value);
+                if let Some(known_value) = known_value {
+                    let known_value_is_address = known_value == address;
+                    if known_value_is_address && !self.inside_rc_reload {
+                        self.instructions_to_remove.insert(instruction);
+                    }
+                }
+
                 references.set_known_value(address, value);
                 references.last_stores.insert(address, instruction);
             }
@@ -350,6 +378,18 @@ impl<'f> PerFunctionContext<'f> {
             Instruction::Call { arguments, .. } => self.mark_all_unknown(arguments, references),
             _ => (),
         }
+
+        self.track_rc_reload_state(instruction);
+    }
+
+    fn track_rc_reload_state(&mut self, instruction: InstructionId) {
+        match &self.inserter.function.dfg[instruction] {
+            // We just had an increment or decrement to an array's reference counter
+            Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. } => {
+                self.inside_rc_reload = true;
+            }
+            _ => self.inside_rc_reload = false,
+        }
     }
 
     fn check_array_aliasing(&self, references: &mut Block, array: ValueId) {

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs

@@ -15,7 +15,9 @@ use acvm::acir::AcirField;
 
 use crate::ssa::{
     ir::{
-        basic_block::BasicBlockId, cfg::ControlFlowGraph, function::Function,
+        basic_block::BasicBlockId,
+        cfg::ControlFlowGraph,
+        function::{Function, RuntimeType},
         instruction::TerminatorInstruction,
     },
     ssa_gen::Ssa,
@@ -30,7 +32,7 @@ impl Ssa {
     /// 5. Replacing any jmpifs with constant conditions with jmps. If this causes the block to have
     ///    only 1 successor then (2) also will be applied.
     ///
-    /// Currently, 1 and 4 are unimplemented.
+    /// Currently, 1 is unimplemented.
     #[tracing::instrument(level = "trace", skip(self))]
     pub(crate) fn simplify_cfg(mut self) -> Self {
         for function in self.functions.values_mut() {
@@ -72,6 +74,10 @@ fn simplify_function(function: &mut Function) {
             // optimizations performed after this point on the same block should check if
             // the inlining here was successful before continuing.
             try_inline_into_predecessor(function, &mut cfg, block, predecessor);
+        } else {
+            drop(predecessors);
+
+            check_for_double_jmp(function, block, &mut cfg);
         }
     }
 }
@@ -102,6 +108,71 @@ fn check_for_constant_jmpif(
     }
 }
 
+/// Optimize a jmp to a block which immediately jmps elsewhere to just jmp to the second block.
+fn check_for_double_jmp(function: &mut Function, block: BasicBlockId, cfg: &mut ControlFlowGraph) {
+    if matches!(function.runtime(), RuntimeType::Acir(_)) {
+        // We can't remove double jumps in ACIR functions as this interferes with the `flatten_cfg` pass.
+        return;
+    }
+
+    if !function.dfg[block].instructions().is_empty()
+        || !function.dfg[block].parameters().is_empty()
+    {
+        return;
+    }
+
+    let Some(TerminatorInstruction::Jmp { destination: final_destination, arguments, .. }) =
+        function.dfg[block].terminator()
+    else {
+        return;
+    };
+
+    if !arguments.is_empty() {
+        return;
+    }
+
+    let final_destination = *final_destination;
+
+    let predecessors: Vec<_> = cfg.predecessors(block).collect();
+    for predecessor_block in predecessors {
+        let terminator_instruction = function.dfg[predecessor_block].take_terminator();
+        let redirected_terminator_instruction = match terminator_instruction {
+            TerminatorInstruction::JmpIf {
+                condition,
+                then_destination,
+                else_destination,
+                call_stack,
+            } => {
+                let then_destination =
+                    if then_destination == block { final_destination } else { then_destination };
+                let else_destination =
+                    if else_destination == block { final_destination } else { else_destination };
+                TerminatorInstruction::JmpIf {
+                    condition,
+                    then_destination,
+                    else_destination,
+                    call_stack,
+                }
+            }
+            TerminatorInstruction::Jmp { destination, arguments, call_stack } => {
+                assert_eq!(
+                    destination, block,
+                    "ICE: predecessor block doesn't jump to current block"
+                );
+                assert!(arguments.is_empty(), "ICE: predecessor jmp has arguments");
+                TerminatorInstruction::Jmp { destination: final_destination, arguments, call_stack }
+            }
+            TerminatorInstruction::Return { .. } => {
+                unreachable!("ICE: predecessor block should not have return terminator instruction")
+            }
+        };
+
+        function.dfg[predecessor_block].set_terminator(redirected_terminator_instruction);
+        cfg.recompute_block(function, predecessor_block);
+    }
+    cfg.recompute_block(function, block);
+}
+
 /// If the given block has block parameters, replace them with the jump arguments from the predecessor.
 ///
 /// Currently, if this function is needed, `try_inline_into_predecessor` will also always apply,

noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs

@@ -292,6 +292,7 @@ pub trait Recoverable {
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct ModuleDeclaration {
     pub ident: Ident,
+    pub outer_attributes: Vec<SecondaryAttribute>,
 }
 
 impl std::fmt::Display for ModuleDeclaration {

noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs

@@ -16,7 +16,7 @@ use crate::{
         QuotedTypeId,
     },
     parser::{Item, ItemKind, ParsedSubModule},
-    token::Tokens,
+    token::{SecondaryAttribute, Tokens},
     ParsedModule, QuotedType,
 };
 
@@ -432,6 +432,8 @@ pub trait Visitor {
     fn visit_struct_pattern(&mut self, _: &Path, _: &[(Ident, Pattern)], _: Span) -> bool {
         true
     }
+
+    fn visit_secondary_attribute(&mut self, _: &SecondaryAttribute, _: Span) {}
 }
 
 impl ParsedModule {
@@ -481,6 +483,9 @@ impl Item {
             ItemKind::ModuleDecl(module_declaration) => {
                 module_declaration.accept(self.span, visitor);
             }
+            ItemKind::InnerAttribute(attribute) => {
+                attribute.accept(self.span, visitor);
+            }
         }
     }
 }
@@ -1289,6 +1294,12 @@ impl Pattern {
     }
 }
 
+impl SecondaryAttribute {
+    pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
+        visitor.visit_secondary_attribute(self, span);
+    }
+}
+
 fn visit_expressions(expressions: &[Expression], visitor: &mut impl Visitor) {
     for expression in expressions {
         expression.accept(visitor);