diff --git a/.noir-sync-commit b/.noir-sync-commit index eb2e669ce67..3222858403e 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -d44f882be094bf492b1742370fd3896b0c371f59 +bb6913ac53620fabd73e24ca1a2b1369225903ec diff --git a/noir-projects/aztec-nr/aztec/src/utils/test.nr b/noir-projects/aztec-nr/aztec/src/utils/test.nr index d8c1edd5e2f..5ea5ad23624 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/test.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/test.nr @@ -75,7 +75,8 @@ fn verify_collapse_hints_hint_length_mismatch() { verify_collapse_hints(original, collapsed, collapsed_to_input_index_mapping); } -#[test(should_fail_with="Out of bounds index hint")] +// https://github.com/noir-lang/noir/issues/5464 +#[test(should_fail)] fn verify_collapse_hints_out_of_bounds_index_hint() { let original = [Option::some(7), Option::none(), Option::some(3)]; let collapsed = BoundedVec::from_array([7, 3]); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 3b7d2c1025f..cfcc7a9a997 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -976,7 +976,15 @@ impl<'a> Context<'a> { } }; - if self.handle_constant_index(instruction, dfg, index, array, store_value)? { + let array_id = dfg.resolve(array); + let array_typ = dfg.type_of_value(array_id); + // Compiler sanity checks + assert!(!array_typ.is_nested_slice(), "ICE: Nested slice type has reached ACIR generation"); + let (Type::Array(_, _) | Type::Slice(_)) = &array_typ else { + unreachable!("ICE: expected array or slice type"); + }; + + if self.handle_constant_index_wrapper(instruction, dfg, array, index, store_value)? { return Ok(()); } @@ -984,8 +992,7 @@ impl<'a> Context<'a> { // If we find one, we will use it when computing the index under the enable_side_effect predicate // If not, array_get(..) will use a fallback costing one multiplication in the worst case. // cf. https://github.com/noir-lang/noir/pull/4971 - let array_id = dfg.resolve(array); - let array_typ = dfg.type_of_value(array_id); + // For simplicity we compute the offset only for simple arrays let is_simple_array = dfg.instruction_results(instruction).len() == 1 && can_omit_element_sizes_array(&array_typ); @@ -1018,83 +1025,106 @@ impl<'a> Context<'a> { Ok(()) } - /// Handle constant index: if there is no predicate and we have the array values, - /// we can perform the operation directly on the array - fn handle_constant_index( + fn handle_constant_index_wrapper( &mut self, instruction: InstructionId, dfg: &DataFlowGraph, + array: ValueId, index: ValueId, - array_id: ValueId, store_value: Option, ) -> Result { - let index_const = dfg.get_numeric_constant(index); - let value_type = dfg.type_of_value(array_id); + let array_id = dfg.resolve(array); + let array_typ = dfg.type_of_value(array_id); // Compiler sanity checks - assert!( - !value_type.is_nested_slice(), - "ICE: Nested slice type has reached ACIR generation" - ); - let (Type::Array(_, _) | Type::Slice(_)) = &value_type else { + assert!(!array_typ.is_nested_slice(), "ICE: Nested slice type has reached ACIR generation"); + let (Type::Array(_, _) | Type::Slice(_)) = &array_typ else { unreachable!("ICE: expected array or slice type"); }; match self.convert_value(array_id, dfg) { AcirValue::Var(acir_var, _) => { - return Err(RuntimeError::InternalError(InternalError::Unexpected { + Err(RuntimeError::InternalError(InternalError::Unexpected { expected: "an array value".to_string(), found: format!("{acir_var:?}"), call_stack: self.acir_context.get_call_stack(), })) } AcirValue::Array(array) => { - if let Some(index_const) = index_const { - let array_size = array.len(); - let index = match index_const.try_to_u64() { - Some(index_const) => index_const as usize, - None => { - let call_stack = self.acir_context.get_call_stack(); - return Err(RuntimeError::TypeConversion { - from: "array index".to_string(), - into: "u64".to_string(), - call_stack, - }); - } - }; + // `AcirValue::Array` supports reading/writing to constant indices at compile-time in some cases. + if let Some(constant_index) = dfg.get_numeric_constant(index) { + let store_value = store_value.map(|value| self.convert_value(value, dfg)); + self.handle_constant_index(instruction, dfg, array, constant_index, store_value) + } else { + Ok(false) + } + } + AcirValue::DynamicArray(_) => Ok(false), + } + } - if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) { - // Report the error if side effects are enabled. - if index >= array_size { - let call_stack = self.acir_context.get_call_stack(); - return Err(RuntimeError::IndexOutOfBounds { - index, - array_size, - call_stack, - }); - } else { - let value = match store_value { - Some(store_value) => { - let store_value = self.convert_value(store_value, dfg); - AcirValue::Array(array.update(index, store_value)) - } - None => array[index].clone(), - }; + /// Handle constant index: if there is no predicate and we have the array values, + /// we can perform the operation directly on the array + fn handle_constant_index( + &mut self, + instruction: InstructionId, + dfg: &DataFlowGraph, + array: Vector, + index: FieldElement, + store_value: Option, + ) -> Result { + let array_size: usize = array.len(); + let index = match index.try_to_u64() { + Some(index_const) => index_const as usize, + None => { + let call_stack = self.acir_context.get_call_stack(); + return Err(RuntimeError::TypeConversion { + from: "array index".to_string(), + into: "u64".to_string(), + call_stack, + }); + } + }; - self.define_result(dfg, instruction, value); - return Ok(true); - } - } - // If there is a predicate and the index is not out of range, we can directly perform the read - else if index < array_size && store_value.is_none() { - self.define_result(dfg, instruction, array[index].clone()); - return Ok(true); - } + let side_effects_always_enabled = + self.acir_context.is_constant_one(&self.current_side_effects_enabled_var); + let index_out_of_bounds = index >= array_size; + + // Note that the value of `side_effects_always_enabled` doesn't affect the value which we return here for valid + // indices, just whether we return an error for invalid indices at compile time or defer until execution. + match (side_effects_always_enabled, index_out_of_bounds) { + (true, false) => { + let value = match store_value { + Some(store_value) => AcirValue::Array(array.update(index, store_value)), + None => array[index].clone(), + }; + + self.define_result(dfg, instruction, value); + Ok(true) + } + (false, false) => { + if store_value.is_none() { + // If there is a predicate and the index is not out of range, we can optimistically perform the + // read at compile time as if the predicate is true. + // + // This is as if the predicate is false, any side-effects will be disabled so the value returned + // will not affect the rest of execution. + self.define_result(dfg, instruction, array[index].clone()); + Ok(true) + } else { + // We do not do this for a array writes however. + Ok(false) } } - AcirValue::DynamicArray(_) => (), - }; - Ok(false) + // Report the error if side effects are enabled. + (true, true) => { + let call_stack = self.acir_context.get_call_stack(); + Err(RuntimeError::IndexOutOfBounds { index, array_size, call_stack }) + } + // Index is out of bounds but predicate may result in this array operation being skipped + // so we don't return an error now. + (false, true) => Ok(false), + } } /// We need to properly setup the inputs for array operations in ACIR. diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 6dac99d7a09..5831faa7c4d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -105,7 +105,7 @@ impl Context { .iter() .chain(function.returns()) .filter(|id| function.dfg.get_numeric_constant(**id).is_none()) - .copied(); + .map(|value_id| function.dfg.resolve(*value_id)); let mut connected_sets_indices: HashSet = HashSet::new(); @@ -169,13 +169,13 @@ impl Context { // Insert non-constant instruction arguments function.dfg[*instruction].for_each_value(|value_id| { if function.dfg.get_numeric_constant(value_id).is_none() { - instruction_arguments_and_results.insert(value_id); + instruction_arguments_and_results.insert(function.dfg.resolve(value_id)); } }); // And non-constant results for value_id in function.dfg.instruction_results(*instruction).iter() { if function.dfg.get_numeric_constant(*value_id).is_none() { - instruction_arguments_and_results.insert(*value_id); + instruction_arguments_and_results.insert(function.dfg.resolve(*value_id)); } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index f854e8e0693..b08283a9ceb 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -379,9 +379,17 @@ impl Instruction { { true } - Instruction::EnableSideEffects { .. } - | Instruction::ArrayGet { .. } - | Instruction::ArraySet { .. } => true, + + // `ArrayGet`s which read from "known good" indices from an array don't need a predicate. + Instruction::ArrayGet { array, index } => { + #[allow(clippy::match_like_matches_macro)] + match (dfg.type_of_value(*array), dfg.get_numeric_constant(*index)) { + (Type::Array(_, len), Some(index)) if index.to_u128() < (len as u128) => false, + _ => true, + } + } + + Instruction::EnableSideEffects { .. } | Instruction::ArraySet { .. } => true, Instruction::Call { func, .. } => match dfg[*func] { Value::Function(_) => true, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 48bd70ff139..d7cd366e9af 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -288,7 +288,7 @@ mod test { value::{Value, ValueId}, }, }; - use acvm::acir::AcirField; + use acvm::{acir::AcirField, FieldElement}; #[test] fn simple_constant_fold() { @@ -545,6 +545,73 @@ mod test { assert_eq!(instruction, &Instruction::Cast(v0, Type::unsigned(32))); } + #[test] + fn constant_index_array_access_deduplication() { + // fn main f0 { + // b0(v0: [Field; 4], v1: u32, v2: bool, v3: bool): + // enable_side_effects v2 + // v4 = array_get v0 u32 0 + // v5 = array_get v0 v1 + // enable_side_effects v3 + // v6 = array_get v0 u32 0 + // v7 = array_get v0 v1 + // constrain v4 v6 + // } + // + // After constructing this IR, we run constant folding which should replace the second constant-index array get + // with a reference to the results to the first. This then allows us to optimize away + // the constrain instruction as both inputs are known to be equal. + // + let main_id = Id::test_new(0); + + // Compiling main + let mut builder = FunctionBuilder::new("main".into(), main_id); + + let v0 = builder.add_parameter(Type::Array(Rc::new(vec![Type::field()]), 4)); + let v1 = builder.add_parameter(Type::unsigned(32)); + let v2 = builder.add_parameter(Type::unsigned(1)); + let v3 = builder.add_parameter(Type::unsigned(1)); + + let zero = builder.numeric_constant(FieldElement::zero(), Type::length_type()); + + builder.insert_enable_side_effects_if(v2); + let v4 = builder.insert_array_get(v0, zero, Type::field()); + let _v5 = builder.insert_array_get(v0, v1, Type::field()); + + builder.insert_enable_side_effects_if(v3); + let v6 = builder.insert_array_get(v0, zero, Type::field()); + let _v7 = builder.insert_array_get(v0, v1, Type::field()); + + builder.insert_constrain(v4, v6, None); + + let ssa = builder.finish(); + + println!("{ssa}"); + + let main = ssa.main(); + let instructions = main.dfg[main.entry_block()].instructions(); + assert_eq!(instructions.len(), 7); + + // Expected output: + // + // fn main f0 { + // b0(v0: [Field; 4], v1: u32, v2: bool, v3: bool): + // enable_side_effects v2 + // v10 = array_get v0 u32 0 + // v11 = array_get v0 v1 + // enable_side_effects v3 + // v12 = array_get v0 v1 + // } + let ssa = ssa.fold_constants(); + + println!("{ssa}"); + + let main = ssa.main(); + let instructions = main.dfg[main.entry_block()].instructions(); + + assert_eq!(instructions.len(), 5); + } + #[test] fn constraint_decomposition() { // fn main f0 { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index c30bc884535..09802713363 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/noir/noir-repo/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 = 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/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index f9a3c9a55eb..1584b848564 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -18,6 +18,7 @@ use crate::ssa::{ dfg::DataFlowGraph, function::Function, instruction::{BinaryOp, Instruction, Intrinsic}, + types::Type, value::Value, }, ssa_gen::Ssa, @@ -62,6 +63,7 @@ impl Context { ) { let instructions = function.dfg[block].take_instructions(); + let mut active_condition = function.dfg.make_constant(FieldElement::one(), Type::bool()); let mut last_side_effects_enabled_instruction = None; let mut new_instructions = Vec::with_capacity(instructions.len()); @@ -72,19 +74,26 @@ impl Context { // instructions with side effects then we can drop the instruction we're holding and // continue with the new `Instruction::EnableSideEffects`. if let Instruction::EnableSideEffects { condition } = instruction { + // If this instruction isn't changing the currently active condition then we can ignore it. + if active_condition == *condition { + continue; + } + // If we're seeing an `enable_side_effects u1 1` then we want to insert it immediately. // This is because we want to maximize the effect it will have. - if function + let condition_is_one = function .dfg .get_numeric_constant(*condition) - .map_or(false, |condition| condition.is_one()) - { + .map_or(false, |condition| condition.is_one()); + if condition_is_one { new_instructions.push(instruction_id); last_side_effects_enabled_instruction = None; + active_condition = *condition; continue; } last_side_effects_enabled_instruction = Some(instruction_id); + active_condition = *condition; continue; } @@ -172,3 +181,88 @@ impl Context { } } } + +#[cfg(test)] +mod test { + + use crate::ssa::{ + function_builder::FunctionBuilder, + ir::{ + instruction::{BinaryOp, Instruction}, + map::Id, + types::Type, + }, + }; + + #[test] + fn remove_chains_of_same_condition() { + // acir(inline) fn main f0 { + // b0(v0: Field): + // enable_side_effects u1 1 + // v4 = mul v0, Field 2 + // enable_side_effects u1 1 + // v5 = mul v0, Field 2 + // enable_side_effects u1 1 + // v6 = mul v0, Field 2 + // enable_side_effects u1 1 + // v7 = mul v0, Field 2 + // enable_side_effects u1 1 + // (no terminator instruction) + // } + // + // After constructing this IR, we run constant folding which should replace the second cast + // with a reference to the results to the first. This then allows us to optimize away + // the constrain instruction as both inputs are known to be equal. + // + // The first cast instruction is retained and will be removed in the dead instruction elimination pass. + let main_id = Id::test_new(0); + + // Compiling main + let mut builder = FunctionBuilder::new("main".into(), main_id); + let v0 = builder.add_parameter(Type::field()); + + let two = builder.numeric_constant(2u128, Type::field()); + + let one = builder.numeric_constant(1u128, Type::bool()); + + builder.insert_enable_side_effects_if(one); + builder.insert_binary(v0, BinaryOp::Mul, two); + builder.insert_enable_side_effects_if(one); + builder.insert_binary(v0, BinaryOp::Mul, two); + builder.insert_enable_side_effects_if(one); + builder.insert_binary(v0, BinaryOp::Mul, two); + builder.insert_enable_side_effects_if(one); + builder.insert_binary(v0, BinaryOp::Mul, two); + builder.insert_enable_side_effects_if(one); + + let ssa = builder.finish(); + + println!("{ssa}"); + + let main = ssa.main(); + let instructions = main.dfg[main.entry_block()].instructions(); + assert_eq!(instructions.len(), 9); + + // Expected output: + // + // acir(inline) fn main f0 { + // b0(v0: Field): + // v3 = mul v0, Field 2 + // v4 = mul v0, Field 2 + // v5 = mul v0, Field 2 + // v6 = mul v0, Field 2 + // (no terminator instruction) + // } + let ssa = ssa.remove_enable_side_effects(); + + println!("{ssa}"); + + let main = ssa.main(); + let instructions = main.dfg[main.entry_block()].instructions(); + + assert_eq!(instructions.len(), 4); + for instruction in instructions.iter().take(4) { + assert_eq!(&main.dfg[*instruction], &Instruction::binary(BinaryOp::Mul, v0, two)); + } + } +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs index 5c3866816a6..ad79af4e63d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -29,7 +29,7 @@ use crate::{ }, node_interner::{DefinitionKind, ExprId, FuncId, ReferenceId}, token::Tokens, - Kind, QuotedType, Shared, StructType, Type, + QuotedType, Shared, StructType, Type, }; use super::Elaborator; @@ -51,23 +51,7 @@ impl<'context> Elaborator<'context> { ExpressionKind::Infix(infix) => return self.elaborate_infix(*infix, expr.span), ExpressionKind::If(if_) => self.elaborate_if(*if_), ExpressionKind::Variable(variable, generics) => { - let generics = generics.map(|option_inner| { - option_inner - .into_iter() - .map(|generic| { - // All type expressions should resolve to a `Type::Constant` - if generic.is_type_expression() { - self.resolve_type_inner( - generic, - &Kind::Numeric(Box::new(Type::default_int_type())), - ) - } else { - self.resolve_type(generic) - } - }) - .collect() - }); - return self.elaborate_variable(variable, generics); + return self.elaborate_variable(variable, generics) } ExpressionKind::Tuple(tuple) => self.elaborate_tuple(tuple), ExpressionKind::Lambda(lambda) => self.elaborate_lambda(*lambda), @@ -342,14 +326,18 @@ impl<'context> Elaborator<'context> { } }; - if func_id != FuncId::dummy_id() { + let generics = if func_id != FuncId::dummy_id() { let function_type = self.interner.function_meta(&func_id).typ.clone(); self.try_add_mutable_reference_to_object( &function_type, &mut object_type, &mut object, ); - } + + self.resolve_turbofish_generics(&func_id, method_call.generics, span) + } else { + None + }; // These arguments will be given to the desugared function call. // Compared to the method arguments, they also contain the object. @@ -367,9 +355,6 @@ impl<'context> Elaborator<'context> { let location = Location::new(span, self.file); let method = method_call.method_name; - let generics = method_call.generics.map(|option_inner| { - option_inner.into_iter().map(|generic| self.resolve_type(generic)).collect() - }); let turbofish_generics = generics.clone(); let method_call = HirMethodCallExpression { method, object, arguments, location, generics }; @@ -434,7 +419,7 @@ impl<'context> Elaborator<'context> { }); let referenced = ReferenceId::Struct(struct_type.borrow().id); - let reference = ReferenceId::Variable(Location::new(span, self.file), is_self_type); + let reference = ReferenceId::Reference(Location::new(span, self.file), is_self_type); self.interner.add_reference(referenced, reference); (expr, Type::Struct(struct_type, generics)) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index 18fce032d5f..6104da582a7 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -702,8 +702,7 @@ impl<'context> Elaborator<'context> { let direct_generics = func.def.generics.iter(); let direct_generics = direct_generics - .filter_map(|generic| self.find_generic(&generic.ident().0.contents)) - .map(|ResolvedGeneric { name, type_var, .. }| (name.clone(), type_var.clone())) + .filter_map(|generic| self.find_generic(&generic.ident().0.contents).cloned()) .collect(); let statements = std::mem::take(&mut func.def.body.statements); @@ -1350,6 +1349,8 @@ impl<'context> Elaborator<'context> { self.elaborate_comptime_global(global_id); } + self.interner.add_definition_location(ReferenceId::Global(global_id)); + self.local_module = old_module; self.file = old_file; self.current_item = old_item; @@ -1452,7 +1453,7 @@ impl<'context> Elaborator<'context> { let trait_name = trait_impl.trait_path.last_segment(); let referenced = ReferenceId::Trait(trait_id); - let reference = ReferenceId::Variable( + let reference = ReferenceId::Reference( Location::new(trait_name.span(), trait_impl.file_id), trait_name.is_self_type_name(), ); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs index 81fffb522bf..b1c9b1b37cc 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -3,7 +3,7 @@ use noirc_errors::{Location, Span}; use rustc_hash::FxHashSet as HashSet; use crate::{ - ast::ERROR_IDENT, + ast::{UnresolvedType, ERROR_IDENT}, hir::{ comptime::Interpreter, def_collector::dc_crate::CompilationError, @@ -15,7 +15,9 @@ use crate::{ stmt::HirPattern, }, macros_api::{HirExpression, Ident, Path, Pattern}, - node_interner::{DefinitionId, DefinitionKind, ExprId, GlobalId, ReferenceId, TraitImplKind}, + node_interner::{ + DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, ReferenceId, TraitImplKind, + }, Shared, StructType, Type, TypeBindings, }; @@ -201,7 +203,7 @@ impl<'context> Elaborator<'context> { ); let referenced = ReferenceId::Struct(struct_type.borrow().id); - let reference = ReferenceId::Variable(Location::new(name_span, self.file), is_self_type); + let reference = ReferenceId::Reference(Location::new(name_span, self.file), is_self_type); self.interner.add_reference(referenced, reference); HirPattern::Struct(expected_type, fields, location) @@ -398,15 +400,53 @@ impl<'context> Elaborator<'context> { } } + /// Resolve generics using the expected kinds of the function we are calling + pub(super) fn resolve_turbofish_generics( + &mut self, + func_id: &FuncId, + unresolved_turbofish: Option>, + span: Span, + ) -> Option> { + let direct_generics = self.interner.function_meta(func_id).direct_generics.clone(); + + unresolved_turbofish.map(|option_inner| { + if option_inner.len() != direct_generics.len() { + let type_check_err = TypeCheckError::IncorrectTurbofishGenericCount { + expected_count: direct_generics.len(), + actual_count: option_inner.len(), + span, + }; + self.push_err(type_check_err); + } + + let generics_with_types = direct_generics.iter().zip(option_inner); + vecmap(generics_with_types, |(generic, unresolved_type)| { + self.resolve_type_inner(unresolved_type, &generic.kind) + }) + }) + } + pub(super) fn elaborate_variable( &mut self, variable: Path, - generics: Option>, + unresolved_turbofish: Option>, ) -> (ExprId, Type) { let span = variable.span; let expr = self.resolve_variable(variable); let definition_id = expr.id; + let definition_kind = + self.interner.try_definition(definition_id).map(|definition| definition.kind.clone()); + + // Resolve any generics if we the variable we have resolved is a function + // and if the turbofish operator was used. + let generics = definition_kind.and_then(|definition_kind| match &definition_kind { + DefinitionKind::Function(function) => { + self.resolve_turbofish_generics(function, unresolved_turbofish, span) + } + _ => None, + }); + let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics.clone())); self.interner.push_expr_location(id, span, self.file); @@ -438,6 +478,7 @@ impl<'context> Elaborator<'context> { // Otherwise, then it is referring to an Identifier // This lookup allows support of such statements: let x = foo::bar::SOME_GLOBAL + 10; // If the expression is a singular indent, we search the resolver's current scope as normal. + let span = path.span(); let is_self_type_name = path.last_segment().is_self_type_name(); let (hir_ident, var_scope_index) = self.get_ident_from_path(path); @@ -448,7 +489,8 @@ impl<'context> Elaborator<'context> { self.interner.add_function_dependency(current_item, func_id); } - let variable = ReferenceId::Variable(hir_ident.location, is_self_type_name); + let variable = + ReferenceId::Reference(hir_ident.location, is_self_type_name); let function = ReferenceId::Function(func_id); self.interner.add_reference(function, variable); } @@ -460,7 +502,8 @@ impl<'context> Elaborator<'context> { self.interner.add_global_dependency(current_item, global_id); } - let variable = ReferenceId::Variable(hir_ident.location, is_self_type_name); + let variable = + ReferenceId::Reference(hir_ident.location, is_self_type_name); let global = ReferenceId::Global(global_id); self.interner.add_reference(global, variable); } @@ -477,6 +520,11 @@ impl<'context> Elaborator<'context> { DefinitionKind::Local(_) => { // only local variables can be captured by closures. self.resolve_local_variable(hir_ident.clone(), var_scope_index); + + let referenced = ReferenceId::Local(hir_ident.id); + let reference = + ReferenceId::Reference(Location::new(span, self.file), false); + self.interner.add_reference(referenced, reference); } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs index 6c556e000d5..b7016280453 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs @@ -53,7 +53,7 @@ impl<'context> Elaborator<'context> { resolver.resolve(self.def_maps, path.clone(), &mut Some(&mut references))?; for (referenced, ident) in references.iter().zip(path.segments) { - let reference = ReferenceId::Variable( + let reference = ReferenceId::Reference( Location::new(ident.span(), self.file), ident.is_self_type_name(), ); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs index 6aed0ab196d..1fc92ad28ba 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs @@ -16,7 +16,7 @@ use crate::{ macros_api::{ ForLoopStatement, ForRange, HirStatement, LetStatement, Path, Statement, StatementKind, }, - node_interner::{DefinitionId, DefinitionKind, GlobalId, StmtId}, + node_interner::{DefinitionId, DefinitionKind, GlobalId, ReferenceId, StmtId}, Type, }; @@ -256,6 +256,10 @@ impl<'context> Elaborator<'context> { typ.follow_bindings() }; + let referenced = ReferenceId::Local(ident.id); + let reference = ReferenceId::Reference(Location::new(span, self.file), false); + self.interner.add_reference(referenced, reference); + (HirLValue::Ident(ident.clone(), typ.clone()), typ, mutable) } LValue::MemberAccess { object, field_name, span } => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index 1aebe6a0ee2..698114cfb5e 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -154,9 +154,6 @@ impl<'context> Elaborator<'context> { }; if let Some(unresolved_span) = typ.span { - let reference = - ReferenceId::Variable(Location::new(unresolved_span, self.file), is_self_type_name); - match resolved_type { Type::Struct(ref struct_type, _) => { // Record the location of the type reference @@ -167,11 +164,19 @@ impl<'context> Elaborator<'context> { if !is_synthetic { let referenced = ReferenceId::Struct(struct_type.borrow().id); + let reference = ReferenceId::Reference( + Location::new(unresolved_span, self.file), + is_self_type_name, + ); self.interner.add_reference(referenced, reference); } } Type::Alias(ref alias_type, _) => { let referenced = ReferenceId::Alias(alias_type.borrow().id); + let reference = ReferenceId::Reference( + Location::new(unresolved_span, self.file), + is_self_type_name, + ); self.interner.add_reference(referenced, reference); } _ => (), @@ -364,6 +369,11 @@ impl<'context> Elaborator<'context> { self.interner.add_global_dependency(current_item, id); } + let referenced = ReferenceId::Global(id); + let reference = + ReferenceId::Reference(Location::new(path.span(), self.file), false); + self.interner.add_reference(referenced, reference); + Some(Type::Constant(self.eval_global_as_array_length(id, path))) } _ => None, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index ee9981863cb..43ab6224ea7 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -344,7 +344,7 @@ impl DefCollector { for (referenced, ident) in references.iter().zip(&collected_import.path.segments) { let reference = - ReferenceId::Variable(Location::new(ident.span(), file_id), false); + ReferenceId::Reference(Location::new(ident.span(), file_id), false); context.def_interner.add_reference(*referenced, reference); } @@ -511,25 +511,18 @@ fn add_import_reference( return; } - match def_id { - crate::macros_api::ModuleDefId::FunctionId(func_id) => { - let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); - interner.add_reference(ReferenceId::Function(func_id), variable); - } - crate::macros_api::ModuleDefId::TypeId(struct_id) => { - let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); - interner.add_reference(ReferenceId::Struct(struct_id), variable); - } - crate::macros_api::ModuleDefId::TraitId(trait_id) => { - let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); - interner.add_reference(ReferenceId::Trait(trait_id), variable); - } + let referenced = match def_id { + crate::macros_api::ModuleDefId::ModuleId(module_id) => ReferenceId::Module(module_id), + crate::macros_api::ModuleDefId::FunctionId(func_id) => ReferenceId::Function(func_id), + crate::macros_api::ModuleDefId::TypeId(struct_id) => ReferenceId::Struct(struct_id), + crate::macros_api::ModuleDefId::TraitId(trait_id) => ReferenceId::Trait(trait_id), crate::macros_api::ModuleDefId::TypeAliasId(type_alias_id) => { - let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); - interner.add_reference(ReferenceId::Alias(type_alias_id), variable); + ReferenceId::Alias(type_alias_id) } - _ => (), - } + crate::macros_api::ModuleDefId::GlobalId(global_id) => ReferenceId::Global(global_id), + }; + let reference = ReferenceId::Reference(Location::new(name.span(), file_id), false); + interner.add_reference(referenced, reference); } fn inject_prelude( diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 483b061998e..48985116f4f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -650,7 +650,7 @@ impl<'a> ModCollector<'a> { Ok(child_mod_id) => { // Track that the "foo" in `mod foo;` points to the module "foo" let referenced = ReferenceId::Module(child_mod_id); - let reference = ReferenceId::Variable(location, false); + let reference = ReferenceId::Reference(location, false); context.def_interner.add_reference(referenced, reference); errors.extend(collect_defs( diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 364d694462b..793362bb3d6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1096,8 +1096,7 @@ impl<'a> Resolver<'a> { let direct_generics = func.def.generics.iter(); let direct_generics = direct_generics - .filter_map(|generic| self.find_generic(&generic.ident().0.contents)) - .map(|ResolvedGeneric { name, type_var, .. }| (name.clone(), type_var.clone())) + .filter_map(|generic| self.find_generic(&generic.ident().0.contents).cloned()) .collect(); FuncMeta { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/mod.rs index a9a51b636a8..1a70bade863 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -22,7 +22,7 @@ use crate::{ traits::TraitConstraint, }, node_interner::{ExprId, FuncId, GlobalId, NodeInterner}, - Kind, Type, TypeBindings, + Kind, ResolvedGeneric, Type, TypeBindings, }; pub use self::errors::Source; @@ -281,8 +281,10 @@ pub(crate) fn check_trait_impl_method_matches_declaration( } // Substitute each generic on the trait function with the corresponding generic on the impl function - for ((_, trait_fn_generic), (name, impl_fn_generic)) in - trait_fn_meta.direct_generics.iter().zip(&meta.direct_generics) + for ( + ResolvedGeneric { type_var: trait_fn_generic, .. }, + ResolvedGeneric { name, type_var: impl_fn_generic, .. }, + ) in trait_fn_meta.direct_generics.iter().zip(&meta.direct_generics) { let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone(), Kind::Normal); bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), arg)); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs index a4a9f855c62..fa8bb55abee 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs @@ -1,8 +1,6 @@ use iter_extended::vecmap; use noirc_errors::{Location, Span}; -use std::rc::Rc; - use super::expr::{HirBlockExpression, HirExpression, HirIdent}; use super::stmt::HirPattern; use super::traits::TraitConstraint; @@ -10,7 +8,7 @@ use crate::ast::{FunctionKind, FunctionReturnType, Visibility}; use crate::graph::CrateId; use crate::macros_api::BlockExpression; use crate::node_interner::{ExprId, NodeInterner, TraitImplId}; -use crate::{ResolvedGeneric, Type, TypeVariable}; +use crate::{ResolvedGeneric, Type}; /// A Hir function is a block expression /// with a list of statements @@ -113,7 +111,7 @@ pub struct FuncMeta { /// This does not include generics from an outer scope, like those introduced by /// an `impl` block. This also does not include implicit generics added by the compiler /// such as a trait's `Self` type variable. - pub direct_generics: Vec<(Rc, TypeVariable)>, + pub direct_generics: Vec, /// All the generics used by this function, which includes any implicit generics or generics /// from outer scopes, such as those introduced by an impl. diff --git a/noir/noir-repo/compiler/noirc_frontend/src/locations.rs b/noir/noir-repo/compiler/noirc_frontend/src/locations.rs index 30095fbb8c7..fcaef0a8dd6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/locations.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/locations.rs @@ -48,7 +48,8 @@ impl NodeInterner { let alias_type = alias_type.borrow(); Location::new(alias_type.name.span(), alias_type.location.file) } - ReferenceId::Variable(location, _) => location, + ReferenceId::Local(id) => self.definition(id).location, + ReferenceId::Reference(location, _) => location, } } @@ -117,26 +118,18 @@ impl NodeInterner { let node_index = self.location_indices.get_node_from_location(location)?; let reference_node = self.reference_graph[node_index]; - let found_locations: Vec = match reference_node { - ReferenceId::Global(_) | ReferenceId::Module(_) => todo!(), - ReferenceId::Function(_) - | ReferenceId::Struct(_) - | ReferenceId::Trait(_) - | ReferenceId::Alias(_) => self.find_all_references_for_index( - node_index, - include_referenced, - include_self_type_name, - ), - - ReferenceId::Variable(_, _) => { - let referenced_node_index = self.referenced_index(node_index)?; - self.find_all_references_for_index( - referenced_node_index, - include_referenced, - include_self_type_name, - ) - } + let referenced_node_index = if let ReferenceId::Reference(_, _) = reference_node { + self.referenced_index(node_index)? + } else { + node_index }; + + let found_locations = self.find_all_references_for_index( + referenced_node_index, + include_referenced, + include_self_type_name, + ); + Some(found_locations) } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs index 1618058a9f0..588b56afa1a 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs @@ -245,12 +245,13 @@ pub enum ReferenceId { Global(GlobalId), Function(FuncId), Alias(TypeAliasId), - Variable(Location, bool /* is Self */), + Local(DefinitionId), + Reference(Location, bool /* is Self */), } impl ReferenceId { pub fn is_self_type_name(&self) -> bool { - matches!(self, Self::Variable(_, true)) + matches!(self, Self::Reference(_, true)) } } @@ -836,12 +837,19 @@ impl NodeInterner { location: Location, ) -> DefinitionId { let id = DefinitionId(self.definitions.len()); + let is_local = matches!(definition, DefinitionKind::Local(_)); + if let DefinitionKind::Function(func_id) = definition { self.function_definition_ids.insert(func_id, id); } let kind = definition; self.definitions.push(DefinitionInfo { name, mutable, comptime, kind, location }); + + if is_local { + self.add_definition_location(ReferenceId::Local(id)); + } + id } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index 10183ae0ac9..70f8f785d68 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -1455,6 +1455,79 @@ fn specify_method_types_with_turbofish() { assert_eq!(errors.len(), 0); } +#[test] +fn incorrect_turbofish_count_function_call() { + let src = r#" + trait Default { + fn default() -> Self; + } + + impl Default for Field { + fn default() -> Self { 0 } + } + + impl Default for u64 { + fn default() -> Self { 0 } + } + + // Need the above as we don't have access to the stdlib here. + // We also need to construct a concrete value of `U` without giving away its type + // as otherwise the unspecified type is ignored. + + fn generic_func() -> (T, U) where T: Default, U: Default { + (T::default(), U::default()) + } + + fn main() { + let _ = generic_func::(); + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::TypeError(TypeCheckError::IncorrectTurbofishGenericCount { .. }), + )); +} + +#[test] +fn incorrect_turbofish_count_method_call() { + let src = r#" + trait Default { + fn default() -> Self; + } + + impl Default for Field { + fn default() -> Self { 0 } + } + + // Need the above as we don't have access to the stdlib here. + // We also need to construct a concrete value of `U` without giving away its type + // as otherwise the unspecified type is ignored. + + struct Foo { + inner: T + } + + impl Foo { + fn generic_method(_self: Self) -> U where U: Default { + U::default() + } + } + + fn main() { + let foo: Foo = Foo { inner: 1 }; + let _ = foo.generic_method::(); + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::TypeError(TypeCheckError::IncorrectTurbofishGenericCount { .. }), + )); +} + #[test] fn struct_numeric_generic_in_function() { let src = r#" @@ -1983,3 +2056,56 @@ fn underflowing_i8() { panic!("Expected OverflowingAssignment error, got {:?}", errors[0].0); } } + +#[test] +fn turbofish_numeric_generic_nested_call() { + // Check for turbofish numeric generics used with function calls + let src = r#" + fn foo() -> [u8; N] { + [0; N] + } + + fn bar() -> [u8; N] { + foo::() + } + + global M: u32 = 3; + + fn main() { + let _ = bar::(); + } + "#; + let errors = get_program_errors(src); + assert!(errors.is_empty()); + + // Check for turbofish numeric generics used with method calls + let src = r#" + struct Foo { + a: T + } + + impl Foo { + fn static_method() -> [u8; N] { + [0; N] + } + + fn impl_method(self) -> [T; N] { + [self.a; N] + } + } + + fn bar() -> [u8; N] { + let _ = Foo::static_method::(); + let x: Foo = Foo { a: 0 }; + x.impl_method::() + } + + global M: u32 = 3; + + fn main() { + let _ = bar::(); + } + "#; + let errors = get_program_errors(src); + assert!(errors.is_empty()); +} diff --git a/noir/noir-repo/noir_stdlib/src/hash/mod.nr b/noir/noir-repo/noir_stdlib/src/hash/mod.nr index 493430c99a4..65f3b9419ff 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/mod.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/mod.nr @@ -25,6 +25,7 @@ pub fn blake3(input: [u8; N]) -> [u8; 32] // docs:end:blake3 {} +#[no_predicates] // docs:start:pedersen_commitment pub fn pedersen_commitment(input: [Field; N]) -> EmbeddedCurvePoint { // docs:end:pedersen_commitment @@ -46,6 +47,7 @@ fn pedersen_commitment_with_separator_noir(input: [Field; N], separa EmbeddedCurvePoint { x: values[0], y: values[1], is_infinite: values[2] as bool } } +#[no_predicates] pub fn pedersen_commitment_with_separator(input: [Field; N], separator: u32) -> EmbeddedCurvePoint { let values = __pedersen_commitment_with_separator(input, separator); EmbeddedCurvePoint { x: values[0], y: values[1], is_infinite: false } diff --git a/noir/noir-repo/noir_stdlib/src/hash/poseidon/bn254.nr b/noir/noir-repo/noir_stdlib/src/hash/poseidon/bn254.nr index 0e47ca11e20..103ab3d166a 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/poseidon/bn254.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/poseidon/bn254.nr @@ -12,6 +12,7 @@ pub fn sponge(msg: [Field; N]) -> Field { // Various instances of the Poseidon hash function // Consistent with Circom's implementation +#[no_predicates] pub fn hash_1(input: [Field; 1]) -> Field { let mut state = [0; 2]; for i in 0..input.len() { @@ -21,6 +22,7 @@ pub fn hash_1(input: [Field; 1]) -> Field { perm::x5_2(state)[0] } +#[no_predicates] pub fn hash_2(input: [Field; 2]) -> Field { let mut state = [0; 3]; for i in 0..input.len() { @@ -30,6 +32,7 @@ pub fn hash_2(input: [Field; 2]) -> Field { perm::x5_3(state)[0] } +#[no_predicates] pub fn hash_3(input: [Field; 3]) -> Field { let mut state = [0; 4]; for i in 0..input.len() { @@ -39,6 +42,7 @@ pub fn hash_3(input: [Field; 3]) -> Field { perm::x5_4(state)[0] } +#[no_predicates] pub fn hash_4(input: [Field; 4]) -> Field { let mut state = [0; 5]; for i in 0..input.len() { @@ -48,6 +52,7 @@ pub fn hash_4(input: [Field; 4]) -> Field { perm::x5_5(state)[0] } +#[no_predicates] pub fn hash_5(input: [Field; 5]) -> Field { let mut state = [0; 6]; for i in 0..input.len() { @@ -57,6 +62,7 @@ pub fn hash_5(input: [Field; 5]) -> Field { perm::x5_6(state)[0] } +#[no_predicates] pub fn hash_6(input: [Field; 6]) -> Field { let mut state = [0; 7]; for i in 0..input.len() { @@ -66,6 +72,7 @@ pub fn hash_6(input: [Field; 6]) -> Field { perm::x5_7(state)[0] } +#[no_predicates] pub fn hash_7(input: [Field; 7]) -> Field { let mut state = [0; 8]; for i in 0..input.len() { @@ -75,6 +82,7 @@ pub fn hash_7(input: [Field; 7]) -> Field { perm::x5_8(state)[0] } +#[no_predicates] pub fn hash_8(input: [Field; 8]) -> Field { let mut state = [0; 9]; for i in 0..input.len() { @@ -84,6 +92,7 @@ pub fn hash_8(input: [Field; 8]) -> Field { perm::x5_9(state)[0] } +#[no_predicates] pub fn hash_9(input: [Field; 9]) -> Field { let mut state = [0; 10]; for i in 0..input.len() { @@ -93,6 +102,7 @@ pub fn hash_9(input: [Field; 9]) -> Field { perm::x5_10(state)[0] } +#[no_predicates] pub fn hash_10(input: [Field; 10]) -> Field { let mut state = [0; 11]; for i in 0..input.len() { @@ -102,6 +112,7 @@ pub fn hash_10(input: [Field; 10]) -> Field { perm::x5_11(state)[0] } +#[no_predicates] pub fn hash_11(input: [Field; 11]) -> Field { let mut state = [0; 12]; for i in 0..input.len() { @@ -111,6 +122,7 @@ pub fn hash_11(input: [Field; 11]) -> Field { perm::x5_12(state)[0] } +#[no_predicates] pub fn hash_12(input: [Field; 12]) -> Field { let mut state = [0; 13]; for i in 0..input.len() { @@ -120,6 +132,7 @@ pub fn hash_12(input: [Field; 12]) -> Field { perm::x5_13(state)[0] } +#[no_predicates] pub fn hash_13(input: [Field; 13]) -> Field { let mut state = [0; 14]; for i in 0..input.len() { @@ -129,6 +142,7 @@ pub fn hash_13(input: [Field; 13]) -> Field { perm::x5_14(state)[0] } +#[no_predicates] pub fn hash_14(input: [Field; 14]) -> Field { let mut state = [0; 15]; for i in 0..input.len() { @@ -138,6 +152,7 @@ pub fn hash_14(input: [Field; 14]) -> Field { perm::x5_15(state)[0] } +#[no_predicates] pub fn hash_15(input: [Field; 15]) -> Field { let mut state = [0; 16]; for i in 0..input.len() { @@ -147,6 +162,7 @@ pub fn hash_15(input: [Field; 15]) -> Field { perm::x5_16(state)[0] } +#[no_predicates] pub fn hash_16(input: [Field; 16]) -> Field { let mut state = [0; 17]; for i in 0..input.len() { diff --git a/noir/noir-repo/noir_stdlib/src/hash/poseidon2.nr b/noir/noir-repo/noir_stdlib/src/hash/poseidon2.nr index e34992364ab..08cf68d1f82 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/poseidon2.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/poseidon2.nr @@ -11,6 +11,7 @@ struct Poseidon2 { } impl Poseidon2 { + #[no_predicates] pub fn hash(input: [Field; N], message_size: u32) -> Field { if message_size == N { Poseidon2::hash_internal(input, N, false) diff --git a/noir/noir-repo/noir_stdlib/src/sha256.nr b/noir/noir-repo/noir_stdlib/src/sha256.nr index 0161756c1d0..b5dc958e9d1 100644 --- a/noir/noir-repo/noir_stdlib/src/sha256.nr +++ b/noir/noir-repo/noir_stdlib/src/sha256.nr @@ -15,7 +15,9 @@ fn msg_u8_to_u32(msg: [u8; 64]) -> [u32; 16] { msg32 } + // SHA-256 hash function +#[no_predicates] pub fn digest(msg: [u8; N]) -> [u8; 32] { sha256_var(msg, N as u64) } diff --git a/noir/noir-repo/noir_stdlib/src/sha512.nr b/noir/noir-repo/noir_stdlib/src/sha512.nr index aed6c2878b3..be255a594af 100644 --- a/noir/noir-repo/noir_stdlib/src/sha512.nr +++ b/noir/noir-repo/noir_stdlib/src/sha512.nr @@ -49,7 +49,9 @@ fn sha_w(msg: [u64; 16]) -> [u64; 80] // Expanded message blocks } w } + // SHA-512 compression function +#[no_predicates] fn sha_c(msg: [u64; 16], hash: [u64; 8]) -> [u64; 8] { // noir-fmt:ignore let K: [u64; 80] = [4794697086780616226, 8158064640168781261, 13096744586834688815, 16840607885511220156, 4131703408338449720, 6480981068601479193, 10538285296894168987, 12329834152419229976, 15566598209576043074, 1334009975649890238, 2608012711638119052, 6128411473006802146, 8268148722764581231, 9286055187155687089, 11230858885718282805, 13951009754708518548, 16472876342353939154, 17275323862435702243, 1135362057144423861, 2597628984639134821, 3308224258029322869, 5365058923640841347, 6679025012923562964, 8573033837759648693, 10970295158949994411, 12119686244451234320, 12683024718118986047, 13788192230050041572, 14330467153632333762, 15395433587784984357, 489312712824947311, 1452737877330783856, 2861767655752347644, 3322285676063803686, 5560940570517711597, 5996557281743188959, 7280758554555802590, 8532644243296465576, 9350256976987008742, 10552545826968843579, 11727347734174303076, 12113106623233404929, 14000437183269869457, 14369950271660146224, 15101387698204529176, 15463397548674623760, 17586052441742319658, 1182934255886127544, 1847814050463011016, 2177327727835720531, 2830643537854262169, 3796741975233480872, 4115178125766777443, 5681478168544905931, 6601373596472566643, 7507060721942968483, 8399075790359081724, 8693463985226723168, 9568029438360202098, 10144078919501101548, 10430055236837252648, 11840083180663258601, 13761210420658862357, 14299343276471374635, 14566680578165727644, 15097957966210449927, 16922976911328602910, 17689382322260857208, 500013540394364858, 748580250866718886, 1242879168328830382, 1977374033974150939, 2944078676154940804, 3659926193048069267, 4368137639120453308, 4836135668995329356, 5532061633213252278, 6448918945643986474, 6902733635092675308, 7801388544844847127]; // first 64 bits of fractional parts of cube roots of first 80 primes diff --git a/noir/noir-repo/test_programs/execution_success/regression_5435/Nargo.toml b/noir/noir-repo/test_programs/execution_success/regression_5435/Nargo.toml new file mode 100644 index 00000000000..6affc423b7a --- /dev/null +++ b/noir/noir-repo/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/noir/noir-repo/test_programs/execution_success/regression_5435/Prover.toml b/noir/noir-repo/test_programs/execution_success/regression_5435/Prover.toml new file mode 100644 index 00000000000..f3e2bbe32e7 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/regression_5435/Prover.toml @@ -0,0 +1,2 @@ +input = "0" +enable = false diff --git a/noir/noir-repo/test_programs/execution_success/regression_5435/src/main.nr b/noir/noir-repo/test_programs/execution_success/regression_5435/src/main.nr new file mode 100644 index 00000000000..65f13c5b201 --- /dev/null +++ b/noir/noir-repo/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); +} diff --git a/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs b/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs index fe591a433cf..3713e8b646a 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs @@ -196,4 +196,9 @@ mod goto_definition_tests { ) .await; } + + #[test] + async fn goto_for_local_variable() { + expect_goto_for_all_references("local_variable", "some_var", 0).await; + } } diff --git a/noir/noir-repo/tooling/lsp/src/requests/rename.rs b/noir/noir-repo/tooling/lsp/src/requests/rename.rs index 54c3297afb9..ac6c6792e15 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/rename.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/rename.rs @@ -21,7 +21,7 @@ pub(crate) fn on_prepare_rename_request( let reference_id = interner.reference_at_location(location); let rename_possible = match reference_id { // Rename shouldn't be possible when triggered on top of "Self" - Some(ReferenceId::Variable(_, true /* is self type name */)) => false, + Some(ReferenceId::Reference(_, true /* is self type name */)) => false, Some(_) => true, None => false, }; @@ -189,4 +189,14 @@ mod rename_tests { async fn test_rename_type_alias() { check_rename_succeeds("rename_type_alias", "Bar").await; } + + #[test] + async fn test_rename_global() { + check_rename_succeeds("rename_global", "FOO").await; + } + + #[test] + async fn test_rename_local_variable() { + check_rename_succeeds("local_variable", "some_var").await; + } } diff --git a/noir/noir-repo/tooling/lsp/test_programs/local_variable/Nargo.toml b/noir/noir-repo/tooling/lsp/test_programs/local_variable/Nargo.toml new file mode 100644 index 00000000000..df881fb5f4d --- /dev/null +++ b/noir/noir-repo/tooling/lsp/test_programs/local_variable/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "local_variable" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/tooling/lsp/test_programs/local_variable/src/main.nr b/noir/noir-repo/tooling/lsp/test_programs/local_variable/src/main.nr new file mode 100644 index 00000000000..e41cbed085f --- /dev/null +++ b/noir/noir-repo/tooling/lsp/test_programs/local_variable/src/main.nr @@ -0,0 +1,5 @@ +fn main() { + let mut some_var = 1; + some_var = 2; + let _ = some_var; +} diff --git a/noir/noir-repo/tooling/lsp/test_programs/rename_global/Nargo.toml b/noir/noir-repo/tooling/lsp/test_programs/rename_global/Nargo.toml new file mode 100644 index 00000000000..350c6fe5506 --- /dev/null +++ b/noir/noir-repo/tooling/lsp/test_programs/rename_global/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "rename_global" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/tooling/lsp/test_programs/rename_global/src/main.nr b/noir/noir-repo/tooling/lsp/test_programs/rename_global/src/main.nr new file mode 100644 index 00000000000..3ae8cf4bfc3 --- /dev/null +++ b/noir/noir-repo/tooling/lsp/test_programs/rename_global/src/main.nr @@ -0,0 +1,22 @@ +mod foo { + global FOO = 1; +} + +use foo::FOO; + +fn main() { + let _ = foo::FOO; + let _ = FOO; + let _: [Field; FOO] = [1]; +} + +trait WithNumber { + +} + +struct Some { +} + +impl WithNumber for Some { + +} diff --git a/noir/noir-repo/tooling/lsp/test_programs/rename_type_alias/src/main.nr b/noir/noir-repo/tooling/lsp/test_programs/rename_type_alias/src/main.nr index 4a0c497a21f..2072d9cae87 100644 --- a/noir/noir-repo/tooling/lsp/test_programs/rename_type_alias/src/main.nr +++ b/noir/noir-repo/tooling/lsp/test_programs/rename_type_alias/src/main.nr @@ -3,10 +3,17 @@ mod foo { } type Bar = Foo; + + mod bar { + struct Baz { + + } + } } use foo::Foo; use foo::Bar; +use foo::bar; fn main() { let x: Bar = Foo {};